alexfh added a comment.
This looks like a more promising direction. Thanks for the readiness to
experiment with this.
See the comments inline.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:130
+
+ // If we have alternative fixes (attached via diagnostic::Notes), we just
+ // choose the first one to apply.
----------------
Could you leave a FIXME here to explore options around interactive fix
selection?
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:132
+ // choose the first one to apply.
+ const llvm::StringMap<Replacements> *ErrorFix = nullptr;
+ if (!Error.Message.Fix.empty())
----------------
`ErrorFix` brings more questions than it answers. Maybe `SelectedFix`,
`ChosenFix`, or just `Fix`?
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:133
+ const llvm::StringMap<Replacements> *ErrorFix = nullptr;
+ if (!Error.Message.Fix.empty())
+ ErrorFix = &Error.Message.Fix;
----------------
nit: I'd add braces here, since the `else` branch has them.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:144
+
+ if (ApplyFixes && ErrorFix) {
+ for (const auto &FileAndReplacements : *ErrorFix) {
----------------
The nesting level starts getting out of control here. I'd try to pull the loop
into a function.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:187
}
+ reportFix(Diag, Error.Message.Fix);
}
----------------
Why `Error.Message.Fix`? Should this use `*SelectedFix` instead?
================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:262
+ for (const auto &Repl : FileAndReplacements.second) {
+ if (Repl.isApplicable()) {
+ SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
----------------
nit: Early "continue" would make sense here.
if (!Repl.isApplicable())
continue;
...
================
Comment at: clang/include/clang/Tooling/Core/Diagnostic.h:46
+
+ /// Fixes for this diagnostic.
+ llvm::StringMap<Replacements> Fix;
----------------
Some information from the original comment was lost here:
/// Fixes to apply, grouped by file path.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59932/new/
https://reviews.llvm.org/D59932
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
