[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:363-366
+    if (&BR.getBugType() != &UseAfterRelease &&
+        &BR.getBugType() != &LeakBugType &&
+        &BR.getBugType() != &DoubleReleaseBugType)
+      return "";
----------------
NoQ wrote:
> Ugh, it sucks that we have to do this by hand.
> 
> Why would the symbol be interesting if the bugtype is wrong? You imagine something like division by a zero handle? Do we really mind the updates to the handle highlighted in this case? I guess we do.
> 
> Maybe we should make "note tag tags" to avoid users writing this by hand. Like, note tags are tagged with note tag tags and the report is also tagged with a note tag tag and the tag is invoked only if the tag tag on the report matches the tag tag on the tag. Setting a tag tag on the report will be equivalent to calling an "addVisitor" on the report in the visitor API, which was actually a good part of the visitor API.
> 
> Just thinking out loud, please ignore. This code is a nice example to generalize from.
I totally agree :) But if we do something like this please do not call the tag tag a tag. It will confuse people. :D


================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:369
+      std::string text = note(BR);
+      if (!text.empty())
+        return text;
----------------
NoQ wrote:
> Another option here is to concatenate the notes if there are indeed two interesting handles at once: `Handle is allocated through 2nd parameter; Handle is released through 3rd parameter". In this checker it probably never happens, as every report is about exactly one handle (is it true for leak reports though?).
I think this is true for leaks as well, as we will generate a new report for each of the symbols.


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

https://reviews.llvm.org/D70725