[clang] [Clang][Sema] clang generates incorrect fix-its for API_AVAILABLE (PR #105855)
Ian Anderson via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 23 11:22:34 PDT 2024
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 <iana at 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
More information about the cfe-commits
mailing list