zaks.anna added a comment.
Looks great overall. I have minor comments below. Thanks for the awesome
comments!!!
================
Comment at: lib/StaticAnalyzer/Checkers/GTestChecker.cpp:152
+
+ ProgramStateRef State = C.getState();
+
----------------
This could be moved up as you are using the state on the previous line.
================
Comment at: lib/StaticAnalyzer/Checkers/GTestChecker.cpp:159
+
+ State = assumeValuesEqual(ThisSuccess, BooleanArgVal, State, C);
+ C.addTransition(State);
----------------
What happens when they are known not to be equal? I am guessing the transition
is just not added, correct? Do you test for that (I did not check.)?
================
Comment at: lib/StaticAnalyzer/Checkers/GTestChecker.cpp:172
+ const CXXConstructorCall *Call, CheckerContext &C) const {
+ assert(Call->getNumArgs() > 0);
+
----------------
"getNumArgs() == 1" ?
================
Comment at: lib/StaticAnalyzer/Checkers/GTestChecker.cpp:209
+
+ if (CtorDecl->getNumParams() == 0)
+ return;
----------------
It seems that the num of parameter checking logic here is less restrictive than
it could be and that makes this a bit hard to read without looking into the
'model' methods. Ex: I think there are 2 cases:
- Constructor taking a bool can have either 1 or 2 arguments.
- Copy constructor taking exactly one argument.
https://reviews.llvm.org/D27773
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
