Hi Richard, Thanks a lot for your help on this. Highly appreciate it!
I have closed the bug. Thanks to Suyog Sarda for helping me out on this! Thanks, Rahul On Wed, Apr 23, 2014 at 4:51 AM, Richard Smith <[email protected]>wrote: > Committed with minor tweaks as r206929. Thanks! > > > On Sat, Apr 19, 2014 at 3:44 AM, Rahul Jain <[email protected]>wrote: > >> >> Hi Richard, >> >> Thanks for your valuable inputs! >> Updated patch as per comments conserving the compat warnings! >> >> Please if you could review. >> If the changes look good, please if could commit the same for me. >> >> Thanks, >> Rahul >> >> >> On Thu, Apr 17, 2014 at 11:15 PM, Richard Smith <[email protected]>wrote: >> >>> On Thu, Apr 17, 2014 at 5:02 AM, Rahul Jain <[email protected]>wrote: >>> >>>> >>>> Hi Richard, >>>> >>>> Thanks for your valuable inputs! >>>> Updated patch to take care of the other case as well. >>>> Added a test case for that too! >>>> >>>> >>>> Index: lib/Sema/SemaDeclCXX.cpp >>>> =================================================================== >>>> --- lib/Sema/SemaDeclCXX.cpp (revision 206450) >>>> +++ lib/Sema/SemaDeclCXX.cpp (working copy) >>>> @@ -1175,14 +1175,21 @@ >>>> } else { >>>> if (ReturnStmts.empty()) { >>>> // C++1y doesn't require constexpr functions to contain a >>>> 'return' >>>> - // statement. We still do, unless the return type is void, >>>> because >>>> + // statement. We still do, unless the return type might be void, >>>> because >>>> // otherwise if there's no return statement, the function cannot >>>> - // be used in a core constant expression. >>>> + // be used in a core constant expression. Also, deduced return >>>> type >>>> + // with no return statements are perfectly legal. For example: >>>> + // template<typename T> struct X { constexpr auto f() {} }; >>>> >>> >>> This second comment change is now redundant; this is covered by 'might >>> be void'. >>> >>> >>>> + >>>> bool OK = getLangOpts().CPlusPlus1y && >>>> Dcl->getReturnType()->isVoidType(); >>>> + >>>> + if (!Dcl->getReturnType()->isVoidType() && >>>> + !Dcl->getReturnType()->isDependentType()) { >>>> >>> >>> 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. >>> >>> >>>> Diag(Dcl->getLocation(), >>>> OK ? diag::warn_cxx11_compat_constexpr_body_no_return >>>> : diag::err_constexpr_body_no_return); >>>> return OK; >>>> + } >>>> } >>>> if (ReturnStmts.size() > 1) { >>>> Diag(ReturnStmts.back(), >>>> Index: test/SemaCXX/cxx1y-deduced-return-type.cpp >>>> =================================================================== >>>> --- test/SemaCXX/cxx1y-deduced-return-type.cpp (revision 206450) >>>> +++ test/SemaCXX/cxx1y-deduced-return-type.cpp (working copy) >>>> @@ -261,6 +261,8 @@ >>>> >>>> namespace Constexpr { >>>> constexpr auto f1(int n) { return n; } >>>> + template<typename T> struct X { constexpr auto f() {} }; // PR18746 >>>> + template<typename T> struct Y { constexpr T f() {} }; // ok >>>> struct NonLiteral { ~NonLiteral(); } nl; // expected-note >>>> {{user-provided destructor}} >>>> constexpr auto f2(int n) { return nl; } // expected-error {{return >>>> type 'Constexpr::NonLiteral' is not a literal type}} >>>> } >>>> >>>> >>>> >>>> Please if you could review the same! >>>> >>>> Thanks, >>>> Rahul >>>> >>>> >>>> On Mon, Apr 14, 2014 at 6:51 AM, Richard Smith >>>> <[email protected]>wrote: >>>> >>>>> On Tue, Apr 8, 2014 at 10:56 AM, Rahul Jain >>>>> <[email protected]>wrote: >>>>> >>>>>> >>>>>> Hi Richard, >>>>>> >>>>>> Hi all, >>>>>> >>>>>> This patch fixes PR18746. >>>>>> >>>>>> >>>>>> Index: lib/Sema/SemaDeclCXX.cpp >>>>>> =================================================================== >>>>>> --- lib/Sema/SemaDeclCXX.cpp (revision 205775) >>>>>> +++ lib/Sema/SemaDeclCXX.cpp (working copy) >>>>>> @@ -1177,12 +1177,19 @@ >>>>>> // C++1y doesn't require constexpr functions to contain a >>>>>> 'return' >>>>>> // statement. We still do, unless the return type is void, >>>>>> because >>>>>> // otherwise if there's no return statement, the function >>>>>> cannot >>>>>> - // be used in a core constant expression. >>>>>> + // be used in a core constant expression. Also, deduced return >>>>>> type >>>>>> + // with no return statements are perfectly legal. For example: >>>>>> + // template<typename T> struct X { constexpr auto f() {} }; >>>>>> + >>>>>> bool OK = getLangOpts().CPlusPlus1y && >>>>>> Dcl->getReturnType()->isVoidType(); >>>>>> + >>>>>> + if (!getLangOpts().CPlusPlus1y || >>>>>> + !Dcl->getReturnType()->getContainedAutoType()) { >>>>>> >>>>> >>>>> I don't think this is quite the right test. We have the same issue >>>>> here: >>>>> >>>>> template<typename T> constexpr T f() {} >>>>> >>>>> So... >>>>> 1) Update the comment to say 'unless the return type might be void' >>>>> instead of 'unless the return type is void', and >>>>> 2) Check for (Dcl->getReturnType()->isVoidType() || >>>>> Dcl->getReturnType()->isDependentType()) >>>>> >>>>> 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. >>>>> >>>>> Diag(Dcl->getLocation(), >>>>>> OK ? diag::warn_cxx11_compat_constexpr_body_no_return >>>>>> : diag::err_constexpr_body_no_return); >>>>>> return OK; >>>>>> + } >>>>>> >>>>> >>>>> The `if` around this looks wrong. We want to provide the cxx11_compat >>>>> diagnostic in all cases where the C++1y rule applies. >>>>> >>>>> >>>>>> } >>>>>> if (ReturnStmts.size() > 1) { >>>>>> Diag(ReturnStmts.back(), >>>>>> Index: test/SemaCXX/cxx1y-deduced-return-type.cpp >>>>>> =================================================================== >>>>>> --- test/SemaCXX/cxx1y-deduced-return-type.cpp (revision 205775) >>>>>> +++ test/SemaCXX/cxx1y-deduced-return-type.cpp (working copy) >>>>>> @@ -261,6 +261,7 @@ >>>>>> >>>>>> namespace Constexpr { >>>>>> constexpr auto f1(int n) { return n; } >>>>>> + template<typename T> struct X { constexpr auto f() {} }; // PR18746 >>>>>> struct NonLiteral { ~NonLiteral(); } nl; // expected-note >>>>>> {{user-provided destructor}} >>>>>> constexpr auto f2(int n) { return nl; } // expected-error {{return >>>>>> type 'Constexpr::NonLiteral' is not a literal type}} >>>>>> } >>>>>> >>>>>> >>>>>> Please if someone could review the same. >>>>>> >>>>>> Thanks, >>>>>> Rahul >>>>>> >>>>> >>>>> >>>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
