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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
