Szelethus added a comment.
Just littering some more inlines, don't mind me :) Lets still wait on the
dependency patch before updating.
================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:296
+def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
+ HelpText<"Check constraints of arguments of C standard library functions">,
+ Dependencies<[StdCLibraryFunctionsChecker]>,
----------------
martong wrote:
> Szelethus wrote:
> > How about we add an example as well?
> You mean like NonNull or other constraints?
Like
```
Check constraints of arguments of C standard library functions, such as whether
the parameter of isalpha is in the range [0, 255] or is EOF.
```
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:161
+
+ ValueRange negate() const {
+ ValueRange tmp(*this);
----------------
martong wrote:
> Szelethus wrote:
> > Maybe `complement` would be a better name? That sounds a lot more like a
> > set operation. Also, this function highlights well that inheritance might
> > not be the best solution here.
> Well, we check the argument constraint validity by trying to apply it's
> logical negation. In case of a range inclusion this is being out of that
> range. In case of non-null this is being null. And so on. The logic how we
> try to check an argument constraint is the same in all cases of the different
> constraints. And that is the point: in order to support a new kind of
> constraint we just have to figure out how to "apply" and "negate" one
> constraint. In my opinion this is a perfect case for polimorphism.
We agreed on inheritance in the previous patch, and regarding the name, sure,
leave it as-is. :)
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:92-93
+ class ValueConstraint;
+ using ValueConstraintPtr = std::shared_ptr<ValueConstraint>;
class ValueConstraint {
----------------
How about `ValueConstraintRef`?
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
