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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to