Bug report warning message duplicated as a note by most checkers
BugReport introduces two separate data members called
ShortDescription and nicely documents their purpose:
Descriptionis “A verbose warning message that is appropriate for displaying next to the source code that introduces the problem.”
ShortDescriptionis “A short general warning message that is appropriate for displaying in the list of all reported bugs. It should describe what kind of bug is found but does not need to try to go into details of that specific bug.”
However, in practice these two data members almost always contain the same string in classes like
PathSensitiveBugReport that derive from
BugReport . The only exception is
BitwiseShiftChecker, where I introduced separate full and short descriptions a few months ago (because I trusted the comments in
BugReport and didn’t realize that this distinction doesn’t appear anywhere else).
In the analyzer output the
ShortDescription appears as the warning message, while
Description is printed as a note at the same location, so currently practically every checker produces a message that’s printed twice (once as a note, once as a warning).
The repetition is not misleading, but I think that this a high priority issue, because it is very visible and completely pointless and has an obvious solution, so it creates an impression that the Static Analyzer is an unfinished, “dirty” project.
I think there are three potential decisions at this point:
- Just eliminate all the redundant note messages and remove the distinction between the two Description fields. This is very straightforward, although it would change lots of lines in the test files.
- Gradually separate short and full messages in each checker. This is a very long project with 100+ small commits, but could eventually lead to better messages.
- Hybrid solution: Don’t print the note message when it just repeats the warning, but let the checkers provide a separate longer
Descriptionif they want (and gradually introduce these full descriptions when we’re improving checkers).
I would be happy to implement either the simple elimination, or the hybrid solution (or more precisely its first phase that eliminates the repetition). I don’t think that the gradual separation is an adequate solution, as separately editing all the checkers would be a very slow process with limited benefits (and this issue would persist until its completion).
I understand that eliminating all those useless note messages would be a breaking change in the output of the Clang Static Analyzer (for example, bugreport hashes that include the note messages would change and I’d guess that there could be frontends that don’t display the warning because they except that its content was already displayed as a note).
I think that despite these potential difficulties this issue must be fixed, but I would be very careful when I release these changes. (E.g. if needed, I could introduce a compatibility flag that can reactivate the legacy behavior for users that depend on it.)
@NoQ @Xazax-hun @steakhal @Szelethus @gamesh411 @ anybody else
What do you think about this issue? What kind of solution would you prefer? What do you know about the potential impact of this change, what should I do to ensure that it’s acceptable for the users?
By the way, this question is relevant for me because I’m trying to “clean up” alpha checkers (currently ArrayBoundV2) and when I work on improving the diagnostics, I need to decide whether I want to separate the short and full descriptions. As a concrete example, a few days ago I uploaded PR #70056 for review and that change would separate the short and full description in ArrayBoundV2 (among other improvements). Resolving this issue would help me move forward with that process.
I couldn’t find any earlier discussion about this topic; I’m sorry if I’m duplicating some old ticket.
 I checked all calls to
make_unique<BasicBugReport>() with the help of