arphaman created this revision.
arphaman added reviewers: erik.pilkington, aaron.ballman.
Herald added subscribers: dexonsmith, jkorous.
We have an issue when using `#pragma clang attribute` with availability
attributes:
- The explicit attribute that's specified next to the declaration is not
guaranteed to be preferred over the attribute specified in the pragma.
This patch fixes this by introducing a `priority` field to the availability
attribute to control how they're merged. Attributes with higher priority are
applied over attributes with lower priority for the same platform. The
implicitly inferred attributes are given the lower priority. This ensures that:
1. explicit attributes are preferred over all other attributes.
2. implicitly inferred attributes that are inferred from an explicit attribute
are discarded if there's an explicit attribute or an attribute specified using
a `#pragma` for the same platform.
3. implicitly inferred attributes that are inferred from an attribute in the
`#pragma` are not used if there's an explicit, explicit `#pragma`, or an
implicit attribute inferred from an explicit attribute for the declaration.
This is the resulting ranking:
`platform availability > platform availability from pragma > inferred
availability > inferred availability from pragma`
rdar://46390243
Repository:
rC Clang
https://reviews.llvm.org/D56892
Files:
include/clang/Basic/Attr.td
include/clang/Sema/ParsedAttr.h
include/clang/Sema/Sema.h
lib/Sema/SemaAttr.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/SemaObjC/attr-availability-priority.m
Index: test/SemaObjC/attr-availability-priority.m
===================================================================
--- /dev/null
+++ test/SemaObjC/attr-availability-priority.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -triple arm64-apple-tvos12.0 -fsyntax-only -verify %s
+
+void explicit() __attribute__((availability(tvos, introduced=11.0, deprecated=12.0))); // expected-note {{marked deprecated here}}
+void inferred() __attribute__((availability(ios, introduced=11.0, deprecated=12.0))); // expected-note {{marked deprecated here}}
+void explicitOverInferred()
+__attribute__((availability(ios, introduced=11.0, deprecated=12.0)))
+__attribute__((availability(tvos, introduced=11.0)));
+void explicitOverInferred2()
+__attribute__((availability(tvos, introduced=11.0)))
+__attribute__((availability(ios, introduced=11.0, deprecated=12.0)));
+
+void simpleUsage() {
+ explicit(); // expected-warning{{'explicit' is deprecated: first deprecated in tvOS 12.0}}
+ inferred(); // expected-warning{{'inferred' is deprecated: first deprecated in tvOS 12.0}}
+ // ok, not deprecated for tvOS.
+ explicitOverInferred();
+ explicitOverInferred2();
+}
+
+#pragma clang attribute push (__attribute__((availability(tvos, introduced=11.0, deprecated=12.0))), apply_to=function)
+
+void explicitFromPragma(); // expected-note {{marked deprecated here}}
+void explicitWinsOverExplicitFromPragma() __attribute__((availability(tvos, introduced=11.0)));
+void implicitLosesOverExplicitFromPragma() __attribute__((availability(ios, introduced=11.0))); // expected-note {{marked deprecated here}}
+
+#pragma clang attribute pop
+
+#pragma clang attribute push (__attribute__((availability(ios, introduced=11.0, deprecated=12.0))), apply_to=function)
+
+void implicitFromPragma(); // expected-note {{marked deprecated here}}
+void explicitWinsOverImplicitFromPragma() __attribute__((availability(tvos, introduced=11.0)));
+void implicitWinsOverImplicitFromPragma() __attribute__((availability(ios, introduced=11.0)));
+
+#pragma clang attribute pop
+
+#pragma clang attribute push (__attribute__((availability(tvos, introduced=11.0, deprecated=12.0))), apply_to=function)
+#pragma clang attribute push (__attribute__((availability(ios, introduced=11.0, deprecated=11.3))), apply_to=function)
+
+void pragmaExplicitWinsOverPragmaImplicit(); // expected-note {{marked deprecated here}}
+
+#pragma clang attribute pop
+#pragma clang attribute pop
+
+void pragmaUsage() {
+ explicitFromPragma(); // expected-warning {{'explicitFromPragma' is deprecated: first deprecated in tvOS 12.0}}
+ explicitWinsOverExplicitFromPragma(); // ok
+ implicitLosesOverExplicitFromPragma(); // expected-warning {{'implicitLosesOverExplicitFromPragma' is deprecated: first deprecated in tvOS 12.0}}
+
+ implicitFromPragma(); // expected-warning {{'implicitFromPragma' is deprecated: first deprecated in tvOS 12.0}}
+ explicitWinsOverImplicitFromPragma(); // ok
+ implicitWinsOverImplicitFromPragma(); // ok
+ pragmaExplicitWinsOverPragmaImplicit(); // expected-warning {{'pragmaExplicitWinsOverPragmaImplicit' is deprecated: first deprecated in tvOS 12.0}}
+}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2284,18 +2284,11 @@
return false;
}
-AvailabilityAttr *Sema::mergeAvailabilityAttr(NamedDecl *D, SourceRange Range,
- IdentifierInfo *Platform,
- bool Implicit,
- VersionTuple Introduced,
- VersionTuple Deprecated,
- VersionTuple Obsoleted,
- bool IsUnavailable,
- StringRef Message,
- bool IsStrict,
- StringRef Replacement,
- AvailabilityMergeKind AMK,
- unsigned AttrSpellingListIndex) {
+AvailabilityAttr *Sema::mergeAvailabilityAttr(
+ NamedDecl *D, SourceRange Range, IdentifierInfo *Platform, bool Implicit,
+ VersionTuple Introduced, VersionTuple Deprecated, VersionTuple Obsoleted,
+ bool IsUnavailable, StringRef Message, bool IsStrict, StringRef Replacement,
+ AvailabilityMergeKind AMK, int Priority, unsigned AttrSpellingListIndex) {
VersionTuple MergedIntroduced = Introduced;
VersionTuple MergedDeprecated = Deprecated;
VersionTuple MergedObsoleted = Obsoleted;
@@ -2329,16 +2322,15 @@
}
// If there is an existing availability attribute for this platform that
- // is explicit and the new one is implicit use the explicit one and
- // discard the new implicit attribute.
- if (!OldAA->isImplicit() && Implicit) {
+ // has a higher priority use the existing one and discard the new
+ // attribute.
+ if (OldAA->getPriority() > Priority)
return nullptr;
- }
- // If there is an existing attribute for this platform that is implicit
- // and the new attribute is explicit then erase the old one and
- // continue processing the attributes.
- if (!Implicit && OldAA->isImplicit()) {
+ // If there is an existing attribute for this platform that has a lower
+ // priority than the new attribute then erase the old one and continue
+ // processing the attributes.
+ if (OldAA->getPriority() < Priority) {
Attrs.erase(Attrs.begin() + i);
--e;
continue;
@@ -2437,11 +2429,10 @@
if (!checkAvailabilityAttr(*this, Range, Platform, MergedIntroduced,
MergedDeprecated, MergedObsoleted) &&
!OverrideOrImpl) {
- auto *Avail = ::new (Context) AvailabilityAttr(Range, Context, Platform,
- Introduced, Deprecated,
- Obsoleted, IsUnavailable, Message,
- IsStrict, Replacement,
- AttrSpellingListIndex);
+ auto *Avail = ::new (Context)
+ AvailabilityAttr(Range, Context, Platform, Introduced, Deprecated,
+ Obsoleted, IsUnavailable, Message, IsStrict,
+ Replacement, Priority, AttrSpellingListIndex);
Avail->setImplicit(Implicit);
return Avail;
}
@@ -2484,15 +2475,12 @@
}
}
- AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(ND, AL.getRange(), II,
- false/*Implicit*/,
- Introduced.Version,
- Deprecated.Version,
- Obsoleted.Version,
- IsUnavailable, Str,
- IsStrict, Replacement,
- Sema::AMK_None,
- Index);
+ int PriorityModifier =
+ AL.isPragmaClangAttribute() ? Sema::AP_PragmaClangAttribute : 0;
+ AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(
+ ND, AL.getRange(), II, false /*Implicit*/, Introduced.Version,
+ Deprecated.Version, Obsoleted.Version, IsUnavailable, Str, IsStrict,
+ Replacement, Sema::AMK_None, PriorityModifier + Sema::AP_Explicit, Index);
if (NewAttr)
D->addAttr(NewAttr);
@@ -2528,18 +2516,11 @@
auto NewDeprecated = adjustWatchOSVersion(Deprecated.Version);
auto NewObsoleted = adjustWatchOSVersion(Obsoleted.Version);
- AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(ND,
- AL.getRange(),
- NewII,
- true/*Implicit*/,
- NewIntroduced,
- NewDeprecated,
- NewObsoleted,
- IsUnavailable, Str,
- IsStrict,
- Replacement,
- Sema::AMK_None,
- Index);
+ AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(
+ ND, AL.getRange(), NewII, true /*Implicit*/, NewIntroduced,
+ NewDeprecated, NewObsoleted, IsUnavailable, Str, IsStrict,
+ Replacement, Sema::AMK_None,
+ PriorityModifier + Sema::AP_InferredFromOtherPlatform, Index);
if (NewAttr)
D->addAttr(NewAttr);
}
@@ -2553,20 +2534,13 @@
NewII = &S.Context.Idents.get("tvos_app_extension");
if (NewII) {
- AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(ND,
- AL.getRange(),
- NewII,
- true/*Implicit*/,
- Introduced.Version,
- Deprecated.Version,
- Obsoleted.Version,
- IsUnavailable, Str,
- IsStrict,
- Replacement,
- Sema::AMK_None,
- Index);
- if (NewAttr)
- D->addAttr(NewAttr);
+ AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(
+ ND, AL.getRange(), NewII, true /*Implicit*/, Introduced.Version,
+ Deprecated.Version, Obsoleted.Version, IsUnavailable, Str, IsStrict,
+ Replacement, Sema::AMK_None,
+ PriorityModifier + Sema::AP_InferredFromOtherPlatform, Index);
+ if (NewAttr)
+ D->addAttr(NewAttr);
}
}
}
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2427,13 +2427,11 @@
InheritableAttr *NewAttr = nullptr;
unsigned AttrSpellingListIndex = Attr->getSpellingListIndex();
if (const auto *AA = dyn_cast<AvailabilityAttr>(Attr))
- NewAttr = S.mergeAvailabilityAttr(D, AA->getRange(), AA->getPlatform(),
- AA->isImplicit(), AA->getIntroduced(),
- AA->getDeprecated(),
- AA->getObsoleted(), AA->getUnavailable(),
- AA->getMessage(), AA->getStrict(),
- AA->getReplacement(), AMK,
- AttrSpellingListIndex);
+ NewAttr = S.mergeAvailabilityAttr(
+ D, AA->getRange(), AA->getPlatform(), AA->isImplicit(),
+ AA->getIntroduced(), AA->getDeprecated(), AA->getObsoleted(),
+ AA->getUnavailable(), AA->getMessage(), AA->getStrict(),
+ AA->getReplacement(), AMK, AA->getPriority(), AttrSpellingListIndex);
else if (const auto *VA = dyn_cast<VisibilityAttr>(Attr))
NewAttr = S.mergeVisibilityAttr(D, VA->getRange(), VA->getVisibility(),
AttrSpellingListIndex);
Index: lib/Sema/SemaAttr.cpp
===================================================================
--- lib/Sema/SemaAttr.cpp
+++ lib/Sema/SemaAttr.cpp
@@ -523,6 +523,7 @@
void Sema::ActOnPragmaAttributeAttribute(
ParsedAttr &Attribute, SourceLocation PragmaLoc,
attr::ParsedSubjectMatchRuleSet Rules) {
+ Attribute.setIsPragmaClangAttribute();
SmallVector<attr::SubjectMatchRule, 4> SubjectMatchRules;
// Gather the subject match rules that are supported by the attribute.
SmallVector<std::pair<attr::SubjectMatchRule, bool>, 4>
@@ -680,6 +681,8 @@
for (auto &Entry : Group.Entries) {
ParsedAttr *Attribute = Entry.Attribute;
assert(Attribute && "Expected an attribute");
+ assert(Attribute->isPragmaClangAttribute() &&
+ "expected #pragma clang attribute");
// Ensure that the attribute can be applied to the given declaration.
bool Applies = false;
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2456,18 +2456,35 @@
AMK_ProtocolImplementation,
};
+ /// Describes the kind of priority given to an availability attribute.
+ ///
+ /// The sum of priorities deteremines the final priority of the attribute.
+ /// The final priority determines how the attribute will be merged.
+ /// An attribute with a higher priority will always remove lower priority
+ /// attributes for the specified platform when it is being applied. An
+ /// attribute with a lower priority will not be applied if the declaration
+ /// already has an availability attribute with a higher priority for the
+ /// specified platform.
+ enum AvailabilityPriority : int {
+ /// The availability attribute was specified explicitly next to the
+ /// declaration.
+ AP_Explicit = 0,
+
+ /// The availability attribute was applied using '#pragma clang attribute'.
+ AP_PragmaClangAttribute = -1,
+
+ /// The availability attribute for a specific platform was inferred from
+ /// an availability attribute for another platform.
+ AP_InferredFromOtherPlatform = -2
+ };
+
/// Attribute merging methods. Return true if a new attribute was added.
- AvailabilityAttr *mergeAvailabilityAttr(NamedDecl *D, SourceRange Range,
- IdentifierInfo *Platform,
- bool Implicit,
- VersionTuple Introduced,
- VersionTuple Deprecated,
- VersionTuple Obsoleted,
- bool IsUnavailable,
- StringRef Message,
- bool IsStrict, StringRef Replacement,
- AvailabilityMergeKind AMK,
- unsigned AttrSpellingListIndex);
+ AvailabilityAttr *mergeAvailabilityAttr(
+ NamedDecl *D, SourceRange Range, IdentifierInfo *Platform, bool Implicit,
+ VersionTuple Introduced, VersionTuple Deprecated, VersionTuple Obsoleted,
+ bool IsUnavailable, StringRef Message, bool IsStrict,
+ StringRef Replacement, AvailabilityMergeKind AMK, int Priority,
+ unsigned AttrSpellingListIndex);
TypeVisibilityAttr *mergeTypeVisibilityAttr(Decl *D, SourceRange Range,
TypeVisibilityAttr::VisibilityType Vis,
unsigned AttrSpellingListIndex);
Index: include/clang/Sema/ParsedAttr.h
===================================================================
--- include/clang/Sema/ParsedAttr.h
+++ include/clang/Sema/ParsedAttr.h
@@ -208,6 +208,9 @@
/// A cached value.
mutable unsigned ProcessingCache : 8;
+ /// True if the attribute is specified using '#pragma clang attribute'.
+ mutable unsigned IsPragmaClangAttribute : 1;
+
/// The location of the 'unavailable' keyword in an
/// availability attribute.
SourceLocation UnavailableLoc;
@@ -239,7 +242,8 @@
ScopeLoc(scopeLoc), EllipsisLoc(ellipsisLoc), NumArgs(numArgs),
SyntaxUsed(syntaxUsed), Invalid(false), UsedAsTypeAttr(false),
IsAvailability(false), IsTypeTagForDatatype(false), IsProperty(false),
- HasParsedType(false), HasProcessingCache(false) {
+ HasParsedType(false), HasProcessingCache(false),
+ IsPragmaClangAttribute(false) {
if (numArgs) memcpy(getArgsBuffer(), args, numArgs * sizeof(ArgsUnion));
AttrKind = getKind(getName(), getScopeName(), syntaxUsed);
}
@@ -256,8 +260,8 @@
ScopeLoc(scopeLoc), NumArgs(1), SyntaxUsed(syntaxUsed), Invalid(false),
UsedAsTypeAttr(false), IsAvailability(true),
IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
- HasProcessingCache(false), UnavailableLoc(unavailable),
- MessageExpr(messageExpr) {
+ HasProcessingCache(false), IsPragmaClangAttribute(false),
+ UnavailableLoc(unavailable), MessageExpr(messageExpr) {
ArgsUnion PVal(Parm);
memcpy(getArgsBuffer(), &PVal, sizeof(ArgsUnion));
new (getAvailabilityData()) detail::AvailabilityData(
@@ -274,7 +278,7 @@
ScopeLoc(scopeLoc), NumArgs(3), SyntaxUsed(syntaxUsed), Invalid(false),
UsedAsTypeAttr(false), IsAvailability(false),
IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(false),
- HasProcessingCache(false) {
+ HasProcessingCache(false), IsPragmaClangAttribute(false) {
ArgsUnion *Args = getArgsBuffer();
Args[0] = Parm1;
Args[1] = Parm2;
@@ -291,7 +295,7 @@
ScopeLoc(scopeLoc), NumArgs(1), SyntaxUsed(syntaxUsed), Invalid(false),
UsedAsTypeAttr(false), IsAvailability(false),
IsTypeTagForDatatype(true), IsProperty(false), HasParsedType(false),
- HasProcessingCache(false) {
+ HasProcessingCache(false), IsPragmaClangAttribute(false) {
ArgsUnion PVal(ArgKind);
memcpy(getArgsBuffer(), &PVal, sizeof(ArgsUnion));
detail::TypeTagForDatatypeData &ExtraData = getTypeTagForDatatypeDataSlot();
@@ -309,7 +313,7 @@
ScopeLoc(scopeLoc), NumArgs(0), SyntaxUsed(syntaxUsed), Invalid(false),
UsedAsTypeAttr(false), IsAvailability(false),
IsTypeTagForDatatype(false), IsProperty(false), HasParsedType(true),
- HasProcessingCache(false) {
+ HasProcessingCache(false), IsPragmaClangAttribute(false) {
new (&getTypeBuffer()) ParsedType(typeArg);
AttrKind = getKind(getName(), getScopeName(), syntaxUsed);
}
@@ -323,7 +327,7 @@
ScopeLoc(scopeLoc), NumArgs(0), SyntaxUsed(syntaxUsed), Invalid(false),
UsedAsTypeAttr(false), IsAvailability(false),
IsTypeTagForDatatype(false), IsProperty(true), HasParsedType(false),
- HasProcessingCache(false) {
+ HasProcessingCache(false), IsPragmaClangAttribute(false) {
new (&getPropertyDataBuffer()) detail::PropertyData(getterId, setterId);
AttrKind = getKind(getName(), getScopeName(), syntaxUsed);
}
@@ -437,6 +441,11 @@
bool isUsedAsTypeAttr() const { return UsedAsTypeAttr; }
void setUsedAsTypeAttr() { UsedAsTypeAttr = true; }
+ /// True if the attribute is specified using '#pragma clang attribute'.
+ bool isPragmaClangAttribute() const { return IsPragmaClangAttribute; }
+
+ void setIsPragmaClangAttribute() { IsPragmaClangAttribute = true; }
+
bool isPackExpansion() const { return EllipsisLoc.isValid(); }
SourceLocation getEllipsisLoc() const { return EllipsisLoc; }
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -715,7 +715,8 @@
let Args = [IdentifierArgument<"platform">, VersionArgument<"introduced">,
VersionArgument<"deprecated">, VersionArgument<"obsoleted">,
BoolArgument<"unavailable">, StringArgument<"message">,
- BoolArgument<"strict">, StringArgument<"replacement">];
+ BoolArgument<"strict">, StringArgument<"replacement">,
+ IntArgument<"priority">];
let AdditionalMembers =
[{static llvm::StringRef getPrettyPlatformName(llvm::StringRef Platform) {
return llvm::StringSwitch<llvm::StringRef>(Platform)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
