[PATCH] D43666: [analyzer] When constructing a temporary without construction context, track it for destruction anyway.

Devin Coughlin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Feb 24 10:48:21 PST 2018


dcoughlin added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2234
+  // should go away, but the assertion should remain, without the
+  // "DidCacheOutOnCleanup" part, of course.
+  bool DidCacheOutOnCleanup = false;
----------------
Can you add a comment explaining the criteria under which the DidCacheOutOnCleanup part can be removed?


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2257
+      } else {
+        Pred = *CleanUpTemporaries.begin();
+      }
----------------
I realize this isn't new code in this patch, but can you use the result of Bldr.generateNode() to determine whether we cached out and also to get the new Pred? That seems cleaner than querying the ExplodedNodeSet.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2262
+  assert(DidCacheOutOnCleanup ||
+         areInitializedTemporariesClear(Pred->getState(),
                                         Pred->getLocationContext(),
----------------
It sounds like this means if we did cache out then there are places where the initialized temporaries are not cleared (that is, we have extra gunk in the state that we don't want).

Is that true? If so, then this relaxation of the assertion doesn't seem right to me.

Do we need to introduce a new program point when calling `Bldr.generateNode` on the cleaned up state (for example, with a new tag or a new program point kind)? This would make it so that when we cache out when generating a node for the state with the cleaned up temporaries we know that it is safe to early return from processEndOfFunction(). It would be safe because we would know that the other node (the one we cached out because of) has already had its temporaries cleared and notified the checkers about the end of the function, etc.







Repository:
  rC Clang

https://reviews.llvm.org/D43666





More information about the cfe-commits mailing list