Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 26 additions & 7 deletions clang/lib/Sema/SemaAvailability.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions clang/test/FixIt/fixit-availability-maccatalyst.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 2 additions & 1 deletion clang/test/FixIt/fixit-availability.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading