r284265 - [Sema] Refactor context checking for availability diagnostics
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 18 08:47:52 PDT 2016
Hi Bob,
I think the code in the header is fine here, so I reverted in r284486. Here’s a reduced version:
typedef int unavail_int __attribute__((availability(tvos, unavailable)));
__attribute__((availability(tvos, unavailable)))
@interface A
extern unavail_int foo;
@end
The problem is that ‘foo’ is actually at file context, not in the context of the interface, because we temporarily switched contexts to parse it. This means we can’t just look at DeclContexts in the DelayedDiagnostic case, which is what I was doing here. I’ll try to get a fix out for this soon!
Thanks for pointing this out!
Erik
> On Oct 18, 2016, at 1:37 AM, Bob Wilson <bob.wilson at apple.com> wrote:
>
> Hi Erik,
>
> This change does not work with one of the headers from the AVFoundation framework in tvOS 10.0. We can try to get a fix into the tvOS SDK, but it will probably be a while before we could release an SDK with that change. In the meantime, this is kind of disruptive. Can you find a way to keep the previous behavior, at least until we have a chance to fix that header?
>
> The header in question is System/Library/Frameworks/AVFoundation.framework/Headers/AVCaptureDevice.h in the AppleTVSimulator.sdk directory from Xcode 8.0. The problematic declaration is this one:
>
> AVF_EXPORT const AVCaptureWhiteBalanceGains AVCaptureWhiteBalanceGainsCurrent NS_AVAILABLE_IOS(8_0);
>
> The problem is that the type is declared like this:
>
> typedef struct {
> float redGain;
> float greenGain;
> float blueGain;
> } AVCaptureWhiteBalanceGains NS_AVAILABLE_IOS(8_0) __TVOS_PROHIBITED;
>
> The variable is missing the __TVOS_PROHIBITED attribute. You can reproduce this easily:
>
> $ cat t.m
> #import <AVFoundation/AVFoundation.h>
> $ clang -c -arch x86_64 -mtvos-simulator-version-min=10.0 -isysroot /Applications/Xcode-8.0.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk t.m
> /Applications/Xcode-8.0.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk/System/Library/Frameworks/AVFoundation.framework/Headers/AVCaptureDevice.h:1184:14: error:
> 'AVCaptureWhiteBalanceGains' is unavailable: not available on tvOS
> extern const AVCaptureWhiteBalanceGains AVCaptureWhiteBalanceGainsCurren...
> ^
> /Applications/Xcode-8.0.app/Contents/Developer/Platforms/AppleTVSimulator.platform/Developer/SDKs/AppleTVSimulator.sdk/System/Library/Frameworks/AVFoundation.framework/Headers/AVCaptureDevice.h:1081:3: note:
> 'AVCaptureWhiteBalanceGains' has been explicitly marked unavailable here
> } AVCaptureWhiteBalanceGains __attribute__((availability(ios,introduced=...
> ^
> 1 error generated.
>
> Maybe we can carve out an exception based on the fact that this is just an extern declaration of the variable — it’s not actually being used here. It is also defined within the @interface for an Objective-C class, in case that helps at all.
>
>> On Oct 14, 2016, at 12:08 PM, Erik Pilkington via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>>
>> Author: epilk
>> Date: Fri Oct 14 14:08:01 2016
>> New Revision: 284265
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=284265&view=rev
>> Log:
>> [Sema] Refactor context checking for availability diagnostics
>>
>> This commit combines a couple of redundant functions that do availability
>> attribute context checking into a more correct/simpler one.
>>
>> Differential revision: https://reviews.llvm.org/D25283
>>
>> Modified:
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/test/SemaObjC/class-unavail-warning.m
>>
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=284265&r1=284264&r2=284265&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Fri Oct 14 14:08:01 2016
>> @@ -9889,23 +9889,16 @@ public:
>> return OriginalLexicalContext ? OriginalLexicalContext : CurContext;
>> }
>>
>> - AvailabilityResult getCurContextAvailability() const;
>> -
>> - /// \brief Get the verison that this context implies.
>> - /// For instance, a method in an interface that is annotated with an
>> - /// availability attribuite effectively has the availability of the interface.
>> - VersionTuple getVersionForDecl(const Decl *Ctx) const;
>> -
>> /// \brief The diagnostic we should emit for \c D, or \c AR_Available.
>> ///
>> /// \param D The declaration to check. Note that this may be altered to point
>> /// to another declaration that \c D gets it's availability from. i.e., we
>> /// walk the list of typedefs to find an availability attribute.
>> ///
>> - /// \param ContextVersion The version to compare availability against.
>> - AvailabilityResult
>> - ShouldDiagnoseAvailabilityOfDecl(NamedDecl *&D, VersionTuple ContextVersion,
>> - std::string *Message);
>> + /// \param Message If non-null, this will be populated with the message from
>> + /// the availability attribute that is selected.
>> + AvailabilityResult ShouldDiagnoseAvailabilityOfDecl(NamedDecl *&D,
>> + std::string *Message);
>>
>> const DeclContext *getCurObjCLexicalContext() const {
>> const DeclContext *DC = getCurLexicalContext();
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=284265&r1=284264&r2=284265&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Oct 14 14:08:01 2016
>> @@ -15615,29 +15615,3 @@ void Sema::ActOnPragmaWeakAlias(Identifi
>> Decl *Sema::getObjCDeclContext() const {
>> return (dyn_cast_or_null<ObjCContainerDecl>(CurContext));
>> }
>> -
>> -AvailabilityResult Sema::getCurContextAvailability() const {
>> - const Decl *D = cast_or_null<Decl>(getCurObjCLexicalContext());
>> - if (!D)
>> - return AR_Available;
>> -
>> - // If we are within an Objective-C method, we should consult
>> - // both the availability of the method as well as the
>> - // enclosing class. If the class is (say) deprecated,
>> - // the entire method is considered deprecated from the
>> - // purpose of checking if the current context is deprecated.
>> - if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
>> - AvailabilityResult R = MD->getAvailability();
>> - if (R != AR_Available)
>> - return R;
>> - D = MD->getClassInterface();
>> - }
>> - // If we are within an Objective-c @implementation, it
>> - // gets the same availability context as the @interface.
>> - else if (const ObjCImplementationDecl *ID =
>> - dyn_cast<ObjCImplementationDecl>(D)) {
>> - D = ID->getClassInterface();
>> - }
>> - // Recover from user error.
>> - return D ? D->getAvailability() : AR_Available;
>> -}
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=284265&r1=284264&r2=284265&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Oct 14 14:08:01 2016
>> @@ -6317,30 +6317,6 @@ static void handleDelayedForbiddenType(S
>> diag.Triggered = true;
>> }
>>
>> -static bool isDeclDeprecated(Decl *D) {
>> - do {
>> - if (D->isDeprecated())
>> - return true;
>> - // A category implicitly has the availability of the interface.
>> - if (const ObjCCategoryDecl *CatD = dyn_cast<ObjCCategoryDecl>(D))
>> - if (const ObjCInterfaceDecl *Interface = CatD->getClassInterface())
>> - return Interface->isDeprecated();
>> - } while ((D = cast_or_null<Decl>(D->getDeclContext())));
>> - return false;
>> -}
>> -
>> -static bool isDeclUnavailable(Decl *D) {
>> - do {
>> - if (D->isUnavailable())
>> - return true;
>> - // A category implicitly has the availability of the interface.
>> - if (const ObjCCategoryDecl *CatD = dyn_cast<ObjCCategoryDecl>(D))
>> - if (const ObjCInterfaceDecl *Interface = CatD->getClassInterface())
>> - return Interface->isUnavailable();
>> - } while ((D = cast_or_null<Decl>(D->getDeclContext())));
>> - return false;
>> -}
>> -
>> static const AvailabilityAttr *getAttrForPlatform(ASTContext &Context,
>> const Decl *D) {
>> // Check each AvailabilityAttr to find the one for this platform.
>> @@ -6369,6 +6345,49 @@ static const AvailabilityAttr *getAttrFo
>> return nullptr;
>> }
>>
>> +/// \brief whether we should emit a diagnostic for \c K and \c DeclVersion in
>> +/// the context of \c Ctx. For example, we should emit an unavailable diagnostic
>> +/// in a deprecated context, but not the other way around.
>> +static bool ShouldDiagnoseAvailabilityInContext(Sema &S, AvailabilityResult K,
>> + VersionTuple DeclVersion,
>> + Decl *Ctx) {
>> + assert(K != AR_Available && "Expected an unavailable declaration here!");
>> +
>> + // Checks if we should emit the availability diagnostic in the context of C.
>> + auto CheckContext = [&](const Decl *C) {
>> + if (K == AR_NotYetIntroduced) {
>> + if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, C))
>> + if (AA->getIntroduced() >= DeclVersion)
>> + return true;
>> + } else if (K == AR_Deprecated)
>> + if (C->isDeprecated())
>> + return true;
>> +
>> + if (C->isUnavailable())
>> + return true;
>> + return false;
>> + };
>> +
>> + do {
>> + if (CheckContext(Ctx))
>> + return false;
>> +
>> + // An implementation implicitly has the availability of the interface.
>> + if (auto *CatOrImpl = dyn_cast<ObjCImplDecl>(Ctx)) {
>> + if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface())
>> + if (CheckContext(Interface))
>> + return false;
>> + }
>> + // A category implicitly has the availability of the interface.
>> + else if (auto *CatD = dyn_cast<ObjCCategoryDecl>(Ctx))
>> + if (const ObjCInterfaceDecl *Interface = CatD->getClassInterface())
>> + if (CheckContext(Interface))
>> + return false;
>> + } while ((Ctx = cast_or_null<Decl>(Ctx->getDeclContext())));
>> +
>> + return true;
>> +}
>> +
>> static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
>> Decl *Ctx, const NamedDecl *D,
>> StringRef Message, SourceLocation Loc,
>> @@ -6385,11 +6404,15 @@ static void DoEmitAvailabilityWarning(Se
>> // Matches diag::note_availability_specified_here.
>> unsigned available_here_select_kind;
>>
>> - // Don't warn if our current context is deprecated or unavailable.
>> + VersionTuple DeclVersion;
>> + if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, D))
>> + DeclVersion = AA->getIntroduced();
>> +
>> + if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx))
>> + return;
>> +
>> switch (K) {
>> case AR_Deprecated:
>> - if (isDeclDeprecated(Ctx) || isDeclUnavailable(Ctx))
>> - return;
>> diag = !ObjCPropertyAccess ? diag::warn_deprecated
>> : diag::warn_property_method_deprecated;
>> diag_message = diag::warn_deprecated_message;
>> @@ -6399,8 +6422,6 @@ static void DoEmitAvailabilityWarning(Se
>> break;
>>
>> case AR_Unavailable:
>> - if (isDeclUnavailable(Ctx))
>> - return;
>> diag = !ObjCPropertyAccess ? diag::err_unavailable
>> : diag::err_property_method_unavailable;
>> diag_message = diag::err_unavailable_message;
>> @@ -6615,29 +6636,6 @@ void Sema::EmitAvailabilityWarning(Avail
>> ObjCProperty, ObjCPropertyAccess);
>> }
>>
>> -VersionTuple Sema::getVersionForDecl(const Decl *D) const {
>> - assert(D && "Expected a declaration here!");
>> -
>> - VersionTuple DeclVersion;
>> - if (const auto *AA = getAttrForPlatform(getASTContext(), D))
>> - DeclVersion = AA->getIntroduced();
>> -
>> - const ObjCInterfaceDecl *Interface = nullptr;
>> -
>> - if (const auto *MD = dyn_cast<ObjCMethodDecl>(D))
>> - Interface = MD->getClassInterface();
>> - else if (const auto *ID = dyn_cast<ObjCImplementationDecl>(D))
>> - Interface = ID->getClassInterface();
>> -
>> - if (Interface) {
>> - if (const auto *AA = getAttrForPlatform(getASTContext(), Interface))
>> - if (AA->getIntroduced() > DeclVersion)
>> - DeclVersion = AA->getIntroduced();
>> - }
>> -
>> - return std::max(DeclVersion, Context.getTargetInfo().getPlatformMinVersion());
>> -}
>> -
>> namespace {
>>
>> /// \brief This class implements -Wunguarded-availability.
>> @@ -6651,6 +6649,7 @@ class DiagnoseUnguardedAvailability
>> typedef RecursiveASTVisitor<DiagnoseUnguardedAvailability> Base;
>>
>> Sema &SemaRef;
>> + Decl *Ctx;
>>
>> /// Stack of potentially nested 'if (@available(...))'s.
>> SmallVector<VersionTuple, 8> AvailabilityStack;
>> @@ -6658,9 +6657,10 @@ class DiagnoseUnguardedAvailability
>> void DiagnoseDeclAvailability(NamedDecl *D, SourceRange Range);
>>
>> public:
>> - DiagnoseUnguardedAvailability(Sema &SemaRef, VersionTuple BaseVersion)
>> - : SemaRef(SemaRef) {
>> - AvailabilityStack.push_back(BaseVersion);
>> + DiagnoseUnguardedAvailability(Sema &SemaRef, Decl *Ctx)
>> + : SemaRef(SemaRef), Ctx(Ctx) {
>> + AvailabilityStack.push_back(
>> + SemaRef.Context.getTargetInfo().getPlatformMinVersion());
>> }
>>
>> void IssueDiagnostics(Stmt *S) { TraverseStmt(S); }
>> @@ -6693,8 +6693,8 @@ void DiagnoseUnguardedAvailability::Diag
>> NamedDecl *D, SourceRange Range) {
>>
>> VersionTuple ContextVersion = AvailabilityStack.back();
>> - if (AvailabilityResult Result = SemaRef.ShouldDiagnoseAvailabilityOfDecl(
>> - D, ContextVersion, nullptr)) {
>> + if (AvailabilityResult Result =
>> + SemaRef.ShouldDiagnoseAvailabilityOfDecl(D, nullptr)) {
>> // All other diagnostic kinds have already been handled in
>> // DiagnoseAvailabilityOfDecl.
>> if (Result != AR_NotYetIntroduced)
>> @@ -6703,6 +6703,14 @@ void DiagnoseUnguardedAvailability::Diag
>> const AvailabilityAttr *AA = getAttrForPlatform(SemaRef.getASTContext(), D);
>> VersionTuple Introduced = AA->getIntroduced();
>>
>> + if (ContextVersion >= Introduced)
>> + return;
>> +
>> + // If the context of this function is less available than D, we should not
>> + // emit a diagnostic.
>> + if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced, Ctx))
>> + return;
>> +
>> SemaRef.Diag(Range.getBegin(), diag::warn_unguarded_availability)
>> << Range << D
>> << AvailabilityAttr::getPrettyPlatformName(
>> @@ -6777,6 +6785,5 @@ void Sema::DiagnoseUnguardedAvailability
>>
>> assert(Body && "Need a body here!");
>>
>> - VersionTuple BaseVersion = getVersionForDecl(D);
>> - DiagnoseUnguardedAvailability(*this, BaseVersion).IssueDiagnostics(Body);
>> + DiagnoseUnguardedAvailability(*this, D).IssueDiagnostics(Body);
>> }
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=284265&r1=284264&r2=284265&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Oct 14 14:08:01 2016
>> @@ -103,9 +103,9 @@ static bool HasRedeclarationWithoutAvail
>> return false;
>> }
>>
>> -AvailabilityResult Sema::ShouldDiagnoseAvailabilityOfDecl(
>> - NamedDecl *&D, VersionTuple ContextVersion, std::string *Message) {
>> - AvailabilityResult Result = D->getAvailability(Message, ContextVersion);
>> +AvailabilityResult
>> +Sema::ShouldDiagnoseAvailabilityOfDecl(NamedDecl *&D, std::string *Message) {
>> + AvailabilityResult Result = D->getAvailability(Message);
>>
>> // For typedefs, if the typedef declaration appears available look
>> // to the underlying type to see if it is more restrictive.
>> @@ -113,7 +113,7 @@ AvailabilityResult Sema::ShouldDiagnoseA
>> if (Result == AR_Available) {
>> if (const TagType *TT = TD->getUnderlyingType()->getAs<TagType>()) {
>> D = TT->getDecl();
>> - Result = D->getAvailability(Message, ContextVersion);
>> + Result = D->getAvailability(Message);
>> continue;
>> }
>> }
>> @@ -124,7 +124,7 @@ AvailabilityResult Sema::ShouldDiagnoseA
>> if (ObjCInterfaceDecl *IDecl = dyn_cast<ObjCInterfaceDecl>(D)) {
>> if (IDecl->getDefinition()) {
>> D = IDecl->getDefinition();
>> - Result = D->getAvailability(Message, ContextVersion);
>> + Result = D->getAvailability(Message);
>> }
>> }
>>
>> @@ -132,18 +132,10 @@ AvailabilityResult Sema::ShouldDiagnoseA
>> if (Result == AR_Available) {
>> const DeclContext *DC = ECD->getDeclContext();
>> if (const EnumDecl *TheEnumDecl = dyn_cast<EnumDecl>(DC))
>> - Result = TheEnumDecl->getAvailability(Message, ContextVersion);
>> + Result = TheEnumDecl->getAvailability(Message);
>> }
>>
>> - switch (Result) {
>> - case AR_Available:
>> - return Result;
>> -
>> - case AR_Unavailable:
>> - case AR_Deprecated:
>> - return getCurContextAvailability() != Result ? Result : AR_Available;
>> -
>> - case AR_NotYetIntroduced: {
>> + if (Result == AR_NotYetIntroduced) {
>> // Don't do this for enums, they can't be redeclared.
>> if (isa<EnumConstantDecl>(D) || isa<EnumDecl>(D))
>> return AR_Available;
>> @@ -166,23 +158,18 @@ AvailabilityResult Sema::ShouldDiagnoseA
>>
>> return Warn ? AR_NotYetIntroduced : AR_Available;
>> }
>> - }
>> - llvm_unreachable("Unknown availability result!");
>> +
>> + return Result;
>> }
>>
>> static void
>> DiagnoseAvailabilityOfDecl(Sema &S, NamedDecl *D, SourceLocation Loc,
>> const ObjCInterfaceDecl *UnknownObjCClass,
>> bool ObjCPropertyAccess) {
>> - VersionTuple ContextVersion;
>> - if (const DeclContext *DC = S.getCurObjCLexicalContext())
>> - ContextVersion = S.getVersionForDecl(cast<Decl>(DC));
>> -
>> std::string Message;
>> - // See if this declaration is unavailable, deprecated, or partial in the
>> - // current context.
>> + // See if this declaration is unavailable, deprecated, or partial.
>> if (AvailabilityResult Result =
>> - S.ShouldDiagnoseAvailabilityOfDecl(D, ContextVersion, &Message)) {
>> + S.ShouldDiagnoseAvailabilityOfDecl(D, &Message)) {
>>
>> if (Result == AR_NotYetIntroduced && S.getCurFunctionOrMethodDecl()) {
>> S.getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
>> @@ -192,8 +179,7 @@ DiagnoseAvailabilityOfDecl(Sema &S, Name
>> const ObjCPropertyDecl *ObjCPDecl = nullptr;
>> if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
>> if (const ObjCPropertyDecl *PD = MD->findPropertyDecl()) {
>> - AvailabilityResult PDeclResult =
>> - PD->getAvailability(nullptr, ContextVersion);
>> + AvailabilityResult PDeclResult = PD->getAvailability(nullptr);
>> if (PDeclResult == Result)
>> ObjCPDecl = PD;
>> }
>>
>> Modified: cfe/trunk/test/SemaObjC/class-unavail-warning.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/class-unavail-warning.m?rev=284265&r1=284264&r2=284265&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/class-unavail-warning.m (original)
>> +++ cfe/trunk/test/SemaObjC/class-unavail-warning.m Fri Oct 14 14:08:01 2016
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-darwin10 -verify %s
>> +// RUN: %clang_cc1 -fsyntax-only -fblocks -triple x86_64-apple-darwin10 -verify %s
>> // rdar://9092208
>>
>> __attribute__((unavailable("not available")))
>> @@ -98,3 +98,19 @@ UNAVAILABLE
>> @end
>> @interface UnavailSub(cat)<SomeProto> // no error
>> @end
>> +
>> +int unavail_global UNAVAILABLE;
>> +
>> +UNAVAILABLE __attribute__((objc_root_class))
>> + at interface TestAttrContext
>> +-meth;
>> + at end
>> +
>> + at implementation TestAttrContext
>> +-meth {
>> + unavail_global = 2; // no warn
>> + (void) ^{
>> + unavail_global = 4; // no warn
>> + };
>> +}
>> + at end
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161018/84ef142b/attachment-0001.html>
More information about the cfe-commits
mailing list