[PATCH] D82256: [analyzer] Enable constructor support in evalCall event

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Thanks! I'll commit.



================
Comment at: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:669
       ProgramPoint L = ProgramPoint::getProgramPoint(
-          cast<CallExpr>(Call.getOriginExpr()),
+          Call.getOriginExpr(),
           ProgramPoint::PostStmtKind,
----------------
vrnithinkumar wrote:
> This is for fixing the unit test failures. 
> Previously `assert` to check the type inside `cast<CallExpr>`  was causing the unit test failures.
> The reason was `CXXConstructExpr` was not inherited from `CallExpr` and the cast was causing the assert failure with `isa` check.
> 
> I am not sure removing the cast is the best solution.
> And the TODO comment, I did not understood properly.
> 
> Alternative approach was to cast it to `CXXConstructExpr` if it is not `CallExpr` but not sure whether `runCheckersForEvalCall` should aware of  `CXXConstructExpr`.
Wait, was it that easy?

Yeah, looks like the cast is indeed unnecessary. Somebody probably meant something when they put it here but they didn't bless us with a comment and i don't see any immediate reasons why would this code require a `CallExpr` specifically.

The TODO above is about supporting the situation when there is no expression *at all*. Like, `CXXConstructExpr` is still *something*, but in some cases the call isn't a result of evaluating any expression at all. One of the most common examples of such calls is destructor calls. Say, automatic destructors of local variables get triggered at the end of scope rather than at an expression. We'll have to get back to it when we `evalCall()` a destructor :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82256/new/

https://reviews.llvm.org/D82256