arphaman updated this revision to Diff 96538.
arphaman added a comment.
Now the patch takes the following situations into account:
- Enclose only the statement in a `case`.
- If the fixit has to enclose a declaration statement, then the fixit will try
to enclose the appropriate uses as well.
Repository:
rL LLVM
https://reviews.llvm.org/D32424
Files:
include/clang/Lex/Lexer.h
lib/Lex/Lexer.cpp
lib/Sema/SemaDeclAttr.cpp
test/FixIt/fixit-availability.c
test/FixIt/fixit-availability.mm
Index: test/FixIt/fixit-availability.mm
===================================================================
--- /dev/null
+++ test/FixIt/fixit-availability.mm
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunguarded-availability -fdiagnostics-parseable-fixits -triple x86_64-apple-darwin9 %s 2>&1 | FileCheck %s
+
+__attribute__((availability(macos, introduced=10.12)))
+int function(void);
+
+void anotherFunction(int function);
+
+int use() {
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:14-[[@LINE-2]]:14}:"\n } else {\n // Fallback on earlier versions\n }"
+ int y = function(), x = 0;
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:29-[[@LINE-2]]:29}:"\n } else {\n // Fallback on earlier versions\n }"
+ x += function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:19-[[@LINE-2]]:19}:"\n } else {\n // Fallback on earlier versions\n }"
+ if (1) {
+ x = function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:20-[[@LINE-2]]:20}:"\n } else {\n // Fallback on earlier versions\n }"
+ }
+ anotherFunction(function());
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:31-[[@LINE-2]]:31}:"\n } else {\n // Fallback on earlier versions\n }"
+ if (function()) {
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE+1]]:4-[[@LINE+1]]:4}:"\n } else {\n // Fallback on earlier versions\n }"
+ }
+ while (function())
+ // CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+ // CHECK-NEXT: fix-it:{{.*}}:{[[@LINE+1]]:6-[[@LINE+1]]:6}:"\n } else {\n // Fallback on earlier versions\n }"
+ ;
+ do
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n } else {\n // Fallback on earlier versions\n }"
+ while (1);
+ for (int i = 0; i < 10; ++i)
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n } else {\n // Fallback on earlier versions\n }"
+ switch (x) {
+ case 0:
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n } else {\n // Fallback on earlier versions\n }"
+ case 2:
+ anotherFunction(1);
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n } else {\n // Fallback on earlier versions\n }"
+ break;
+ default:
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n } else {\n // Fallback on earlier versions\n }"
+ break;
+ }
+ return function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:21-[[@LINE-2]]:21}:"\n } else {\n // Fallback on earlier versions\n }"
+}
+
+#define MYFUNCTION function
+
+#define MACRO_ARGUMENT(X) X
+#define MACRO_ARGUMENT_SEMI(X) X;
+#define MACRO_ARGUMENT_2(X) if (1) X;
+
+#define INNER_MACRO if (1) MACRO_ARGUMENT(function()); else ;
+
+void useInMacros() {
+ MYFUNCTION();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:16-[[@LINE-2]]:16}:"\n } else {\n // Fallback on earlier versions\n }"
+
+ MACRO_ARGUMENT_SEMI(function())
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:34-[[@LINE-2]]:34}:"\n } else {\n // Fallback on earlier versions\n }"
+ MACRO_ARGUMENT(function());
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:30-[[@LINE-2]]:30}:"\n } else {\n // Fallback on earlier versions\n }"
+ MACRO_ARGUMENT_2(function());
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:32-[[@LINE-2]]:32}:"\n } else {\n // Fallback on earlier versions\n }"
+
+ INNER_MACRO
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:14-[[@LINE-2]]:14}:"\n } else {\n // Fallback on earlier versions\n }"
+}
+
+void wrapDeclStmtUses() {
+ int x = 0, y = function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE+13]]:22-[[@LINE+13]]:22}:"\n } else {\n // Fallback on earlier versions\n }"
+ {
+ int z = function();
+ if (z) {
+
+ }
+// CHECK: fix-it:{{.*}}:{[[@LINE-4]]:5-[[@LINE-4]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:6-[[@LINE-2]]:6}:"\n } else {\n // Fallback on earlier versions\n }"
+ }
+ if (y)
+ int z = function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:5-[[@LINE-1]]:5}:"if (@available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:24-[[@LINE-2]]:24}:"\n } else {\n // Fallback on earlier versions\n }"
+ anotherFunction(y);
+ anotherFunction(x);
+}
Index: test/FixIt/fixit-availability.c
===================================================================
--- /dev/null
+++ test/FixIt/fixit-availability.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunguarded-availability -fdiagnostics-parseable-fixits -triple x86_64-apple-darwin9 %s 2>&1 | FileCheck %s
+
+__attribute__((availability(macos, introduced=10.12)))
+int function(void);
+
+int use() {
+ function();
+// CHECK: fix-it:{{.*}}:{[[@LINE-1]]:3-[[@LINE-1]]:3}:"if (__builtin_available(macos 10.12, *)) {\n "
+// CHECK-NEXT: fix-it:{{.*}}:{[[@LINE-2]]:14-[[@LINE-2]]:14}:"\n } else {\n // Fallback on earlier versions\n }"
+}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7076,6 +7076,83 @@
namespace {
+/// Returns true if the given statement can be a body-like child of \p Parent.
+bool isBodyLikeChildStmt(const Stmt *S, const Stmt *Parent) {
+ switch (Parent->getStmtClass()) {
+ case Stmt::IfStmtClass:
+ return cast<IfStmt>(Parent)->getThen() == S ||
+ cast<IfStmt>(Parent)->getElse() == S;
+ case Stmt::WhileStmtClass:
+ return cast<WhileStmt>(Parent)->getBody() == S;
+ case Stmt::DoStmtClass:
+ return cast<DoStmt>(Parent)->getBody() == S;
+ case Stmt::ForStmtClass:
+ return cast<ForStmt>(Parent)->getBody() == S;
+ case Stmt::CXXForRangeStmtClass:
+ return cast<CXXForRangeStmt>(Parent)->getBody() == S;
+ case Stmt::ObjCForCollectionStmtClass:
+ return cast<ObjCForCollectionStmt>(Parent)->getBody() == S;
+ case Stmt::CaseStmtClass:
+ case Stmt::DefaultStmtClass:
+ return cast<SwitchCase>(Parent)->getSubStmt() == S;
+ default:
+ return false;
+ }
+}
+
+class StmtUSEFinder : public RecursiveASTVisitor<StmtUSEFinder> {
+ const Stmt *Target;
+
+public:
+ bool VisitStmt(Stmt *S) { return S != Target; }
+
+ /// Returns true if the given statement is present in the given declaration.
+ static bool isContained(const Stmt *Target, const Decl *D) {
+ StmtUSEFinder Visitor;
+ Visitor.Target = Target;
+ return !Visitor.TraverseDecl(const_cast<Decl *>(D));
+ }
+};
+
+/// Traverses the AST and finds the last statement that used a given
+/// declaration.
+class LastDeclUSEFinder : public RecursiveASTVisitor<LastDeclUSEFinder> {
+ const Decl *D;
+ const CompoundStmt *Scope;
+ const Stmt *FirstNonScopeStmt = nullptr;
+ const Stmt *LastMatchingDREFirstNonScopeStmt = nullptr;
+
+public:
+ bool TraverseStmt(Stmt *S) {
+ if (!S)
+ return true;
+ bool ClearStmtStack = false;
+ if (S != Scope && !FirstNonScopeStmt) {
+ FirstNonScopeStmt = S;
+ ClearStmtStack = true;
+ }
+ bool Result = RecursiveASTVisitor::TraverseStmt(S);
+ if (ClearStmtStack)
+ FirstNonScopeStmt = nullptr;
+ return Result;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ if (DRE->getDecl() == D)
+ LastMatchingDREFirstNonScopeStmt = FirstNonScopeStmt;
+ return true;
+ }
+
+ static const Stmt *findLastStmtThatUsesDecl(const Decl *D,
+ const CompoundStmt *Scope) {
+ LastDeclUSEFinder Visitor;
+ Visitor.D = D;
+ Visitor.Scope = Scope;
+ Visitor.TraverseStmt(const_cast<CompoundStmt *>(Scope));
+ return Visitor.LastMatchingDREFirstNonScopeStmt;
+ }
+};
+
/// \brief This class implements -Wunguarded-availability.
///
/// This is done with a traversal of the AST of a function that makes reference
@@ -7091,6 +7168,7 @@
/// Stack of potentially nested 'if (@available(...))'s.
SmallVector<VersionTuple, 8> AvailabilityStack;
+ SmallVector<const Stmt *, 16> StmtStack;
void DiagnoseDeclAvailability(NamedDecl *D, SourceRange Range);
@@ -7101,6 +7179,15 @@
SemaRef.Context.getTargetInfo().getPlatformMinVersion());
}
+ bool TraverseStmt(Stmt *S) {
+ if (!S)
+ return true;
+ StmtStack.push_back(S);
+ bool Result = Base::TraverseStmt(S);
+ StmtStack.pop_back();
+ return Result;
+ }
+
void IssueDiagnostics(Stmt *S) { TraverseStmt(S); }
bool TraverseIfStmt(IfStmt *If);
@@ -7158,9 +7245,71 @@
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)
+ auto FixitDiag =
+ SemaRef.Diag(Range.getBegin(), diag::note_unguarded_available_silence)
<< Range << D;
+
+ // Find the statement which should be enclosed in the if @available check.
+ if (StmtStack.empty())
+ return;
+ const Stmt *StmtOfUse = StmtStack.back();
+ const CompoundStmt *Scope = nullptr;
+ for (const Stmt *S : llvm::reverse(StmtStack)) {
+ if (const auto *CS = dyn_cast<CompoundStmt>(S)) {
+ Scope = CS;
+ break;
+ }
+ if (isBodyLikeChildStmt(StmtOfUse, S)) {
+ // The declaration won't be seen outside of the statement, so we don't
+ // have to wrap the uses of any declared variables in if (@available).
+ // Therefore we can avoid setting Scope here.
+ break;
+ }
+ StmtOfUse = S;
+ }
+ const Stmt *LastStmtOfUse = nullptr;
+ if (isa<DeclStmt>(StmtOfUse) && Scope) {
+ for (const Decl *D : cast<DeclStmt>(StmtOfUse)->decls()) {
+ if (StmtUSEFinder::isContained(StmtStack.back(), D)) {
+ LastStmtOfUse = LastDeclUSEFinder::findLastStmtThatUsesDecl(D, Scope);
+ break;
+ }
+ }
+ }
+
+ const SourceManager &SM = SemaRef.getSourceManager();
+ SourceLocation IfInsertionLoc =
+ SM.getExpansionLoc(StmtOfUse->getLocStart());
+ SourceLocation StmtEndLoc =
+ SM.getExpansionRange(
+ (LastStmtOfUse ? LastStmtOfUse : StmtOfUse)->getLocEnd())
+ .second;
+ if (SM.getFileID(IfInsertionLoc) != SM.getFileID(StmtEndLoc))
+ return;
+
+ StringRef Indentation = Lexer::getIndentationForLine(IfInsertionLoc, SM);
+ const char *ExtraIndentation = " ";
+ std::string FixItString;
+ llvm::raw_string_ostream FixItOS(FixItString);
+ FixItOS << "if (" << (SemaRef.getLangOpts().ObjC1 ? "@available"
+ : "__builtin_available")
+ << "(" << SemaRef.getASTContext().getTargetInfo().getPlatformName()
+ << " " << Introduced.getAsString() << ", *)) {\n"
+ << Indentation << ExtraIndentation;
+ FixitDiag << FixItHint::CreateInsertion(IfInsertionLoc, FixItOS.str());
+ SourceLocation ElseInsertionLoc = Lexer::findLocationAfterToken(
+ StmtEndLoc, tok::semi, SM, SemaRef.getLangOpts(),
+ /*SkipTrailingWhitespaceAndNewLine=*/false);
+ if (ElseInsertionLoc.isInvalid())
+ ElseInsertionLoc =
+ Lexer::getLocForEndOfToken(StmtEndLoc, 0, SM, SemaRef.getLangOpts());
+ FixItOS.str().clear();
+ FixItOS << "\n"
+ << Indentation << "} else {\n"
+ << Indentation << ExtraIndentation
+ << "// Fallback on earlier versions\n"
+ << Indentation << "}";
+ FixitDiag << FixItHint::CreateInsertion(ElseInsertionLoc, FixItOS.str());
}
}
Index: lib/Lex/Lexer.cpp
===================================================================
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -452,6 +452,29 @@
return false;
}
+/// Returns the pointer that points to the beginning of line that contains
+/// the given offset, or null if the offset if invalid.
+static const char *findBeginningOfLine(StringRef Buffer, unsigned Offset) {
+ const char *BufStart = Buffer.data();
+ if (Offset >= Buffer.size())
+ return nullptr;
+ const char *StrData = BufStart + Offset;
+
+ if (StrData[0] == '\n' || StrData[0] == '\r')
+ return StrData;
+
+ const char *LexStart = StrData;
+ while (LexStart != BufStart) {
+ if (LexStart[0] == '\n' || LexStart[0] == '\r') {
+ ++LexStart;
+ break;
+ }
+
+ --LexStart;
+ }
+ return LexStart;
+}
+
static SourceLocation getBeginningOfFileToken(SourceLocation Loc,
const SourceManager &SM,
const LangOptions &LangOpts) {
@@ -467,27 +490,15 @@
// Back up from the current location until we hit the beginning of a line
// (or the buffer). We'll relex from that point.
- const char *BufStart = Buffer.data();
- if (LocInfo.second >= Buffer.size())
- return Loc;
-
- const char *StrData = BufStart+LocInfo.second;
- if (StrData[0] == '\n' || StrData[0] == '\r')
+ const char *StrData = Buffer.data() + LocInfo.second;
+ const char *LexStart = findBeginningOfLine(Buffer, LocInfo.second);
+ if (!LexStart || LexStart == StrData)
return Loc;
-
- const char *LexStart = StrData;
- while (LexStart != BufStart) {
- if (LexStart[0] == '\n' || LexStart[0] == '\r') {
- ++LexStart;
- break;
- }
-
- --LexStart;
- }
// Create a lexer starting at the beginning of this token.
SourceLocation LexerStartLoc = Loc.getLocWithOffset(-LocInfo.second);
- Lexer TheLexer(LexerStartLoc, LangOpts, BufStart, LexStart, Buffer.end());
+ Lexer TheLexer(LexerStartLoc, LangOpts, Buffer.data(), LexStart,
+ Buffer.end());
TheLexer.SetCommentRetentionState(true);
// Lex tokens until we find the token that contains the source location.
@@ -1038,6 +1049,27 @@
return isIdentifierBody(c, LangOpts.DollarIdents);
}
+StringRef Lexer::getIndentationForLine(SourceLocation Loc,
+ const SourceManager &SM) {
+ if (Loc.isInvalid() || Loc.isMacroID())
+ return "";
+ std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
+ if (LocInfo.first.isInvalid())
+ return "";
+ bool Invalid = false;
+ StringRef Buffer = SM.getBufferData(LocInfo.first, &Invalid);
+ if (Invalid)
+ return "";
+ const char *Line = findBeginningOfLine(Buffer, LocInfo.second);
+ if (!Line)
+ return "";
+ StringRef Rest = Buffer.substr(Line - Buffer.data());
+ size_t NumWhitespaceChars = Rest.find_first_not_of(" \t");
+ return NumWhitespaceChars == StringRef::npos
+ ? ""
+ : Rest.take_front(NumWhitespaceChars);
+}
+
//===----------------------------------------------------------------------===//
// Diagnostics forwarding code.
//===----------------------------------------------------------------------===//
Index: include/clang/Lex/Lexer.h
===================================================================
--- include/clang/Lex/Lexer.h
+++ include/clang/Lex/Lexer.h
@@ -478,6 +478,11 @@
return getCharAndSizeSlowNoWarn(Ptr, Size, LangOpts);
}
+ /// Returns the leading whitespace for line that corresponds to the given
+ /// location \p Loc.
+ static StringRef getIndentationForLine(SourceLocation Loc,
+ const SourceManager &SM);
+
//===--------------------------------------------------------------------===//
// Internal implementation interfaces.
private:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
