Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.
LGTM, aside from some checker tagging nightmare. Its a bit easy to mess up,
please allow me to have a final look before commiting! :)
================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:298
+def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
+ HelpText<"Check constraints of arguments of C standard library functions, "
----------------
Just noticed, this checker still lies in the `apiModeling` package. Could we
find a more appropriate place?
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:92-93
+ class ValueConstraint;
+ using ValueConstraintPtr = std::shared_ptr<ValueConstraint>;
class ValueConstraint {
----------------
martong wrote:
> Szelethus wrote:
> > How about `ValueConstraintRef`?
> Yeah, we have `ProgramStateRef` and `SymbolRef`. And both are actually just
> synonyms to smart pointers. I'd rather not call a pointer as a reference,
> because that can be confusing when reading the code. E.g. when I see that we
> return with a `nullptr` from a function that can return with a `...Ref` I
> start to scratch my head.
Sure, I'm sold.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:260
+ BugType BT{this, "Unsatisfied argument constraints", categories::LogicError};
+
----------------
By passing `this`, the error message will be tied to the modeling checker, not
to the one you just added. `BugType` has a constructor that accepts a string
instead, pass `CheckNames[CK_StdCLibraryFunctionArgsChecker]` in there :) Also,
how about `BT_InvalidArgument` or something?
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
