[Clang][Sema] clang generates incorrect fix-its for API_AVAILABLE

[ian-twilightcoder]

Apple's API_AVAILABLE macro has its own notion of platform names which are supported by __API_AVAILABLE_PLATFORM_ 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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Ian Anderson (ian-twilightcoder)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/105855.diff

4 Files Affected:

• (modified) clang/include/clang/Basic/Attr.td (+42)
• (modified) clang/lib/Sema/SemaAvailability.cpp (+28-7)
• (modified) clang/test/FixIt/fixit-availability-maccatalyst.m (+4-3)
• (modified) clang/test/FixIt/fixit-availability.mm (+2-1)

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..bd6e85110ed180 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -488,22 +488,43 @@ 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.
+      llvm::Twine MacroPrefix = "__API_AVAILABLE_PLATFORM_";
+      StringRef AvailabilityPlatform = {};
+      for (StringRef EquivalentPlatform :
+           AvailabilityAttr::equivalentPlatformNames(PlatformName)) {
+        if (PP.isMacroDefined((MacroPrefix + EquivalentPlatform).str())) {
+          AvailabilityPlatform = EquivalentPlatform;
+          break;
+        }
+      }
+      if (AvailabilityPlatform.empty())
+        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(" +
+           AvailabilityPlatform + "(" + 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

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ian-twilightcoder]

std/algorithms/alg.sorting/alg.merge/pstl.merge.pass.cpp test failure doesn't look related.

[ian-twilightcoder]

std/algorithms/alg.sorting/alg.merge/pstl.merge.pass.cpp test failure doesn't look related.

Maybe a rebase will fix it.

[cachemeifyoucan]

LGTM in general.