<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Aug 9, 2014 at 3:16 AM, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Do we really need to put the "binds parameter" information in the CFG? I was hoping we could look in the parent map to see how the temporary is used (like for 'new' right now).<br>
</blockquote><div><br></div><div>Ah, I didn't know there was a parent map already. I'll look into it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
================<br>
Comment at: include/clang/Analysis/CFG.h:285<br>
@@ -285,1 +284,3 @@<br>
+  CFGTemporaryDtor(CXXBindTemporaryExpr *expr, bool BindsParameter)<br>
+      : CFGImplicitDtor(TemporaryDtor, expr, BindsParameter ? T : nullptr) {}<br>
<br>
----------------<br>
Why not just use "self"? Or &T? Why "T"? Why does T require a dynamic allocation?<br></blockquote><div><br></div><div>Because I hoped you would say something like the above :) and I didn't find a good way to add a bool (the PointerIntPair Data2 already uses the "int" part for the kind, and it doesn't work with pointers to static constants (throws a runtime error in PointerInPair for incorrectly aligned pointer)).</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:688-693<br>
@@ +687,8 @@<br>
+  const MemRegion *Region = nullptr;<br>
+  // If the class does not have any members, there will not be a region<br>
+  // for it bound in the environment.<br>
+  if (Optional<nonloc::LazyCompoundVal> LCV =<br>
+          Val.getAs<nonloc::LazyCompoundVal>()) {<br>
+    Region = LCV->getRegion();<br>
+  }<br>
+<br>
----------------<br>
This is not how LCVs work. An LCV is a snapshot of a region at a particular "time" (state of the Store); the region may not exist in the current store. The "best" answer in the current scheme is to use createTemporaryRegionIfNeeded, but the right answer may end up being to disregard the rvalue-ness of temporaries (as we talked about some on cfe-dev).<br>
</blockquote><div><br></div><div>Can you give me test cases that fail so I can work against them?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: lib/StaticAnalyzer/Core/ProgramState.cpp:508-509<br>
@@ -507,13 +507,4 @@<br>
<br>
 bool ScanReachableSymbols::scan(nonloc::LazyCompoundVal val) {<br>
-  bool wasVisited = !visited.insert(val.getCVData()).second;<br>
-  if (wasVisited)<br>
-    return true;<br>
-<br>
-  StoreManager &StoreMgr = state->getStateManager().getStoreManager();<br>
-  // FIXME: We don't really want to use getBaseRegion() here because pointer<br>
-  // arithmetic doesn't apply, but scanReachableSymbols only accepts base<br>
-  // regions right now.<br>
-  const MemRegion *R = val.getRegion()->getBaseRegion();<br>
-  return StoreMgr.scanReachableSymbols(val.getStore(), R, *this);<br>
+  return scan(val.getRegion());<br>
 }<br>
----------------<br>
This is not correct; see above.</blockquote><div><br></div><div>Can you explain that in more detail - especially for the scan part? What does this break? </div></div><br></div></div>