[analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'

Summary: The new way of checking fix-its is `%check_analyzer_fixit`.

Reviewed By: NoQ, Szelethus, xazax.hun

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

diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index 400e953e3c6c1bb808e69570969fdf7677c8b0a2..ee67f60df9488fc8eead3f52149b687b0a2fb716 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -306,10 +306,6 @@ ANALYZER_OPTION(bool, ShouldTrackConditionsDebug, "track-conditions-debug",
                 "Whether to place an event at each tracked condition.",
                 false)
 
-ANALYZER_OPTION(bool, ShouldEmitFixItHintsAsRemarks, "fixits-as-remarks",
-                "Emit fix-it hints as remarks for testing purposes",
-                false)
-
 ANALYZER_OPTION(bool, ShouldApplyFixIts, "apply-fixits",
                 "Apply the fix-it hints to the files",
                 false)

diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 1e036dba2de55deeedbf4da64484da15b32509d0..417eb7ec3d311b22eaf8803e11f55da6eef1eea8 100644 (file)
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -92,7 +92,6 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
 
   bool IncludePath = false;
   bool ShouldEmitAsError = false;
-  bool FixitsAsRemarks = false;
   bool ApplyFixIts = false;
 
 public:
@@ -110,7 +109,6 @@ public:
 
   void enablePaths() { IncludePath = true; }
   void enableWerror() { ShouldEmitAsError = true; }
-  void enableFixitsAsRemarks() { FixitsAsRemarks = true; }
   void enableApplyFixIts() { ApplyFixIts = true; }
 
   void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
@@ -120,42 +118,24 @@ public:
             ? Diag.getCustomDiagID(DiagnosticsEngine::Error, "%0")
             : Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0");
     unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0");
-    unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
     SourceManager &SM = Diag.getSourceManager();
 
     Replacements Repls;
     auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String,
                            ArrayRef<SourceRange> Ranges,
                            ArrayRef<FixItHint> Fixits) {
-      if (!FixitsAsRemarks && !ApplyFixIts) {
+      if (!ApplyFixIts) {
         Diag.Report(Loc, ID) << String << Ranges << Fixits;
         return;
       }
 
       Diag.Report(Loc, ID) << String << Ranges;
-      if (FixitsAsRemarks) {
-        for (const FixItHint &Hint : Fixits) {
-          llvm::SmallString<128> Str;
-          llvm::raw_svector_ostream OS(Str);
-          // FIXME: Add support for InsertFromRange and
-          // BeforePreviousInsertion.
-          assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!");
-          assert(!Hint.BeforePreviousInsertions && "Not implemented yet!");
-          OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin()) << "-"
-             << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd()) << ": '"
-             << Hint.CodeToInsert << "'";
-          Diag.Report(Loc, RemarkID) << OS.str();
-        }
-      }
+      for (const FixItHint &Hint : Fixits) {
+        Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert);
 
-      if (ApplyFixIts) {
-        for (const FixItHint &Hint : Fixits) {
-          Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert);
-
-          if (llvm::Error Err = Repls.add(Repl)) {
-            llvm::errs() << "Error applying replacement " << Repl.toString()
-                         << ": " << Err << "\n";
-          }
+        if (llvm::Error Err = Repls.add(Repl)) {
+          llvm::errs() << "Error applying replacement " << Repl.toString()
+                       << ": " << Err << "\n";
         }
       }
     };
@@ -298,9 +278,6 @@ public:
       if (Opts->AnalyzerWerror)
         clangDiags->enableWerror();
 
-      if (Opts->ShouldEmitFixItHintsAsRemarks)
-        clangDiags->enableFixitsAsRemarks();
-
       if (Opts->ShouldApplyFixIts)
         clangDiags->enableApplyFixIts();
 

diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 2e4af7109125cbfc60299c8eb384869474200eba..068930e6e57f76278e788bee8a5763a66bdeea8d 100644 (file)
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -59,7 +59,6 @@
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
-// CHECK-NEXT: fixits-as-remarks = false
 // CHECK-NEXT: graph-trim-interval = 1000
 // CHECK-NEXT: inline-lambdas = true
 // CHECK-NEXT: ipa = dynamic-bifurcate
@@ -101,4 +100,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 98
+// CHECK-NEXT: num-entries = 97

diff --git a/clang/test/Analysis/dead-stores.c b/clang/test/Analysis/dead-stores.c
index cbfdabdcec632bde64ee36b65a3a843a3b6452a9..a17e1692496da03d3299588d3fa5fd8bbd06a4b1 100644 (file)
--- a/clang/test/Analysis/dead-stores.c
+++ b/clang/test/Analysis/dead-stores.c
@@ -1,16 +1,16 @@
-// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
+// RUN: %check_analyzer_fixit %s %t \
+// RUN:   -Wunused-variable -fblocks -Wno-unreachable-code \
 // RUN:   -analyzer-checker=core,deadcode.DeadStores \
 // RUN:   -analyzer-config deadcode.DeadStores:ShowFixIts=true \
-// RUN:   -analyzer-config fixits-as-remarks=true \
 // RUN:   -analyzer-config \
 // RUN:       deadcode.DeadStores:WarnForDeadNestedAssignments=false \
-// RUN:   -verify=non-nested %s
+// RUN:   -verify=non-nested
 
-// RUN: %clang_analyze_cc1 -Wunused-variable -fblocks -Wno-unreachable-code \
+// RUN: %check_analyzer_fixit %s %t \
+// RUN:   -Wunused-variable -fblocks -Wno-unreachable-code \
 // RUN:   -analyzer-checker=core,deadcode.DeadStores \
 // RUN:   -analyzer-config deadcode.DeadStores:ShowFixIts=true \
