https://github.com/ian-twilightcoder updated https://github.com/llvm/llvm-project/pull/105855
>From 540420bb178d2e2b0b92caf1741ec71bde48184f Mon Sep 17 00:00:00 2001 From: Ian Anderson <i...@apple.com> Date: Fri, 23 Aug 2024 09:43:22 -0700 Subject: [PATCH] [Clang][Sema] clang generates incorrect fix-its for API_AVAILABLE Apple's API_AVAILABLE macro has its own notion of platform names which are supported by __API_AVAILABLE_PLATFORM_<name> macros. They don't follow a consistent naming convention, but there's at least one that matches a valid availability attribute platform name. Instead of lowercasing the source spelling name, search for a defined macro and use that in the fix-it. --- clang/include/clang/Basic/Attr.td | 42 +++++++++++++++++++ clang/lib/Sema/SemaAvailability.cpp | 33 +++++++++++---- .../FixIt/fixit-availability-maccatalyst.m | 7 ++-- clang/test/FixIt/fixit-availability.mm | 3 +- 4 files changed, 74 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 98bedfe20f5d98..cb7cc5502c912e 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1073,6 +1073,48 @@ static llvm::StringRef canonicalizePlatformName(llvm::StringRef Platform) { .Case("ShaderModel", "shadermodel") .Default(Platform); } +static llvm::ArrayRef<llvm::StringRef> equivalentPlatformNames(llvm::StringRef Platform) { + // Apple's API_AVAILABLE macro expands roughly like this. + // API_AVAILABLE(ios(17.0)) + // __attribute__((availability(__API_AVAILABLE_PLATFORM_ios(17.0))) + // __attribute__((availability(ios,introduced=17.0))) + // In other words, the platform name in API_AVAILABLE has to be backed by an + // __API_AVAILABLE_PLATFORM_ macro. The __API_AVAILABLE_PLATFORM_ macros + // aren't consistent with using the canonical platform name, source spelling + // name, or one of the other supported names (i.e. one of the keys in + // canonicalizePlatformName that's neither). This function maps each + // platform name to the equivalent platform names to facilitate finding the + // platform name that has an __API_AVAILABLE_PLATFORM_ macro for use with + // API_AVAILABLE. + return llvm::StringSwitch<llvm::ArrayRef<llvm::StringRef>>(Platform) + .Case("ios", {"ios", "iOS"}) + .Case("iOS", {"ios", "iOS"}) + .Case("macos", {"macos", "macOS"}) + .Case("macOS", {"macos", "macOS"}) + .Case("tvos", {"tvos", "tvOS"}) + .Case("tvOS", {"tvos", "tvOS"}) + .Case("watchos", {"watchos", "watchOS"}) + .Case("watchOS", {"watchos", "watchOS"}) + .Case("ios_app_extension", {"iOSApplicationExtension", "ios_app_extension"}) + .Case("iOSApplicationExtension", {"iOSApplicationExtension", "ios_app_extension"}) + .Case("macos_app_extension", {"macOSApplicationExtension", "macos_app_extension"}) + .Case("macOSApplicationExtension", {"macOSApplicationExtension", "macos_app_extension"}) + .Case("tvos_app_extension", {"tvOSApplicationExtension", "tvos_app_extension"}) + .Case("tvOSApplicationExtension", {"tvOSApplicationExtension", "tvos_app_extension"}) + .Case("watchos_app_extension", {"watchOSApplicationExtension", "watchos_app_extension"}) + .Case("watchOSApplicationExtension", {"watchOSApplicationExtension", "watchos_app_extension"}) + .Case("maccatalyst", {"macCatalyst", "maccatalyst"}) + .Case("macCatalyst", {"macCatalyst", "maccatalyst"}) + .Case("maccatalyst_app_extension", {"macCatalystApplicationExtension", "maccatalyst_app_extension"}) + .Case("macCatalystApplicationExtension", {"macCatalystApplicationExtension", "maccatalyst_app_extension"}) + .Case("xros", {"visionos", "visionOS", "xros"}) + .Case("visionOS", {"visionos", "visionOS", "xros"}) + .Case("visionos", {"visionos", "visionOS", "xros"}) + .Case("xros_app_extension", {"visionOSApplicationExtension", "visionos_app_extension", "xros_app_extension"}) + .Case("visionOSApplicationExtension", {"visionOSApplicationExtension", "visionos_app_extension", "xros_app_extension"}) + .Case("visionos_app_extension", {"visionOSApplicationExtension", "visionos_app_extension", "xros_app_extension"}) + .Default({Platform}); +} static llvm::Triple::EnvironmentType getEnvironmentType(llvm::StringRef Environment) { return llvm::StringSwitch<llvm::Triple::EnvironmentType>(Environment) .Case("pixel", llvm::Triple::Pixel) diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp index 17566c226ec807..66339c031709e2 100644 --- a/clang/lib/Sema/SemaAvailability.cpp +++ b/clang/lib/Sema/SemaAvailability.cpp @@ -488,22 +488,41 @@ static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K, // Don't offer a fixit for declarations with availability attributes. if (Enclosing->hasAttr<AvailabilityAttr>()) return; - if (!S.getPreprocessor().isMacroDefined("API_AVAILABLE")) + Preprocessor &PP = S.getPreprocessor(); + if (!PP.isMacroDefined("API_AVAILABLE")) return; std::optional<AttributeInsertion> Insertion = createAttributeInsertion( Enclosing, S.getSourceManager(), S.getLangOpts()); if (!Insertion) return; - std::string PlatformName = - AvailabilityAttr::getPlatformNameSourceSpelling( - S.getASTContext().getTargetInfo().getPlatformName()) - .lower(); + StringRef PlatformName = + S.getASTContext().getTargetInfo().getPlatformName(); + + // Apple's API_AVAILABLE macro expands roughly like this. + // API_AVAILABLE(ios(17.0)) + // __attribute__((availability(__API_AVAILABLE_PLATFORM_ios(17.0))) + // __attribute__((availability(ios,introduced=17.0))) + // In other words, the platform name in API_AVAILABLE has to be backed by an + // __API_AVAILABLE_PLATFORM_ macro. The __API_AVAILABLE_PLATFORM_ macros + // aren't consistent with using the canonical platform name, source spelling + // name, or one of the other supported names (i.e. one of the keys in + // canonicalizePlatformName that's neither). Look for a macro matching any + // of the supported attribute names. + ArrayRef<StringRef> EquivalentPlatforms = + AvailabilityAttr::equivalentPlatformNames(PlatformName); + llvm::Twine MacroPrefix = "__API_AVAILABLE_PLATFORM_"; + const StringRef *AvailablePlatform = + llvm::find_if(EquivalentPlatforms, [&](StringRef EquivalentPlatform) { + return PP.isMacroDefined((MacroPrefix + EquivalentPlatform).str()); + }); + if (AvailablePlatform == EquivalentPlatforms.end()) + return; std::string Introduced = OffendingDecl->getVersionIntroduced().getAsString(); FixitNoteDiag << FixItHint::CreateInsertion( Insertion->Loc, - (llvm::Twine(Insertion->Prefix) + "API_AVAILABLE(" + PlatformName + - "(" + Introduced + "))" + Insertion->Suffix) + (llvm::Twine(Insertion->Prefix) + "API_AVAILABLE(" + + *AvailablePlatform + "(" + Introduced + "))" + Insertion->Suffix) .str()); } return; diff --git a/clang/test/FixIt/fixit-availability-maccatalyst.m b/clang/test/FixIt/fixit-availability-maccatalyst.m index 2fa03d718eded5..1b4cec8a9fe4db 100644 --- a/clang/test/FixIt/fixit-availability-maccatalyst.m +++ b/clang/test/FixIt/fixit-availability-maccatalyst.m @@ -11,14 +11,15 @@ int use(void) { // CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:14-[[@LINE-2]]:14}:"\n } else {\n // Fallback on earlier versions\n }" } -#define API_AVAILABLE(X) __attribute__((availability(macCatalyst, introduced=13.2))) __attribute__((availability(ios, introduced=13.1))) // dummy macro +#define API_AVAILABLE(x) __attribute__((availability(__API_AVAILABLE_PLATFORM_##x))) +#define __API_AVAILABLE_PLATFORM_macCatalyst(x) macCatalyst,introduced=x -API_AVAILABLE(macos(10.12)) +API_AVAILABLE(macCatalyst(13.2)) @interface NewClass @end @interface OldButOfferFixit @property(copy) NewClass *prop; -// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:1-[[@LINE-2]]:1}:"API_AVAILABLE(maccatalyst(13.2))\n" +// CHECK: fix-it:{{.*}}:{[[@LINE-2]]:1-[[@LINE-2]]:1}:"API_AVAILABLE(macCatalyst(13.2))\n" @end diff --git a/clang/test/FixIt/fixit-availability.mm b/clang/test/FixIt/fixit-availability.mm index a5660825327f77..1d802985e1d894 100644 --- a/clang/test/FixIt/fixit-availability.mm +++ b/clang/test/FixIt/fixit-availability.mm @@ -118,7 +118,8 @@ void wrapDeclStmtUses() { } } -#define API_AVAILABLE(X) __attribute__((availability(macos, introduced=10.12))) // dummy macro +#define API_AVAILABLE(x) __attribute__((availability(__API_AVAILABLE_PLATFORM_##x))) +#define __API_AVAILABLE_PLATFORM_macos(x) macos,introduced=x API_AVAILABLE(macos(10.12)) @interface NewClass _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits