[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yup, this makes sense now! I'll do some nit-picking for a little longer.



================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:839
+    // as a 'StmtPoint' so we have to bypass it.
+    const bool IsBypass = isa<CXXNewExpr>(S);
+
----------------
For now this hack is clearly only for CXXNewExpr. And it happens because our AST is weird and there's no separate sub-expression dedicated entirely to invoking the allocator. Therefore let's give this variable a better name, maybe `BypassCXXNewExprEvaluation` or something like that.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:856-860
+      if (Optional<CallExitBegin> CEB = Node->getLocationAs<CallExitBegin>())
+        CurrentSFC = CEB->getStackFrame();
+
+      if (Optional<CallEnter> CE = Node->getLocationAs<CallEnter>())
+        CurrentSFC = CE->getStackFrame();
----------------
Branches here are unnecessary because there's no performance win here. Let's just
```lang=c++
CurrentSFC = Node->getStackFrame();
```


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:862-866
+      // If that is satisfied we found our statement.
+      if (!IsBypass)
+        if (Optional<StmtPoint> SP = Node->getLocationAs<StmtPoint>())
+          if (SP->getStmt() == S && SP->getStackFrame() == CurrentSFC)
+            break;
----------------
This code would currently bypass the call site for the conservatively evaluated allocator. It doesn't seem to be immediately important but let's add a FIXME.


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

https://reviews.llvm.org/D62926