[Patch] Fix for PR18746 -rejects-valid on a constexpr member function of a class template with a deduced return type and no return statements
Rahul Jain
1989.rahuljain at gmail.com
Tue Apr 22 22:37:26 PDT 2014
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 <richard at metafoo.co.uk>wrote:
> Committed with minor tweaks as r206929. Thanks!
>
>
> On Sat, Apr 19, 2014 at 3:44 AM, Rahul Jain <1989.rahuljain at gmail.com>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 <richard at metafoo.co.uk>wrote:
>>
>>> On Thu, Apr 17, 2014 at 5:02 AM, Rahul Jain <1989.rahuljain at gmail.com>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 <richard at metafoo.co.uk>wrote:
>>>>
>>>>> On Tue, Apr 8, 2014 at 10:56 AM, Rahul Jain <1989.rahuljain at gmail.com>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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140423/17cc4b04/attachment.html>
More information about the cfe-commits
mailing list