Skip to content

Commit

Permalink
[analyzer] AnalyzerOptions: Remove 'fixits-as-remarks'
Browse files Browse the repository at this point in the history
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

(cherry picked from commit abdd33c)
  • Loading branch information
Charusso authored and haoNoQ committed Apr 8, 2020
1 parent 3885ecb commit 18cef5f
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 64 deletions.
4 changes: 0 additions & 4 deletions clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
Expand Up @@ -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)
Expand Down
35 changes: 6 additions & 29 deletions clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
Expand Up @@ -91,7 +91,6 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {

bool IncludePath = false;
bool ShouldEmitAsError = false;
bool FixitsAsRemarks = false;
bool ApplyFixIts = false;

public:
Expand All @@ -109,7 +108,6 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {

void enablePaths() { IncludePath = true; }
void enableWerror() { ShouldEmitAsError = true; }
void enableFixitsAsRemarks() { FixitsAsRemarks = true; }
void enableApplyFixIts() { ApplyFixIts = true; }

void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
Expand All @@ -119,42 +117,24 @@ class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer {
? 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";
}
}
};
Expand Down Expand Up @@ -297,9 +277,6 @@ class AnalysisConsumer : public AnalysisASTConsumer,
if (Opts->AnalyzerWerror)
clangDiags->enableWerror();

if (Opts->ShouldEmitFixItHintsAsRemarks)
clangDiags->enableFixitsAsRemarks();

if (Opts->ShouldApplyFixIts)
clangDiags->enableApplyFixIts();

Expand Down
1 change: 0 additions & 1 deletion clang/test/Analysis/analyzer-config.c
Expand Up @@ -58,7 +58,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
Expand Down
31 changes: 20 additions & 11 deletions clang/test/Analysis/dead-stores.c
@@ -1,31 +1,34 @@
// 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'}}
// non-nested-warning@-1 {{unused variable 'y'}}
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'}}
Expand All @@ -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() {
Expand Down Expand Up @@ -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");
}();
Expand All @@ -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;
}();
}

Expand Down
13 changes: 0 additions & 13 deletions clang/test/Analysis/virtualcall-fixit.cpp

This file was deleted.

15 changes: 9 additions & 6 deletions clang/test/Analysis/virtualcall-fixits.cpp
@@ -1,18 +1,21 @@
// 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 {
virtual void foo();
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();
};
Expand All @@ -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>
Expand Down

0 comments on commit 18cef5f

Please sign in to comment.