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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
