[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Thank you! I think we can start to run this check on real world code bases and evaluate the results.



================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:41
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void ChangeMaps(bool IsBeginFunction, CheckerContext &C) const;
+  void ReportBug(const char *Msg, bool PureError, const MemRegion *Reg,
----------------
Function names should start with a lower case letter. These two could be private. I'd prefer another name for this function like `registerCtorDtorCallInState`.


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:42
+  void ChangeMaps(bool IsBeginFunction, CheckerContext &C) const;
+  void ReportBug(const char *Msg, bool PureError, const MemRegion *Reg,
+                 CheckerContext &C) const;
----------------
Use `StringRef` instead of `const char *`.  It is more idiomatic in LLVM code. 


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:179
+    } else {
+      const char *Msg = "Call to virtual function during construction";
+      ReportBug(Msg, false, Reg, C);
----------------
I'd omit the `Msg` local variable. After that, we do not need to use braces for single statement blocks.


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:210
+    const MemRegion *Reg = ThiSVal.getAsRegion();
+    if (IsBeginFunction) {
+      State = State->set<CtorMap>(Reg, true);
----------------
Braces for single statement blocks. 


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:224
+    const MemRegion *Reg = ThiSVal.getAsRegion();
+    if (IsBeginFunction) {
+      State = State->set<CtorMap>(Reg, true);
----------------
Braces for single statement blocks. 


================
Comment at: lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp:238
+  ExplodedNode *N;
+  if (PureError) {
+    N = C.generateErrorNode();
----------------
Do not use braces for single statement blocks. 


https://reviews.llvm.org/D34275