martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:418
+ if (FailureSt && !SuccessSt) {
+ C.addTransition(FailureSt);
+ if (ExplodedNode *N = C.generateErrorNode(FailureSt))
----------------
balazske wrote:
> Is this `addTransition` needed? It would be OK to call `generateErrorNode`
> with `State`. Even if not, adding the transition before should not be needed?
Yes, you are right it is superfluous, I removed it.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:688
// locale-specific return values.
.Case({ArgumentCondition(0U, WithinRange, {{128,
UCharMax}})})
.Case({ArgumentCondition(0U, OutOfRange,
----------------
balazske wrote:
> Why is this `{128, UCharMax}` here and at the next entry needed?
This is the local specific range , [128, 255]. There are characters like `ä`
which we don't know if they are treated as an alphanumerical character or not.
We can't really tell how a specific libc implementation classifies them. On the
other hand, with English letters we can state the classes confidently.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:696
+ .ArgConstraint(ArgumentCondition(
+ 0U, WithinRange, {{EOFv, EOFv}, {0, UCharMax}}))},
},
----------------
balazske wrote:
> Is this `ArgConstraint` intentionally added only to `isalnum`?
>
Yes, I wanted to create first the infrastructure and then later to add all
these constraints to the rest of the summaries with new tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73898/new/
https://reviews.llvm.org/D73898
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
