<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 1, 2015 at 11:28 AM, Meador Inge <span dir="ltr"><<a href="mailto:meadori@gmail.com" target="_blank">meadori@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Opening up in **constexpr** s in conditions allows "maybe" odr used expression to be collected in cases like:<br>
<br>
  class T {<br>
  public:<br>
     constexpr T(int v) : v(v) { }<br>
     constexpr operator int() const { return v; }<br>
  private:<br>
     int v;<br>
  };<br>
<br>
  int main() {<br>
     if (constexpr T x = 200) { }<br>
  }<br>
<br>
Without the cleanup the following assert fires:<br>
<br>
  (MaybeODRUseExprs.empty() && "Leftover expressions for odr-use checking"), function ActOnFinishFunctionBody, file /Users/meadori/Code/source/llvm/tools/clang/lib/Sema/SemaDecl.cpp, line 10820.<br>
<br>
**MaybeODRUseExprs** is populated in **DoMarkVarDeclReferenced** from **SemaExpr.cpp** (the end of the function does an explicit check for constant expressions).  I don't think this situation could ever happen for conditional declarations before,</blockquote><div><br></div><div>OK, I think you're right. The interesting thing here is that a condition variable seems to involve two separate full-expressions: one for the initializer and one for the reference to the condition variable (and -- except for 'switch' -- for the contextual conversion to bool). We are missing the ActOnFinishFullExpr call for the second full-expression.</div><div><br></div><div>Now, it just so happens that nothing we do in ActOnFinishFullExpr for the condition expression is observable except for resolving potential odr-uses, and without constexpr conditions, every potential use is a real odr-use, so has already been handled.</div><div><br></div><div>I think the right thing to do here is to pass the expression produced by loading and possibly converting the condition variable to ActOnFinishFullExpr, rather than calling MaybeCreateExprWithCleanups.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> but know it can and we need the new cleanup.<br>
<span class=""><br>
<br>
================<br>
Comment at: include/clang/Parse/Parser.h:1760<br>
@@ -1760,1 +1759,3 @@<br>
</span><span class="">+                                   DeclSpecContext DSC = DSC_normal,<br>
+                                   bool AllowConstexprs = false);<br>
<br>
</span>----------------<br>
<span class="">rsmith wrote:<br>
> Please add a new DSC value for conditions rather than adding a flag here.<br>
</span>Will fix.<br>
<span class=""><br>
================<br>
Comment at: lib/Sema/SemaExprCXX.cpp:2585<br>
@@ -2584,1 +2584,3 @@<br>
<br>
</span><span class="">+  Condition = MaybeCreateExprWithCleanups(Condition.get());<br>
+<br>
</span>----------------<br>
<span class="">rsmith wrote:<br>
> Do your tests cover the need for this?<br>
</span>Not yet, but I will add a test for it.<br>
<span class=""><br>
================<br>
Comment at: test/CXX/stmt.stmt/stmt.select/p6.cpp:4-5<br>
@@ +3,4 @@<br>
</span><span class="">+<br>
+// This test verifies the functionality specified by DR948:<br>
+// <a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#948" target="_blank">http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#948</a><br>
+<br>
</span>----------------<br>
<span class="">rsmith wrote:<br>
> The right place for such a test is tests/CXX/drs/dr9xx.cpp.<br>
</span>Will fix.<br>
<div class="HOEnZb"><div class="h5"><br>
<a href="http://reviews.llvm.org/D8978" target="_blank">http://reviews.llvm.org/D8978</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></div><br></div></div>