Page MenuHomePhabricator

[lldb] Also apply Fix-Its in "note:" diagnostics that belong to an error diagnostic
ClosedPublic

Authored by teemperor on Mar 30 2020, 6:31 AM.

Details

Summary

LLDB currently applies Fix-Its if they are attached to a Clang diagnostic that has the
severity "error". Fix-Its connected to warnings and other severities are supposed to
be ignored as LLDB doesn't seem to trust Clang Fix-Its in these situations.

However, LLDB also ignores all Fix-Its coming from "note:" diagnostics. These diagnostics
are usually emitted alongside other diagnostics (both warnings and errors), either to keep
a single diagnostic message shorter or because the Fix-It is in a different source line. As they
are technically their own (non-error) diagnostics, we currently are ignoring all Fix-Its associated with them.

For example, this is a possible Clang diagnostic with a Fix-It that is currently ignored:

error: <user expression 1>:2:10: too many arguments provided to function-like macro invocation
ToStr(0, {,})
         ^
<user expression 1>:1:9: macro 'ToStr' defined here
#define ToStr(x) #x 
        ^
<user expression 1>:2:1: cannot use initializer list at the beginning of a macro argument
ToStr(0, {,})
^        ~~~~

We also don't store "note:" diagnostics at all, as LLDB's abstraction around the whole diagnostic
concept doesn't have such a concept. The text of "note:" diagnostics is instead
appended to the last non-note diagnostic (which is causing that there is no "note:" text in the
diagnostic above, as all the "note:" diagnostics have been appended to the first "error: ..." text).

This patch fixes the ignored Fix-Its in note-diagnostics by appending them to the last non-note
diagnostic, similar to the way we handle the text in these diagnostics.

Diff Detail

Event Timeline

teemperor created this revision.Mar 30 2020, 6:31 AM
teemperor updated this revision to Diff 253862.Mar 31 2020, 6:27 AM
  • Rebase and now only considering error diagnostics.
JDevlieghere added inline comments.Apr 1 2020, 8:52 AM
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
242

This loop and the one 20 lines down seem very similar. Maybe extract a function and share the comments?

teemperor updated this revision to Diff 254498.Apr 2 2020, 6:17 AM
  • Move Fix-It loops into single function.
This revision is now accepted and ready to land.Apr 2 2020, 8:34 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2020, 2:08 AM