Szelethus added a comment. Please run this on open source projects and upload the results.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:164-166 + /// This function is called when the current constraint represents the + /// opposite of a constraint that was not satisfied in a given state, but + /// the opposite is satisfied. In this case the available information in the ---------------- I know what you mean, but this could use an example. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:168-170 + /// description about the original constraint violation. This can be get by + /// try to narrow the current constraint while it remains satisfied in the + /// given program state. If useful information is found it is put into ---------------- This sentence makes so sense, unfortunately :( Could you rephrase, and, again, support it with an example? ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:803 + if (ValuesPrinted) + MsgOs << " that is out of the accepted range; It should be "; + else ---------------- Looking at the tests, this adds very little how about this: " that is out of the accepted range; It should be " -> ", but should be " Do you agree? I won't die on this hill, and am willing to accept this is good as-is if you really think that it is. The other case is fine. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:807 + VC->describe(Call, C.getState(), Summary, MsgOs); + // NegatedVC->describeBug1(Call, N->getState(), Summary, MsgOs); Msg[0] = toupper(Msg[0]); ---------------- Did you mean to leave this here? ================ Comment at: clang/test/Analysis/std-c-library-functions-arg-constraints-notes.cpp:18 __not_null(nullptr); // \ - // expected-warning{{The 1st argument to '__not_null' should not be NULL}} + // expected-warning{{The 1st argument to '__not_null' is NULL that is out of the accepted range; It should be not NULL [}} } ---------------- This english is broken, but more importantly, this is the one scenario where the original message was just good enough -- in fact, better. How about either 1. Restoring the original message (by somehow excluding `NotNullConstraint` in the new `describe()`) 2. Or saying something like "The 1st argument to '__not_null' is NULL, but should be non-NULL" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144003/new/ https://reviews.llvm.org/D144003 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits