[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

Tue Aug 31 04:45:51 PDT 2021

Szelethus added a comment.

In D106262#2939532 <https://reviews.llvm.org/D106262#2939532>, @balazske wrote:

> Really I still not understand why the previous `BugType` dependent `NoteTag` functions were bad design (except that it could make the code difficult to understand). If we would have the BugType available in the NoteTag, we could make the decision about what to display and no "or"s are needed in the message. We do not need a "Stream either reaches end-of-file, or fails and has its file position indicator left indeterminate and the error flag set." message if the information is available about what the exact problem was (from the BugType) and we can build a "Stream reaches end-of-file." if the bug was end-of-file related, and so on. (Really instead of the bug type other information could be used that is passed from the bug report to the note tag, but there is no way for this to do?)

My strongest arguments **for** the the notes I suggested is that this would give the complete picture. Suppose I see the note "stream has its error flag set", and fix the FERROR case (add an `ferror()` check with an early return), only to stumble across the very same bug an hour later, but now with an EOF in the note message. I could have added an early return about EOF as well, but I didn't know that function was modeled with EOF set as well. While my suggested note is long, I think this is about the shortest that would give users the complete picture, and I don't think that a longer note is something to avoid (CodeChecker users in particular have to deal with lot longer macro expansions already, and they are still useful IMO).

While I really believe that this would be the better option of the two, I don't strongly insist on it, especially if others see it a subpar option as well.

> Otherwise just the user has to find out the same thing by looking at later functions and notes.

I believe the general advice for users is to read bug reports not from top to bottom, but from the error report up (as its likely that the essence of the bug can be summarized far before we get to the notes around where the analysis started). I think the long note version would be better in that case, it'd more clearly display how the analyzer reached the conclusion.

> So I want to wait for the opinion of another reviewer(s) before proceeding.

Another set of eyes could never hurt, indeed!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106262/new/

https://reviews.llvm.org/D106262