[clang] [Clang][Sema] clang generates incorrect fix-its for API_AVAILABLE (PR #105855)
Ian Anderson via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 3 11:04:28 PDT 2024
https://github.com/ian-twilightcoder updated https://github.com/llvm/llvm-project/pull/105855
>From f54f7bf02ae2a93e9b321be181b59485e241a51a 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 | 30 +++++++++++++++++
clang/lib/Sema/SemaAvailability.cpp | 33 +++++++++++++++----
.../FixIt/fixit-availability-maccatalyst.m | 7 ++--
clang/test/FixIt/fixit-availability.mm | 3 +-
4 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 79d77b4d49b8fe..8d2a362abc3c32 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1073,6 +1073,36 @@ static llvm::StringRef canonicalizePlatformName(llvm::StringRef Platform) {
.Case("ShaderModel", "shadermodel")
.Default(Platform);
}
+static std::vector<llvm::StringRef> equivalentPlatformNames(llvm::StringRef Platform) {
+ return llvm::StringSwitch<std::vector<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..e04cbeec165552 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 order to figure out which platform name to use in the API_AVAILABLE
+ // macro, the associated __API_AVAILABLE_PLATFORM_ macro needs to be
+ // found. The __API_AVAILABLE_PLATFORM_ macros aren't consistent about
+ // 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). Check all of the supported names for a match.
+ std::vector<StringRef> EquivalentPlatforms =
+ AvailabilityAttr::equivalentPlatformNames(PlatformName);
+ llvm::Twine MacroPrefix = "__API_AVAILABLE_PLATFORM_";
+ auto 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