[Patch] Fix for PR18746 -rejects-valid on a constexpr member function of a class template with a deduced return type and no return statements

[Richard Smith]

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/20140413/bb2b3611/attachment.html>