steakhal added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:688-703 std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe( - ProgramStateRef State, const Summary &Summary) const { + DescritptionKind DK, ProgramStateRef State, const Summary &Summary) const { SmallString<96> Result; - Result += "The size of the "; + const auto Violation = ValueConstraint::DescritptionKind::Violation; + Result += "the size of the "; Result += getArgDesc(ArgN); + Result += DK == Violation ? " should be " : " is "; ---------------- I don't know. Report message construction always seemed clunky. Clang's or ClangTidy's approach seems superior in this regard. Do we have anything better for this @NoQ? Maybe `llvm::format()` could be an option. Regarding this patch: It's fine. Better than it was before! ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:854 + NewState, NewNode, + C.getNoteTag([Msg, ArgSVal](PathSensitiveBugReport &BR, + llvm::raw_ostream &OS) { ---------------- ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:857 + if (BR.isInteresting(ArgSVal)) + OS << Msg; + })); ---------------- Ah, there is a slight issue. You should mark some stuff interesting here, to make this interestingness propagate back transitively. Let's say `ArgSVal` is `x + y` which is considered to be out of range `[42,52]`. We should mark both `x` and `y` interesting because they themselves could have been constrained by the StdLibChecker previously. So, they must be interesting as well. On the same token, IMO `PathSensitiveBugReport::markInteresting(symbol)` should be //transitive//. So that all `SymbolData` in that symbolic expression tree are considered interesting. What do you think @NoQ? If we were doing this, @martong - you could simply acquire the assumption you constructed for the given `ValueConstraint`, and mark that interesting. Then all `SymbolData`s on both sides of the logic operator would become implicitly interesting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101526/new/ https://reviews.llvm.org/D101526 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits