NoQ marked an inline comment as done. NoQ added inline comments.
================ Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:491 + ArrayRef<SourceRange> Ranges = None, + ArrayRef<FixItHint> Fixits = None); ---------------- gribozavr wrote: > I'm not sure if this is the right model for fixits. Fixits are usually > associated with a message that explains what the fixit does. Only in the > unusual case where Clang or ClangTidy is very confident that the fixit is > correct, it is attached to the warning. Most commonly, fixits are attached to > notes. > > Also, for IDE support, it would be really nice if we could provide short > descriptions of edits themselves (e.g., "replace 'virtual' with 'override") > that can be displayed to the user instead of the diff when possible -- right > now we don't and tools using ClangTidy have to use a subpar UI because of > that. For example, when we show UI for fixing a warning, displaying the > complete diff is too much; a concise description would be a lot better. > Fixits are usually associated with a message that explains what the fixit > does. Only in the unusual case where Clang or ClangTidy is very confident > that the fixit is correct, it is attached to the warning. Wait, you guys already have fixits attached to notes? Then, yeah, i need to support this. > Also, for IDE support, it would be really nice if we could provide short > descriptions of edits themselves (e.g., "replace 'virtual' with 'override") > that can be displayed to the user instead of the diff when possible -- right > now we don't and tools using ClangTidy have to use a subpar UI because of > that. Mmm, interesting. I've seen this IDE of ours autogenerate such messages as "replace '`$code_in_removed_range`' with '`$inserted_code`'" (while also combining multiple parts of the fixit into a single replacement under the hood) and it looked quite bearable most of the time and i silently assumed that everybody does the same thing. I agree that a high-level description of a fixit is nice to have. But given that you're attaching fixits to notes, isn't the text of the note text itself sufficient? E.g.: - warning: variable may be used uninitialized here - note: initialize 'x' here to suppress the warning - fixit: add ' = 0' after 'x' CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65182/new/ https://reviews.llvm.org/D65182 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
