[ObjC] Warn on unguarded use of partial declaration
This commit adds a traversal of the AST after Sema of a function that diagnoses
unguarded references to declarations that are partially available (based on
availability attributes). This traversal is only done when we would otherwise
emit -Wpartial-availability.
This commit is part of a feature I proposed here:
http://lists.llvm.org/pipermail/cfe-dev/2016-July/049851.html
Differential revision: https://reviews.llvm.org/D23003
llvm-svn: 278826
diff --git a/clang/lib/Sema/JumpDiagnostics.cpp b/clang/lib/Sema/JumpDiagnostics.cpp
index bdbe06c..0d0c27d 100644
--- a/clang/lib/Sema/JumpDiagnostics.cpp
+++ b/clang/lib/Sema/JumpDiagnostics.cpp
@@ -325,30 +325,27 @@
case Stmt::IfStmtClass: {
IfStmt *IS = cast<IfStmt>(S);
- if (!IS->isConstexpr())
+ if (!(IS->isConstexpr() || IS->isObjCAvailabilityCheck()))
break;
+ unsigned Diag = IS->isConstexpr() ? diag::note_protected_by_constexpr_if
+ : diag::note_protected_by_if_available;
+
if (VarDecl *Var = IS->getConditionVariable())
BuildScopeInformation(Var, ParentScope);
// Cannot jump into the middle of the condition.
unsigned NewParentScope = Scopes.size();
- Scopes.push_back(GotoScope(ParentScope,
- diag::note_protected_by_constexpr_if, 0,
- IS->getLocStart()));
+ Scopes.push_back(GotoScope(ParentScope, Diag, 0, IS->getLocStart()));
BuildScopeInformation(IS->getCond(), NewParentScope);
// Jumps into either arm of an 'if constexpr' are not allowed.
NewParentScope = Scopes.size();
- Scopes.push_back(GotoScope(ParentScope,
- diag::note_protected_by_constexpr_if, 0,
- IS->getLocStart()));
+ Scopes.push_back(GotoScope(ParentScope, Diag, 0, IS->getLocStart()));
BuildScopeInformation(IS->getThen(), NewParentScope);
if (Stmt *Else = IS->getElse()) {
NewParentScope = Scopes.size();
- Scopes.push_back(GotoScope(ParentScope,
- diag::note_protected_by_constexpr_if, 0,
- IS->getLocStart()));
+ Scopes.push_back(GotoScope(ParentScope, Diag, 0, IS->getLocStart()));
BuildScopeInformation(Else, NewParentScope);
}
return;
diff --git a/clang/lib/Sema/ScopeInfo.cpp b/clang/lib/Sema/ScopeInfo.cpp
index 4b2e13e..76240bc 100644
--- a/clang/lib/Sema/ScopeInfo.cpp
+++ b/clang/lib/Sema/ScopeInfo.cpp
@@ -29,6 +29,7 @@
HasIndirectGoto = false;
HasDroppedStmt = false;
HasOMPDeclareReductionCombiner = false;
+ HasPotentialAvailabilityViolations = false;
ObjCShouldCallSuper = false;
ObjCIsDesignatedInit = false;
ObjCWarnForNoDesignatedInitChain = false;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 66a55b2..55b044d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11736,6 +11736,9 @@
return nullptr;
}
+ if (Body && getCurFunction()->HasPotentialAvailabilityViolations)
+ DiagnoseUnguardedAvailabilityViolations(dcl);
+
assert(!getCurFunction()->ObjCShouldCallSuper &&
"This should only be set for ObjC methods, which should have been "
"handled in the block above.");
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index d74cebc..0471f65 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -21,6 +21,7 @@
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Mangle.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
@@ -6330,6 +6331,9 @@
break;
case AR_NotYetIntroduced:
+ assert(!S.getCurFunctionOrMethodDecl() &&
+ "Function-level partial availablity should not be diagnosed here!");
+
diag = diag::warn_partial_availability;
diag_message = diag::warn_partial_message;
diag_fwdclass_message = diag::warn_partial_fwdclass_message;
@@ -6511,3 +6515,146 @@
return std::max(DeclVersion, Context.getTargetInfo().getPlatformMinVersion());
}
+
+namespace {
+
+/// \brief This class implements -Wunguarded-availability.
+///
+/// This is done with a traversal of the AST of a function that makes reference
+/// to a partially available declaration. Whenever we encounter an \c if of the
+/// form: \c if(@available(...)), we use the version from the condition to visit
+/// the then statement.
+class DiagnoseUnguardedAvailability
+ : public RecursiveASTVisitor<DiagnoseUnguardedAvailability> {
+ typedef RecursiveASTVisitor<DiagnoseUnguardedAvailability> Base;
+
+ Sema &SemaRef;
+
+ /// Stack of potentially nested 'if (@available(...))'s.
+ SmallVector<VersionTuple, 8> AvailabilityStack;
+
+ void DiagnoseDeclAvailability(NamedDecl *D, SourceRange Range);
+
+public:
+ DiagnoseUnguardedAvailability(Sema &SemaRef, VersionTuple BaseVersion)
+ : SemaRef(SemaRef) {
+ AvailabilityStack.push_back(BaseVersion);
+ }
+
+ void IssueDiagnostics(Stmt *S) { TraverseStmt(S); }
+
+ bool TraverseIfStmt(IfStmt *If);
+
+ bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
+ if (ObjCMethodDecl *D = Msg->getMethodDecl())
+ DiagnoseDeclAvailability(
+ D, SourceRange(Msg->getSelectorStartLoc(), Msg->getLocEnd()));
+ return true;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ DiagnoseDeclAvailability(DRE->getDecl(),
+ SourceRange(DRE->getLocStart(), DRE->getLocEnd()));
+ return true;
+ }
+
+ bool VisitMemberExpr(MemberExpr *ME) {
+ DiagnoseDeclAvailability(ME->getMemberDecl(),
+ SourceRange(ME->getLocStart(), ME->getLocEnd()));
+ return true;
+ }
+
+ bool VisitTypeLoc(TypeLoc Ty);
+};
+
+void DiagnoseUnguardedAvailability::DiagnoseDeclAvailability(
+ NamedDecl *D, SourceRange Range) {
+
+ VersionTuple ContextVersion = AvailabilityStack.back();
+ if (AvailabilityResult Result = SemaRef.ShouldDiagnoseAvailabilityOfDecl(
+ D, ContextVersion, nullptr)) {
+ // All other diagnostic kinds have already been handled in
+ // DiagnoseAvailabilityOfDecl.
+ if (Result != AR_NotYetIntroduced)
+ return;
+
+ const AvailabilityAttr *AA = getAttrForPlatform(SemaRef.getASTContext(), D);
+ VersionTuple Introduced = AA->getIntroduced();
+
+ SemaRef.Diag(Range.getBegin(), diag::warn_unguarded_availability)
+ << Range << D
+ << AvailabilityAttr::getPrettyPlatformName(
+ SemaRef.getASTContext().getTargetInfo().getPlatformName())
+ << Introduced.getAsString();
+
+ SemaRef.Diag(D->getLocation(), diag::note_availability_specified_here)
+ << D << /* partial */ 3;
+
+ // FIXME: Replace this with a fixit diagnostic.
+ SemaRef.Diag(Range.getBegin(), diag::note_unguarded_available_silence)
+ << Range << D;
+ }
+}
+
+bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
+ const Type *TyPtr = Ty.getTypePtr();
+ SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+
+ if (const TagType *TT = dyn_cast<TagType>(TyPtr)) {
+ TagDecl *TD = TT->getDecl();
+ DiagnoseDeclAvailability(TD, Range);
+
+ } else if (const TypedefType *TD = dyn_cast<TypedefType>(TyPtr)) {
+ TypedefNameDecl *D = TD->getDecl();
+ DiagnoseDeclAvailability(D, Range);
+
+ } else if (const auto *ObjCO = dyn_cast<ObjCObjectType>(TyPtr)) {
+ if (NamedDecl *D = ObjCO->getInterface())
+ DiagnoseDeclAvailability(D, Range);
+ }
+
+ return true;
+}
+
+bool DiagnoseUnguardedAvailability::TraverseIfStmt(IfStmt *If) {
+ VersionTuple CondVersion;
+ if (auto *E = dyn_cast<ObjCAvailabilityCheckExpr>(If->getCond())) {
+ CondVersion = E->getVersion();
+
+ // If we're using the '*' case here or if this check is redundant, then we
+ // use the enclosing version to check both branches.
+ if (CondVersion.empty() || CondVersion <= AvailabilityStack.back())
+ return Base::TraverseStmt(If->getThen()) &&
+ Base::TraverseStmt(If->getElse());
+ } else {
+ // This isn't an availability checking 'if', we can just continue.
+ return Base::TraverseIfStmt(If);
+ }
+
+ AvailabilityStack.push_back(CondVersion);
+ bool ShouldContinue = TraverseStmt(If->getThen());
+ AvailabilityStack.pop_back();
+
+ return ShouldContinue && TraverseStmt(If->getElse());
+}
+
+} // end anonymous namespace
+
+void Sema::DiagnoseUnguardedAvailabilityViolations(Decl *D) {
+ Stmt *Body = nullptr;
+
+ if (auto *FD = D->getAsFunction()) {
+ // FIXME: We only examine the pattern decl for availability violations now,
+ // but we should also examine instantiated templates.
+ if (FD->isTemplateInstantiation())
+ return;
+
+ Body = FD->getBody();
+ } else if (auto *MD = dyn_cast<ObjCMethodDecl>(D))
+ Body = MD->getBody();
+
+ assert(Body && "Need a body here!");
+
+ VersionTuple BaseVersion = getVersionForDecl(D);
+ DiagnoseUnguardedAvailability(*this, BaseVersion).IssueDiagnostics(Body);
+}
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 41c8a4c..4bf17a6 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -184,6 +184,11 @@
if (AvailabilityResult Result =
S.ShouldDiagnoseAvailabilityOfDecl(D, ContextVersion, &Message)) {
+ if (Result == AR_NotYetIntroduced && S.getCurFunctionOrMethodDecl()) {
+ S.getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
+ return;
+ }
+
const ObjCPropertyDecl *ObjCPDecl = nullptr;
if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D)) {
if (const ObjCPropertyDecl *PD = MD->findPropertyDecl()) {
@@ -15198,11 +15203,6 @@
VersionTuple Version;
if (Spec != AvailSpecs.end())
Version = Spec->getVersion();
- else
- // This is the '*' case in @available. We should diagnose this; the
- // programmer should explicitly account for this case if they target this
- // platform.
- Diag(AtLoc, diag::warn_available_using_star_case) << RParen << Platform;
return new (Context)
ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 8e8104e..977d744 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -536,7 +536,7 @@
if (Cond.isInvalid())
return StmtError();
- if (IsConstexpr)
+ if (IsConstexpr || isa<ObjCAvailabilityCheckExpr>(Cond.get().second))
getCurFunction()->setHasBranchProtectedScope();
DiagnoseUnusedExprResult(thenStmt);