<div dir="ltr"><br><div>Hi Richard,</div><div><br></div><div>Thanks for your valuable inputs!</div><div>Updated patch as per comments conserving the compat warnings!</div><div><br></div><div>Please if you could review.</div>
<div>If the changes look good, please if could commit the same for me.</div><div><br></div><div>Thanks,</div><div>Rahul</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Apr 17, 2014 at 11:15 PM, 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 class="">On Thu, Apr 17, 2014 at 5:02 AM, Rahul Jain <span dir="ltr"><<a href="mailto:1989.rahuljain@gmail.com" target="_blank">1989.rahuljain@gmail.com</a>></span> wrote:<br>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div>Hi Richard,</div><div><br></div><div class=""><div>Thanks for your valuable inputs!</div>
<div>Updated patch to take care of the other case as well.</div>
<div>Added a test case for that too!</div><div><br></div><div>
<br></div><div><div>Index: lib/Sema/SemaDeclCXX.cpp</div><div>===================================================================</div><div>--- lib/Sema/SemaDeclCXX.cpp<span style="white-space:pre-wrap"> </span>(revision 206450)</div>
<div>+++ lib/Sema/SemaDeclCXX.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -1175,14 +1175,21 @@</div><div> } else {</div><div> if (ReturnStmts.empty()) {</div><div><div> // C++1y doesn't require constexpr functions to contain a 'return'</div>
</div><div>- // statement. We still do, unless the return type is void, because</div><div>+ // statement. We still do, unless the return type might be void, because</div><div><div> // otherwise if there's no return statement, the function cannot</div>
<div>- // be used in a core constant expression.</div><div>+ // be used in a core constant expression. Also, deduced return type</div><div>+ // with no return statements are perfectly legal. For example:</div>
<div>+ // template<typename T> struct X { constexpr auto f() {} };</div></div></div></div></div></blockquote><div><br></div><div>This second comment change is now redundant; this is covered by 'might be void'.</div>
<div class="">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>+</div><div> bool OK = getLangOpts().CPlusPlus1y && Dcl->getReturnType()->isVoidType();</div>
<div>+</div></div><div>+ if (!Dcl->getReturnType()->isVoidType() && </div>
<div>+ !Dcl->getReturnType()->isDependentType()) {</div></div></div></blockquote><div><br></div></div><div>Having an 'if' here is incorrect -- we should produce the diagnostic regardless, or we'll miss the C++11-compat warning (this should also be conditioned on C++1y mode). Please just add the dependent type check to 'OK' instead.</div>
<div><div class="h5">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div> Diag(Dcl->getLocation(),</div><div> OK ? diag::warn_cxx11_compat_constexpr_body_no_return</div>
<div> : diag::err_constexpr_body_no_return);</div>
<div> return OK;</div><div>+ }</div><div> }</div></div><div><div> if (ReturnStmts.size() > 1) {</div><div> Diag(ReturnStmts.back(),</div><div>Index: test/SemaCXX/cxx1y-deduced-return-type.cpp</div>
<div>===================================================================</div>
</div><div>--- test/SemaCXX/cxx1y-deduced-return-type.cpp<span style="white-space:pre-wrap"> </span>(revision 206450)</div><div>+++ test/SemaCXX/cxx1y-deduced-return-type.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -261,6 +261,8 @@</div><div><div> </div><div> namespace Constexpr {</div><div> constexpr auto f1(int n) { return n; }</div><div>+ template<typename T> struct X { constexpr auto f() {} }; // PR18746</div>
</div><div>+ template<typename T> struct Y { constexpr T f() {} }; // ok</div><div>
<div> struct NonLiteral { ~NonLiteral(); } nl; // expected-note {{user-provided destructor}}</div><div> constexpr auto f2(int n) { return nl; } // expected-error {{return type 'Constexpr::NonLiteral' is not a literal type}}</div>
<div> }</div></div></div><div><br></div><div><br></div><div><br></div><div>Please if you could review the same!</div><div><br></div><div>Thanks,</div><div>Rahul</div></div><div><div><div class="gmail_extra">
<br><br><div class="gmail_quote">
On Mon, Apr 14, 2014 at 6:51 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>On Tue, Apr 8, 2014 at 10:56 AM, Rahul Jain <span dir="ltr"><<a href="mailto:1989.rahuljain@gmail.com" target="_blank">1989.rahuljain@gmail.com</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"><br><div>Hi Richard,</div><div><br></div><div>Hi all,<br></div><div><br></div><div>This patch fixes PR18746.</div>
<div><br></div><div><br></div><div><div>Index: lib/Sema/SemaDeclCXX.cpp</div><div>===================================================================</div>
<div>--- lib/Sema/SemaDeclCXX.cpp<span style="white-space:pre-wrap"> </span>(revision 205775)</div><div>+++ lib/Sema/SemaDeclCXX.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -1177,12 +1177,19 @@</div>
<div> // C++1y doesn't require constexpr functions to contain a 'return'</div><div> // statement. We still do, unless the return type is void, because</div><div> // otherwise if there's no return statement, the function cannot</div>
<div>- // be used in a core constant expression.</div><div>+ // be used in a core constant expression. Also, deduced return type</div><div>+ // with no return statements are perfectly legal. For example:</div>
<div>+ // template<typename T> struct X { constexpr auto f() {} };</div><div>+</div><div> bool OK = getLangOpts().CPlusPlus1y && Dcl->getReturnType()->isVoidType();</div><div>+</div><div>+ if (!getLangOpts().CPlusPlus1y ||</div>
<div>+ !Dcl->getReturnType()->getContainedAutoType()) {</div></div></div></blockquote><div><br></div></div><div>I don't think this is quite the right test. We have the same issue here:</div><div><br></div>
<div>
template<typename T> constexpr T f() {}</div><div><br></div><div>So...</div><div>1) Update the comment to say 'unless the return type might be void' instead of 'unless the return type is void', and</div>
<div>2) Check for (Dcl->getReturnType()->isVoidType() || Dcl->getReturnType()->isDependentType())</div><div><br></div><div>At this point, we should have already replaced the 'auto' return type with a dependent 'auto' return type, so this check should handle both the template case and the 'auto' case.</div>
<div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div> Diag(Dcl->getLocation(),</div><div> OK ? diag::warn_cxx11_compat_constexpr_body_no_return</div>
<div> : diag::err_constexpr_body_no_return);</div>
<div> return OK;</div><div>+ }</div></div></div></blockquote><div><br></div></div><div>The `if` around this looks wrong. We want to provide the cxx11_compat diagnostic in all cases where the C++1y rule applies.</div>
<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><div> }</div><div> if (ReturnStmts.size() > 1) {</div><div> Diag(ReturnStmts.back(),</div>
<div>Index: test/SemaCXX/cxx1y-deduced-return-type.cpp</div><div>===================================================================</div>
<div>--- test/SemaCXX/cxx1y-deduced-return-type.cpp<span style="white-space:pre-wrap"> </span>(revision 205775)</div><div>+++ test/SemaCXX/cxx1y-deduced-return-type.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div>
<div>@@ -261,6 +261,7 @@</div><div> </div><div> namespace Constexpr {</div><div> constexpr auto f1(int n) { return n; }</div><div>+ template<typename T> struct X { constexpr auto f() {} }; // PR18746</div><div> struct NonLiteral { ~NonLiteral(); } nl; // expected-note {{user-provided destructor}}</div>
<div> constexpr auto f2(int n) { return nl; } // expected-error {{return type 'Constexpr::NonLiteral' is not a literal type}}</div><div> }</div></div><div><br></div><div><br></div><div><div>Please if someone could review the same.</div>
</div><div><br></div><div>Thanks,</div><div>Rahul</div></div>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>