<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 31, 2014 at 9:12 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On Wed, Jul 30, 2014 at 8:19 PM, 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"><div style="word-wrap:break-word"><div><div><br><div><div>On Jul 30, 2014, at 8:14 , Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr">Hi Jordan,<div><br></div><div>I made some progress on inlining temporary destructors. My main problem currently is that somehow the initializer values in my constructor get lost if I don't also reference 'this' via some other way.</div>
<div><br></div><div>An examples shows that best. Consider:</div><div>struct A {</div><div> A(int* y) : z(y) {}</div><div> ~A() { *z = 23; }</div><div> int *z;</div><div>};</div><div>void f() {</div><div> int x = 0;</div>
<div> {(A(&x));}</div><div>}</div><div><br></div><div>This leads to a "Dereference of undefined pointer value" warning on *z = 23.</div><div>Funnily enough this warning goes away (and all values get correctly propagated) if change the constructor to be:</div>
<div>A(int* y) : z(y) { (void)this; }</div><div><br></div><div>Obviously this also works fine if I use a local variable instead of a temporary.</div><div><br></div><div>Any ideas what might be going wrong here would be greatly appreciated.</div>
</div></blockquote><br></div></div></div><div>My guess is that the temporary is an rvalue, and in the analyzer a NonLoc, and that the only reason this even appears to work is because we needed to be able to call <i>methods</i> on temporaries and so will create a temporary region for them on demand to serve as the 'this' pointer. However, I don't think we expect that to happen for <i>constructors,</i> and so we don't keep the region around after the constructor has finished executing. (That is, when it's just a method being called—and neither the constructor nor destructor are inlined—it doesn't actually matter what you do to the contents of the region, because nothing else can access it.)</div>
<div><br></div><div>I am a little confused as to how your workaround actually changes anything, but regardless there is an issue here.</div><div><br></div><div>To fix this, we may need to treat temporaries as Loc values always, and figure out how to teach the rest of the analyzer to ignore the fact that the Expr says it's an rvalue. Alternately, we could update the compiler to actually treat these as xvalues (which are glvalues), and therefore we'd have a location we could deal with. I vaguely remember talking to Richard about this at one point and him thinking it to be a reasonable idea.</div>
</div></blockquote><div><br></div></div></div><div>C++ itself is very definitely moving in this direction. When you have:</div><div><br></div><div> struct S { S(); int n; };</div><div><br></div><div>the expression S() is a prvalue but S().n is now an xvalue. Clang doesn't implement that yet, but the obvious way to do it is to create</div>
<div><br></div><div> MemberExpr xvalue .n</div><div> `- MaterializeTemporaryExpr xvalue</div><div> `- CXXTemporaryObjectExpr prvalue S()</div><div><br></div><div>... that is, materialize a temporary any time member access is performed. (The same thing happens when array indexing is performed on an array temporary.) This naturally extends to also materializing a temporary when a member function is called on it.</div>
<div><br></div><div>FWIW, this representation would also fix the wrong lifetime extension in a case like:</div><div><br></div><div> int &&r = S().n;</div><div><br></div><div>... where we currently fail to lifetime-extend the S object because we never materialized it.</div>
</div></div></div></blockquote><div><br></div><div>Not all temporaries have MaterializeTemporary though (some just have a CXXBindTemporaryExpr if they die after the full expression). How should we handle that case then?</div>
<div>Also, while we're at it - would it make sense to have the MaterializeTemporaryExpr somehow reference the CXXBindTemoprary it handles (iff Materialize actually materializes a C++ object). I imagine we'd run into problems with conditional operators, though, where the Materialize can reference one of multiple possible constructed objects (the CXXBindTemporary is inside the conditional operator branches).</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><div style="word-wrap:break-word"><div>Or I could be totally off-base on this, because I'm coming up with this from memory rather than actually digging into the program trace.</div>
<span><font color="#888888"><div><br></div><div>Jordan</div></font></span></div><br></div>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div><br></div></div>