NoQ added a comment.
First of all, thanks everybody for working on this. I'd see what i can do.
================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:29
namespace {
+enum class ObjectState : bool { CtorCalled, DtorCalled };
+} // end namespace
----------------
Please remind me what was wrong with ascending over `StackFrameContext`s to see
if we're inside a constructor or destructor call? Why did we need a state trait
here? Was it too hard to obtain this-values? (Not sure if there's time to fix
this, but it might be worth it to leave a FIXME around).
================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:56
+ CheckerContext &C) const;
+ void reportbug(StringRef Msg, bool PureError, const MemRegion *Reg,
+ CheckerContext &C) const;
----------------
I wish for camelCase.
================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:110
+ auto ThiSVal =
+ State->getSVal(SVB.getCXXThis(MD, LCtx->getCurrentStackFrame()));
+ const MemRegion *Reg = ThiSVal.getAsRegion();
----------------
I don't think this is working. CXXThisRegion is never a region of a C++ object;
it's the segment of the stack where the pointer is passed, you need to
dereference its value to get the actual object region.
Probably tests for the visitor might make things more clear.
================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:194
+ if (IsPureOnly && MD->isPure())
+ reportbug("Call to pure function during construction", true, Reg, C);
+ else
----------------
"Pure function" doesn't mean what we want to say here
(https://en.wikipedia.org/wiki/Pure_function), i think we should stick with
"pure virtual function" anyways.
================
Comment at: test/Analysis/virtualcall.cpp:15
foo();
-#if INTERPROCEDURAL
- // expected-warning-re@-2 {{{{^}}Call Path : foo <-- fCall to pure
virtual function during construction has undefined behavior}}
-#endif
+ // expected-warning:Call to virtual function during construction
}
----------------
Tests need fixing: `expected-warning` requires `{{` and needs to be on the
correct line.
https://reviews.llvm.org/D34275
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
