[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gcc-patches
Subject:    Re: [PATCH] c++: Use error_at rather than warning_at for missing return in constexpr functions [PR96
From:       Jason Merrill via Gcc-patches <gcc-patches () gcc ! gnu ! org>
Date:       2020-07-31 20:28:40
Message-ID: ee488ea1-8f47-c453-579b-c2d6cc564cfa () redhat ! com
[Download RAW message or body]

On 7/31/20 3:52 AM, Jakub Jelinek wrote:
> On Wed, Jul 29, 2020 at 03:30:07PM -0400, Jason Merrill via Gcc-patches wrote:
> > On 7/14/20 4:50 AM, Jakub Jelinek wrote:
> > > For C++11 we already emit an error if a constexpr function doesn't contain
> > > a return statement, because in C++11 that is the only thing it needs to
> > > contain, but for C++14 we would normally issue a -Wreturn-type warning.
> > > 
> > > As mentioned by Jonathan, such constexpr functions are invalid, no
> > > diagnostics required, because there doesn't exist any arguments for
> > > which it would result in valid constant expression.
> > > 
> > > This raises it to an error in such cases.  The !LAMBDA_TYPE_P case
> > > is to avoid error on g++.dg/pr81194.C where the user didn't write
> > > constexpr anywhere and the operator() is compiler generated.
> > 
> > We set DECL_DECLARED_CONSTEXPR_P on lambdas earlier in finish_function if
> > suitable; can we move this diagnostic up before that?
> 
> That works too.  Bootstrapped/regtested on x86_64-linux and i686-linux, ok
> for trunk then?

OK.

> 2020-07-31  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/96182
> 	* decl.c (finish_function): In constexpr functions use for C++14 and
> 	later error instead of warning if no return statement is present and
> 	diagnose it regardless of warn_return_type.  Move the warn_return_type
> 	diagnostics earlier in the function.
> 
> 	* g++.dg/cpp1y/constexpr-96182.C: New test.
> 	* g++.dg/other/error35.C (S<T>::g()): Add return statement.
> 	* g++.dg/cpp1y/pr63996.C (foo): Likewise.
> 	* g++.dg/cpp1y/constexpr-return2.C (f): Likewise.
> 	* g++.dg/cpp1y/var-templ44.C (make_array): Add throw 1.
> 
> --- gcc/cp/decl.c.jj	2020-07-29 11:57:23.340517489 +0200
> +++ gcc/cp/decl.c	2020-07-30 20:44:33.634951396 +0200
> @@ -17112,6 +17112,51 @@ finish_function (bool inline_p)
> 				      DECL_ATTRIBUTES (fndecl)))
> omp_declare_variant_finalize (fndecl, attr);
> 
> +  /* Complain if there's just no return statement.  */
> +  if ((warn_return_type
> +       || (cxx_dialect >= cxx14
> +	   && DECL_DECLARED_CONSTEXPR_P (fndecl)))
> +      && !VOID_TYPE_P (TREE_TYPE (fntype))
> +      && !dependent_type_p (TREE_TYPE (fntype))
> +      && !current_function_returns_value && !current_function_returns_null
> +      /* Don't complain if we abort or throw.  */
> +      && !current_function_returns_abnormally
> +      /* Don't complain if there's an infinite loop.  */
> +      && !current_function_infinite_loop
> +      /* Don't complain if we are declared noreturn.  */
> +      && !TREE_THIS_VOLATILE (fndecl)
> +      && !DECL_NAME (DECL_RESULT (fndecl))
> +      && !TREE_NO_WARNING (fndecl)
> +      /* Structor return values (if any) are set by the compiler.  */
> +      && !DECL_CONSTRUCTOR_P (fndecl)
> +      && !DECL_DESTRUCTOR_P (fndecl)
> +      && targetm.warn_func_return (fndecl))
> +    {
> +      gcc_rich_location richloc (input_location);
> +      /* Potentially add a "return *this;" fix-it hint for
> +	 assignment operators.  */
> +      if (IDENTIFIER_ASSIGN_OP_P (DECL_NAME (fndecl)))
> +	{
> +	  tree valtype = TREE_TYPE (DECL_RESULT (fndecl));
> +	  if (TREE_CODE (valtype) == REFERENCE_TYPE
> +	      && current_class_ref
> +	      && same_type_ignoring_top_level_qualifiers_p
> +		  (TREE_TYPE (valtype), TREE_TYPE (current_class_ref))
> +	      && global_dc->option_enabled (OPT_Wreturn_type,
> +					    global_dc->lang_mask,
> +					    global_dc->option_state))
> +	    add_return_star_this_fixit (&richloc, fndecl);
> +	}
> +      if (cxx_dialect >= cxx14
> +	  && DECL_DECLARED_CONSTEXPR_P (fndecl))
> +	error_at (&richloc, "no return statement in %<constexpr%> function "
> +			    "returning non-void");
> +      else if (warning_at (&richloc, OPT_Wreturn_type,
> +			   "no return statement in function returning "
> +			   "non-void"))
> +	TREE_NO_WARNING (fndecl) = 1;
> +    }
> +
> /* Lambda closure members are implicitly constexpr if possible.  */
> if (cxx_dialect >= cxx17
> && LAMBDA_TYPE_P (CP_DECL_CONTEXT (fndecl)))
> @@ -17163,44 +17208,6 @@ finish_function (bool inline_p)
> to the FUNCTION_DECL node itself.  */
> BLOCK_SUPERCONTEXT (DECL_INITIAL (fndecl)) = fndecl;
> 
> -  /* Complain if there's just no return statement.  */
> -  if (warn_return_type
> -      && !VOID_TYPE_P (TREE_TYPE (fntype))
> -      && !dependent_type_p (TREE_TYPE (fntype))
> -      && !current_function_returns_value && !current_function_returns_null
> -      /* Don't complain if we abort or throw.  */
> -      && !current_function_returns_abnormally
> -      /* Don't complain if there's an infinite loop.  */
> -      && !current_function_infinite_loop
> -      /* Don't complain if we are declared noreturn.  */
> -      && !TREE_THIS_VOLATILE (fndecl)
> -      && !DECL_NAME (DECL_RESULT (fndecl))
> -      && !TREE_NO_WARNING (fndecl)
> -      /* Structor return values (if any) are set by the compiler.  */
> -      && !DECL_CONSTRUCTOR_P (fndecl)
> -      && !DECL_DESTRUCTOR_P (fndecl)
> -      && targetm.warn_func_return (fndecl))
> -    {
> -      gcc_rich_location richloc (input_location);
> -      /* Potentially add a "return *this;" fix-it hint for
> -	 assignment operators.  */
> -      if (IDENTIFIER_ASSIGN_OP_P (DECL_NAME (fndecl)))
> -	{
> -	  tree valtype = TREE_TYPE (DECL_RESULT (fndecl));
> -	  if (TREE_CODE (valtype) == REFERENCE_TYPE
> -	      && current_class_ref
> -	      && same_type_ignoring_top_level_qualifiers_p
> -		  (TREE_TYPE (valtype), TREE_TYPE (current_class_ref))
> -	      && global_dc->option_enabled (OPT_Wreturn_type,
> -					    global_dc->lang_mask,
> -					    global_dc->option_state))
> -	    add_return_star_this_fixit (&richloc, fndecl);
> -	}
> -      if (warning_at (&richloc, OPT_Wreturn_type,
> -	  "no return statement in function returning non-void"))
> -	TREE_NO_WARNING (fndecl) = 1;
> -    }
> -
> /* Store the end of the function, so that we get good line number
> info for the epilogue.  */
> cfun->function_end_locus = input_location;
> --- gcc/testsuite/g++.dg/other/error35.C.jj	2020-01-12 11:54:37.214401324 +0100
> +++ gcc/testsuite/g++.dg/other/error35.C	2020-07-13 22:35:55.359228614 +0200
> @@ -9,6 +9,6 @@ template <typename> struct S {
> enum S<char>::E;
> template <typename T> enum S<T>::E : int { b };
> template <typename T>
> -constexpr int S<T>::g() const { b; } // { dg-error "not declared" }
> +constexpr int S<T>::g() const { b; if (false) return 0; } // { dg-error "not \
> declared" } static_assert(S<char>().g() == 1, ""); // { dg-error "" }
> // { dg-message "in .constexpr. expansion of" "" { target *-*-* } .-1 }
> --- gcc/testsuite/g++.dg/cpp1y/pr63996.C.jj	2020-01-12 11:54:37.000000000 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/pr63996.C	2020-07-13 22:17:39.034004329 +0200
> @@ -5,6 +5,7 @@ constexpr int
> foo (int i)
> {
> int a[i] = { }; // { dg-error "7:ISO C\\+\\+ forbids variable length array .a" }
> +  if (i == 23) return 0;
> }
> 
> constexpr int j = foo (1); // { dg-error "flows off the end|in .constexpr. \
>                 expansion of" }
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-96182.C.jj	2020-07-13 19:16:42.742936492 \
>                 +0200
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-96182.C	2020-07-13 19:16:12.264357640 \
> +0200 @@ -0,0 +1,6 @@
> +// PR c++/96182
> +// { dg-do compile { target c++11 } }
> +
> +constexpr int foo () {} // { dg-error "no return statement in 'constexpr' function \
> returning non-void" "" { target c++14 } } +// { dg-error "body of 'constexpr' \
> function 'constexpr int foo\\\(\\\)' not a return-statement" "" { target c++11_only \
> } .-1 } +// { dg-warning "no return statement in function returning non-void" "" { \
>                 target c++11_only } .-2 }
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-return2.C.jj	2020-01-12 11:54:37.115402818 \
>                 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-return2.C	2020-07-13 22:17:03.582513397 \
> +0200 @@ -3,6 +3,7 @@
> 
> constexpr int f (int i)
> {
> +  if (i == -1) return 0;
> }
> 
> constexpr int i = f(42);	// { dg-error "flows off the end|in .constexpr. expansion \
>                 of " }
> --- gcc/testsuite/g++.dg/cpp1y/var-templ44.C.jj	2020-01-12 11:54:37.123402697 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/var-templ44.C	2020-07-13 22:35:03.322980157 +0200
> @@ -26,5 +26,6 @@ constexpr auto make_array()
> -> array<conditional_t<is_void_v<_Dest>, common_type_t<>, _Dest>,
> sizeof...(_Types)> {
> static_assert(__or_<__not_<is_void<_Dest>>, __and_<>>::value, ""); // { dg-error \
> "static assert" } +  throw 1;
> }
> auto d = make_array();
> 
> 
> 	Jakub
> 


[prev in list] [next in list] [prev in thread] [next in thread]