[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong marked 4 inline comments as done.
martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:836-837
+          NewState, NewNode,
+          C.getNoteTag([Msg](PathSensitiveBugReport &BR,
+                             llvm::raw_ostream &OS) { OS << Msg; }));
     }
----------------
NoQ wrote:
> martong wrote:
> > steakhal wrote:
> > > This way each and every applied constraint will be displayed even if the given argument does not constitute to the bug condition.
> > > I recommend you branching within the lambda, on the interestingness of the given argument constraint.
> > Okay, good point, thanks for the feedback! I am planning to change to this direction.
> Excellent catch @steakhal!
> 
> I think you can always emit the note but only mark it as //unprunable// when the argument is interesting. This way it'd work identically to our normal "Assuming..." notes.
> I think you can always emit the note but only mark it as unprunable when the argument is interesting. This way it'd work identically to our normal "Assuming..." notes.

`IsPrunable` is a `const` member in `NoteTag`. So, we have to decide about prunability when we call `getNoteTag`. To follow your suggestion, we should decide the prunability dynamically in `TagVisitor::VisitNode`. This would require some infrastructural changes in `NoteTag`. We could add e.g. another Callback member that would be able to decide the prunability with the help of a `BugReport&`. I am okay to go into that direction, but it should definitely be separated from this patch (follow-up). I am not sure if  it is an absolutely needed dependency for this change, is it? (If yes then I am going to create the dependent patch first).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101526