Add a fix-it for -Wunguarded-availability
authorAlex Lorenz <arphaman@gmail.com>
Fri, 5 May 2017 16:42:44 +0000 (16:42 +0000)
committerAlex Lorenz <arphaman@gmail.com>
Fri, 5 May 2017 16:42:44 +0000 (16:42 +0000)
This patch adds a fix-it for the -Wunguarded-availability warning. This fix-it
is similar to the Swift one: it suggests that you wrap the statement in an
`if (@available)` check. The produced fixits are indented (just like the Swift
ones) to make them look nice in Xcode's fix-it preview.

rdar://31680358

Differential Revision: https://reviews.llvm.org/D32424

llvm-svn: 302253

clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Lex/Lexer.h
clang/lib/Lex/Lexer.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/FixIt/fixit-availability.c [new file with mode: 0644]
clang/test/FixIt/fixit-availability.mm [new file with mode: 0644]
clang/test/Sema/attr-availability.c

index 444dd97..733d792 100644 (file)
@@ -2868,7 +2868,8 @@ def warn_partial_availability : Warning<"%0 is only available conditionally">,
 def note_partial_availability_silence : Note<
   "explicitly redeclare %0 to silence this warning">;
 def note_unguarded_available_silence : Note<
-  "enclose %0 in an @available check to silence this warning">;
+  "enclose %0 in %select{an @available|a __builtin_available}1 check to silence"
+  " this warning">;
 def warn_partial_message : Warning<"%0 is partial: %1">,
     InGroup<UnguardedAvailability>, DefaultIgnore;
 def warn_partial_fwdclass_message : Warning<
index 6ac6316..3be7331 100644 (file)
@@ -478,6 +478,11 @@ public:
     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:
index 003c9b5..3d6fe91 100644 (file)
@@ -452,6 +452,29 @@ bool Lexer::getRawToken(SourceLocation Loc, Token &Result,
   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 @@ static SourceLocation getBeginningOfFileToken(SourceLocation Loc,
 
   // 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 @@ bool Lexer::isIdentifierBodyChar(char c, const LangOptions &LangOpts) {
   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 9dac93a..9cbfcd1 100644 (file)
@@ -7132,6 +7132,69 @@ void Sema::EmitAvailabilityWarning(AvailabilityResult AR,
 
 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;
+
+public:
+  bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+    if (DRE->getDecl() == D)
+      return false;
+    return true;
+  }
+
+  static const Stmt *findLastStmtThatUsesDecl(const Decl *D,
+                                              const CompoundStmt *Scope) {
+    LastDeclUSEFinder Visitor;
+    Visitor.D = D;
+    for (auto I = Scope->body_rbegin(), E = Scope->body_rend(); I != E; ++I) {
+      const Stmt *S = *I;
+      if (!Visitor.TraverseStmt(const_cast<Stmt *>(S)))
+        return S;
+    }
+    return nullptr;
+  }
+};
+
 /// \brief This class implements -Wunguarded-availability.
 ///
 /// This is done with a traversal of the AST of a function that makes reference
@@ -7147,6 +7210,7 @@ class DiagnoseUnguardedAvailability
 
   /// Stack of potentially nested 'if (@available(...))'s.
   SmallVector<VersionTuple, 8> AvailabilityStack;
+  SmallVector<const Stmt *, 16> StmtStack;
 
   void DiagnoseDeclAvailability(NamedDecl *D, SourceRange Range);
 
@@ -7157,6 +7221,15 @@ public:
         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);
@@ -7214,9 +7287,73 @@ void DiagnoseUnguardedAvailability::DiagnoseDeclAvailability(
     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;
+    auto FixitDiag =
+        SemaRef.Diag(Range.getBegin(), diag::note_unguarded_available_silence)
+        << Range << D
+        << (SemaRef.getLangOpts().ObjC1 ? /*@available*/ 0
+                                        : /*__builtin_available*/ 1);
+
+    // 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());
   }
 }
 
diff --git a/clang/test/FixIt/fixit-availability.c b/clang/test/FixIt/fixit-availability.c
new file mode 100644 (file)
index 0000000..fa641b4
--- /dev/null
@@ -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);
+
+void 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  }"
+}
diff --git a/clang/test/FixIt/fixit-availability.mm b/clang/test/FixIt/fixit-availability.mm
new file mode 100644 (file)
index 0000000..6bf8f49
--- /dev/null
@@ -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 c4133e3..e105037 100644 (file)
@@ -30,7 +30,7 @@ void test_10095131() {
   ATSFontGetPostScriptName(100); // expected-error {{'ATSFontGetPostScriptName' is unavailable: obsoleted in macOS 9.0 - use ATSFontGetFullPostScriptName}}
 
 #if defined(WARN_PARTIAL)
-  // expected-warning@+2 {{is only available on macOS 10.8 or newer}} expected-note@+2 {{enclose 'PartiallyAvailable' in an @available check to silence this warning}}
+  // expected-warning@+2 {{is only available on macOS 10.8 or newer}} expected-note@+2 {{enclose 'PartiallyAvailable' in a __builtin_available check to silence this warning}}
 #endif
   PartiallyAvailable();
 }