Szelethus marked an inline comment as done.
Szelethus added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:361
+ while (!SCtx->inTopFrame()) {
+ auto p = FramesModifying.insert(SCtx);
+ if (!p.second)
----------------
martong wrote:
> Why don't we add the stack frame here to `FramesModifyingCalculated` as well?
> If we are in a nested stack frame and we step back to a parent then it would
> be redundant to do the calculation again for the parent.
>
> Ahh, to be honest, I think the idea of having both `FramesModifying` and
> `FramesModifyingCalculated` separated is prone to errors.
> Why don't we have a simple map<StackFramContext, bool> with the boolean true
> meaning that the frame is modifying? And if the frame is not in the map that
> means it had never been calculated before.
Yes, its stupid, but its just the one thing I didn't want to bother myself
with. Though I agree it'd be better to drop these sets for something a lot more
elegant. Btw mind that we insert into the calculated set each time we enter a
new stack frame.
I'll leave a TODO, if you don't mind.
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:369
+static const ExplodedNode *getCallExitEnd(const ExplodedNode *N) {
+ while (N && !N->getLocationAs<CallExitEnd>())
+ N = N->getFirstSucc();
----------------
NoQ wrote:
> Wait, no, I don't think this test is sufficient. You may run into a
> `CallExitEnd` of a nested stack frame before you reach the `CallExitEnd` of
> the stack frame in question:
> ```
> -> CallEnter foo() --->
> -> CallEnter bar() |
> <- CallExitEnd bar() <--- ???
> <- CallExitEnd foo()
> ```
Good point. Added a bunch of asserts and changed to code to both retrieve, and
ensure that the correct node is retrieved.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108695/new/
https://reviews.llvm.org/D108695
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