-// RUN:   -analyzer-config fixits-as-remarks=true \
-// RUN:   -verify=non-nested,nested %s
+// RUN:   -verify=non-nested,nested
 
 void f1() {
   int k, y; // non-nested-warning {{unused variable 'k'}}
@@ -18,14 +18,17 @@ void f1() {
   int abc = 1;
   long idx = abc + 3 * 5; // non-nested-warning {{never read}}
                           // non-nested-warning@-1 {{unused variable 'idx'}}
-                          // non-nested-remark@-2 {{11-24: ''}}
+  // CHECK-FIXES:      int abc = 1;
+  // CHECK-FIXES-NEXT: long idx;
 }
 
 void f2(void *b) {
   char *c = (char *)b; // no-warning
   char *d = b + 1;     // non-nested-warning {{never read}}
                        // non-nested-warning@-1 {{unused variable 'd'}}
-                       // non-nested-remark@-2 {{10-17: ''}}
+  // CHECK-FIXES:      char *c = (char *)b;
+  // CHECK-FIXES-NEXT: char *d;
+
   printf("%s", c);
   // non-nested-warning@-1 {{implicitly declaring library function 'printf' with type 'int (const char *, ...)'}}
   // non-nested-note@-2 {{include the header <stdio.h> or explicitly provide a declaration for 'printf'}}
@@ -51,7 +54,8 @@ void f5() {
   int x = 4;   // no-warning
   int *p = &x; // non-nested-warning {{never read}}
                // non-nested-warning@-1 {{unused variable 'p'}}
-               // non-nested-remark@-2 {{9-13: ''}}
+  // CHECK-FIXES:      int x = 4;
+  // CHECK-FIXES-NEXT: int *p;
 }
 
 int f6() {
@@ -415,7 +419,8 @@ void f23_pos(int argc, char **argv) {
   int shouldLog = (argc > 1);
   // non-nested-warning@-1 {{Value stored to 'shouldLog' during its initialization is never read}}
   // non-nested-warning@-2 {{unused variable 'shouldLog'}}
-  // non-nested-remark@-3 {{16-28: ''}}
+  // CHECK-FIXES:      void f23_pos(int argc, char **argv) {
+  // CHECK-FIXES-NEXT:   int shouldLog;
   ^{
     f23_aux("I did too use it!\n");
   }();
@@ -428,7 +433,11 @@ void f24_A(int y) {
     int z = x + y;
     // non-nested-warning@-1 {{Value stored to 'z' during its initialization is never read}}
     // non-nested-warning@-2 {{unused variable 'z'}}
-    // non-nested-remark@-3 {{10-17: ''}}
+    // CHECK-FIXES:      void f24_A(int y) {
+    // CHECK-FIXES-NEXT:   //
+    // CHECK-FIXES-NEXT:   int x = (y > 2);
+    // CHECK-FIXES-NEXT:   ^{
+    // CHECK-FIXES-NEXT:     int z;
   }();
 }
 

diff --git a/clang/test/Analysis/virtualcall-fixit.cpp b/clang/test/Analysis/virtualcall-fixit.cpp
deleted file mode 100644 (file)
index e64e63a..0000000
--- a/clang/test/Analysis/virtualcall-fixit.cpp
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %check_analyzer_fixit %s %t \
-// RUN:   -analyzer-checker=core,optin.cplusplus.VirtualCall \
-// RUN:   -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true
-
-struct S {
-  virtual void foo();
-  S() {
-    foo();
-    // expected-warning@-1 {{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
-    // CHECK-FIXES: S::foo();
-  }
-  ~S();
-};

diff --git a/clang/test/Analysis/virtualcall-fixits.cpp b/clang/test/Analysis/virtualcall-fixits.cpp
index ea149d52fcd21a60e4b627283daf36f4ac53f513..b68fcbfea93d5bd57c6a5df45a70f1f4808922d6 100644 (file)
--- a/clang/test/Analysis/virtualcall-fixits.cpp
+++ b/clang/test/Analysis/virtualcall-fixits.cpp
@@ -1,10 +1,11 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
 // RUN:     -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
 // RUN:     %s 2>&1 | FileCheck -check-prefix=TEXT %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,optin.cplusplus.VirtualCall \
+
+// RUN: %check_analyzer_fixit %s %t \
+// RUN:     -analyzer-checker=core,optin.cplusplus.VirtualCall \
 // RUN:     -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true \
-// RUN:     -analyzer-config fixits-as-remarks=true \
-// RUN:     -analyzer-output=plist -o %t.plist -verify %s
+// RUN:     -analyzer-output=plist -o %t.plist
 // RUN: cat %t.plist | FileCheck -check-prefix=PLIST %s
 
 struct S {
@@ -12,7 +13,9 @@ struct S {
   S() {
     foo();
     // expected-warning@-1{{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}}
-    // expected-remark@-2{{5-5: 'S::'}}
+    // CHECK-FIXES:      S() {
+    // CHECK-FIXES-NEXT:   S::foo();
+    // CHECK-FIXES-NEXT: }
   }
   ~S();
 };
@@ -30,12 +33,12 @@ struct S {
 // PLIST-NEXT:    <key>remove_range</key>
 // PLIST-NEXT:    <array>
 // PLIST-NEXT:     <dict>
-// PLIST-NEXT:      <key>line</key><integer>13</integer>
+// PLIST-NEXT:      <key>line</key><integer>14</integer>
 // PLIST-NEXT:      <key>col</key><integer>5</integer>
 // PLIST-NEXT:      <key>file</key><integer>0</integer>
 // PLIST-NEXT:     </dict>
 // PLIST-NEXT:     <dict>
-// PLIST-NEXT:      <key>line</key><integer>13</integer>
+// PLIST-NEXT:      <key>line</key><integer>14</integer>
 // PLIST-NEXT:      <key>col</key><integer>4</integer>
 // PLIST-NEXT:      <key>file</key><integer>0</integer>
 // PLIST-NEXT:     </dict>