balazske added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:419-421
+ const CheckerNameRef CheckerName;
+ SymbolRef StreamSym;
+ const ExplodedNode *NotePosition;
----------------
NoQ wrote:
> I guess it's a matter of preference but I really don't understand the need to
> define a structure when a lambda is sufficient. Lambda is the intended syntax
> sugar for exactly what you have written.
The reason is that the `NoteTag` is added at more than one place. And I do not
want a function that returns a lambda.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:429-435
+ const ExplodedNode *N = BR.getErrorNode();
+ while (N && N != NotePosition) {
+ const StreamState *StreamS = N->getState()->get<StreamMap>(StreamSym);
+ if (!StreamS || !StreamS->ErrorState.FEof)
+ return "";
+ N = N->getFirstPred();
+ }
----------------
NoQ wrote:
> This code says "If there exists a state below in the path where the stream is
> not at EOF, drop the note". The report will not be emitted at all if the
> stream is not at EOF at the end, right? Are you trying to account for the
> situation when the stream gets reset into a non-EOF state and therefore you
> don't need to emit the note above that point?
>
> In such cases the intended solution is to add another note tag to mark the
> stream uninteresting once it gets reset, because all the history before that
> point is irrelevant to our report entirely (whatever the previous position
> was, its get completely overwritten by the reset).
>
> The API for marking objects uninteresting is not yet there but it's suggested
> in D104300. I believe you should rebase your patch on top of it and take
> advantage of it as it lands.
It was unknown to me if the visit of `NoteTag`'s happens from the bug path end
to begin or in different way. If it is from end to begin it may be enough to
remove the interestingness in the current `NoteTag` function when a message is
produced, it should find exactly the last place where that EOF flag is set to
true. And `NotePosition` is not needed.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:429-435
+ const ExplodedNode *N = BR.getErrorNode();
+ while (N && N != NotePosition) {
+ const StreamState *StreamS = N->getState()->get<StreamMap>(StreamSym);
+ if (!StreamS || !StreamS->ErrorState.FEof)
+ return "";
+ N = N->getFirstPred();
+ }
----------------
balazske wrote:
> NoQ wrote:
> > This code says "If there exists a state below in the path where the stream
> > is not at EOF, drop the note". The report will not be emitted at all if the
> > stream is not at EOF at the end, right? Are you trying to account for the
> > situation when the stream gets reset into a non-EOF state and therefore you
> > don't need to emit the note above that point?
> >
> > In such cases the intended solution is to add another note tag to mark the
> > stream uninteresting once it gets reset, because all the history before
> > that point is irrelevant to our report entirely (whatever the previous
> > position was, its get completely overwritten by the reset).
> >
> > The API for marking objects uninteresting is not yet there but it's
> > suggested in D104300. I believe you should rebase your patch on top of it
> > and take advantage of it as it lands.
> It was unknown to me if the visit of `NoteTag`'s happens from the bug path
> end to begin or in different way. If it is from end to begin it may be enough
> to remove the interestingness in the current `NoteTag` function when a
> message is produced, it should find exactly the last place where that EOF
> flag is set to true. And `NotePosition` is not needed.
I tried it out, seems to work. But to continue D104300 should be finished or
the "reset of interestingness" must be split to a separate change (or into this
patch), is this possible?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104925/new/
https://reviews.llvm.org/D104925
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
