NoQ added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:731 + } + return C.getNoteTag([Text, Name](BugReport &BR) -> std::string { + SmallString<256> Msg; ---------------- Szelethus wrote: > NoQ wrote: > > baloghadamsoftware wrote: > > > NoQ wrote: > > > > You'll need to check whether the container is actually of interest to > > > > the bug report. We don't want notes to be added about changes to > > > > irrelevant containers. > > > > > > > > You can use a combination of "Report `BR` was emitted by one of the > > > > iterator checkers" and "The memory region of the container is marked as > > > > interesting" (while also actually marking it as interesting in the > > > > checker). > > > > > > > > Ideally we should instead make a new generic storage inside the > > > > `BugReport` object, in order to pass down the interesting information > > > > from the call site of `emitReport` ("Hi, i'm an iterator checker who > > > > emitted this report and i'm interested in changes made to the size of > > > > this container"). > > > Are you sure in this? I already wondered how it works so I added a test > > > that checks one container and changes another one and there were no note > > > tags displayed for the one we did not check but change. See the last test. > > That's because you didn't do > > ```lang=c++ > > V2.cbegin(); > > V2.cend(); > > ``` > > in the beginning. > A similar conversation sparked up recently in between @boga95, @steakhal and > me regarding reporting taintedness. Bug reports are fine up to the point > where (in reverse) the first propagation happens, but finding out which value > tainted the one that caused the report isn't handled at the moment. One idea > was to mark the initial (again, in reverse) value as interesting, create a > `NoteTag` at the point of propagation, where we should know which value was > the cause of the spread, mark that interesting as well, etc. > > If `NoteTag`s only emit a message when the concerning value is interesting, > this should theoretically solve that problem. I guess you could say that > we're propagating interestingness in reverse. > > I'm not immediately sure if this idea was ever mentioned or implemented here. Yes, that's the intended solution to such problems. `trackExpressionValue` works similarly, just with assignments instead of taint propagations. And in both cases note tags are a much more straightforward solution to the problem. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73720/new/ https://reviews.llvm.org/D73720 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits