[PATCH] D65182: [analyzer] Add fix-it hint support.

This revision was automatically updated to reflect the committed changes. Closed by commit rL371257: [analyzer] Add minimal support for fix-it hints. (authored by dergachev, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://r

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491 + ArrayRef Ranges = None, + ArrayRef Fixits = None); gribozavr wrote:

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:343 + /// to produce a good fix-it hint for most path-sensitive warnings. + void addFixItHint(const FixItHint &F) { +Fixits.push_b

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr accepted this revision. gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:343 + /// to produce a good fix-it hint for most path-sensitive warnings. + void addFixItHint(const FixItHint &F) { +Fixits.push

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491 + ArrayRef Ranges = None, + ArrayRef Fixits = None); NoQ wrote: > gribozavr wrote: > > I'm not sure i

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ updated this revision to Diff 218575. NoQ added a comment. Make fix-it hints attachable to any `PathDiagnosticPiece`, rather than to `PathDiagnostic` as a whole. This basically allows attaching a fixit to any note in the report. For now the plist consumer only supports attaching fixits to t

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/test/Analysis/dead-stores.c:11 long idx=abc+3*5; // expected-warning {{never read}} expected-warning{{unused variable 'idx'}} + // expected-remark-re@-1.*}}:11 - {{.*}}:18 - ''}} } --

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491 + ArrayRef Ranges = None, + ArrayRef Fixits = None); gribozavr wrote:

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/test/Analysis/dead-stores.c:11 long idx=abc+3*5; // expected-warning {{never read}} expected-warning{{unused variable 'idx'}} + // expected-remark-re@-1.*}}:11 - {{.*}}:18 - ''}} } NoQ wrote: > gribozav

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:350 + + llvm::ArrayRef getFixits() const { return Fixits; } + No need to qualify with "llvm::". Comment at: clang/include/clang/St

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:751 +o << "insert_string\n"; +o << "" << Hint.CodeToInsert << "\n"; +o << " \n"; gribozavr wrote: > Escaping? Oh #@&%! Thanks. ==

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ updated this revision to Diff 217982. NoQ marked 8 inline comments as done. NoQ added a comment. Fixits ♫ Fix the Fixits ♫ Yabba dabba doo ♫ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 Files: clang/include/clang/StaticAnalyzer/Checkers

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ marked an inline comment as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:347 + + virtual llvm::ArrayRef getFixits() const { return Fixits; } + gribozavr wrote: > Why is it virtual? In fact, w

[PATCH] D65182: [analyzer] Add fix-it hint support.

gribozavr added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:347 + + virtual llvm::ArrayRef getFixits() const { return Fixits; } + Why is it virtual? In fact, why is BugReporter subclassable at all? ==

[PATCH] D65182: [analyzer] Add fix-it hint support.

a_sidorin added a comment. Hi Artem! The patch looks very promising for CSA, thanks for working on this! Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:343 + /// to produce a good fix-it hint for most path-sensitive warnings. + void addFixItHin

[PATCH] D65182: [analyzer] Add fix-it hint support.

Szelethus accepted this revision. Szelethus added a comment. In D65182#1630540 , @NoQ wrote: > In D65182#1629192 , @Szelethus wrote: > > > Hmm, why the need for checker options? Why not have them by default? If > >

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ added a comment. In D65182#1629192 , @Szelethus wrote: > Hmm, why the need for checker options? Why not have them by default? If > fixits are an experimental feature, maybe we should have a global > `enable-fixits` config. But I don't insist :) I d

[PATCH] D65182: [analyzer] Add fix-it hint support.

Szelethus added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:879 + ArrayRef getFixits() const { return Fixits; } + NoQ wrote: > Szelethus wrote: > > Hmm, will this return an immutable container? If not, can

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ updated this revision to Diff 215236. NoQ added a comment. Add a "not implemented yet" assertion for kinds of fixits that are not supported as of that patch (but will definitely need to be supported in the future). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://r

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ marked 3 inline comments as done. NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:879 + ArrayRef getFixits() const { return Fixits; } + Szelethus wrote: > Hmm, will this return an immutable conta

[PATCH] D65182: [analyzer] Add fix-it hint support.

Szelethus accepted this revision. Szelethus added subscribers: gribozavr, aaron.ballman, alexfh. Szelethus added a comment. Hmm, why the need for checker options? Why not have them by default? If fixits are an experimental feature, maybe we should have a global `enable-fixits` config maybe. But

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ updated this revision to Diff 215011. NoQ added a comment. Rebase! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

[PATCH] D65182: [analyzer] Add fix-it hint support.

Charusso accepted this revision. Charusso added inline comments. Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:103 void enableWerror() { ShouldEmitAsError = true; } + void enableFixitsAsRemarks() { FixitsAsRemarks = true; } NoQ wrote: >

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:561 +CmdLineOption `Show` -> `Enable`? > **recursion**, //n.,// see "recursion" I think it's valuable when the object and the documentation describe the same idea in //different//

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ updated this revision to Diff 214495. NoQ marked 3 inline comments as done. NoQ added a comment. Fix un-singed options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 Files: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td clang/

[PATCH] D65182: [analyzer] Add fix-it hint support.

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I really like that patch, as no human would rewrite 600 `dyn_cast` and `getAs` to `cast` and `castAs`. Thanks you! Comment at: clang/include/clang/StaticAnalyzer/Checker

[PATCH] D65182: [analyzer] Add fix-it hint support.

NoQ updated this revision to Diff 214462. NoQ retitled this revision from "[analyzer] WIP: Add fix-it hint support." to "[analyzer] Add fix-it hint support.". NoQ added a comment. - Hide all current fixits behind a checker option, have them off by default. They're not tested enough yet. - Suppre