diff mbox

[C++] More C++11 and C++14 constexpr work

Message ID 543594DA.1000304@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Oct. 8, 2014, 7:47 p.m. UTC 
Hi,

the below tries to make progress on some relatively easy issues I 
noticed while working on c++/55250. At least:
1- We rejected using declarations and using directives in constructors.
2- In C++14 mode we rejected compound-statements both in constructors 
and elsewhere (and in C++11 we provided both the pedwarn and then an 
hard error)
3- In C++14 mode we rejected local variables in constructors.

The patch itself uses quite straightforward strategies: 
check_constexpr_ctor_body is extended via the new 
check_constexpr_ctor_body_1; recursion on BIND_EXPRs takes care of 
compound-statements; the parser is tweaked to suppress the 
"compound-statement in constexpr function" pedwarn in C++14 mode.

Tested x86_64-linux.

Thanks,
Paolo.

//////////////////////
/cp
2014-10-08  Paolo Carlini  <paolo.carlini@oracle.com>

	* semantics.c (check_constexpr_ctor_body_1): New.
	(check_constexpr_ctor_body): Use it; add bool parameter.
	(build_data_member_initialization): Handle BIND_EXPR and
	USING_STMT in the main conditional.
	(build_constexpr_constructor_member_initializers): Do not
	handle BIND_EXPR here.
	(constexpr_fn_retval): Handle BIND_EXPR in the switch.
	(massage_constexpr_body): Don't do it here.
	* parser.c (cp_parser_ctor_initializer_opt_and_function_body):
	Adjust check_constexpr_ctor_body call.
	(cp_parser_compound_statement): Do not pedwarn for compound-statement
	in constexpr function in C++14 mode.
	* cp-tree.h (check_constexpr_ctor_body): Update declaration.

/testsuite
2014-10-08  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/cpp0x/constexpr-using3.C: New.
	* g++.dg/cpp1y/constexpr-local-compound1.C: Likewise.
	* g++.dg/cpp1y/constexpr-type-def-compound1.C: Likewise.
	* g++.dg/cpp1y/constexpr-local1.C: Extend.
	* g++.dg/cpp0x/constexpr-compound.C: Specify expected error.

Comments

Jason Merrill Oct. 9, 2014, 1:31 p.m. UTC | #1 
On 10/08/2014 03:47 PM, Paolo Carlini wrote:
> 	(check_constexpr_ctor_body): Use it; add bool parameter.

This function seems to only be called in one place; why add the parameter?

Jason
Paolo Carlini Oct. 9, 2014, 1:49 p.m. UTC | #2 
Hi,

On 10/09/2014 03:31 PM, Jason Merrill wrote:
> On 10/08/2014 03:47 PM, Paolo Carlini wrote:
>>     (check_constexpr_ctor_body): Use it; add bool parameter.
> This function seems to only be called in one place; why add the parameter?
Is also called recursively by check_constexpr_ctor_body_1 and without 
the complain boolean we end up printing the error message twice.

Paolo.
Paolo Carlini Oct. 9, 2014, 1:56 p.m. UTC | #3 
.. a simple example in C++11 would be:

struct S
{
   constexpr S() { { struct T { }; } }
};

Paolo.
Jason Merrill Oct. 9, 2014, 2:18 p.m. UTC | #4 
On 10/09/2014 09:49 AM, Paolo Carlini wrote:
> Hi,
>
> On 10/09/2014 03:31 PM, Jason Merrill wrote:
>> On 10/08/2014 03:47 PM, Paolo Carlini wrote:
>>>     (check_constexpr_ctor_body): Use it; add bool parameter.
>> This function seems to only be called in one place; why add the
>> parameter?
> Is also called recursively by check_constexpr_ctor_body_1 and without
> the complain boolean we end up printing the error message twice.

Ah, guess I overlooked that.  OK.

Jason
Paolo Carlini Oct. 9, 2014, 3:15 p.m. UTC | #5 
Hi,

On 10/09/2014 04:18 PM, Jason Merrill wrote:
> On 10/09/2014 09:49 AM, Paolo Carlini wrote:
>> Hi,
>>
>> On 10/09/2014 03:31 PM, Jason Merrill wrote:
>>> On 10/08/2014 03:47 PM, Paolo Carlini wrote:
>>>>     (check_constexpr_ctor_body): Use it; add bool parameter.
>>> This function seems to only be called in one place; why add the
>>> parameter?
>> Is also called recursively by check_constexpr_ctor_body_1 and without
>> the complain boolean we end up printing the error message twice.
> Ah, guess I overlooked that.  OK.
Thanks, I'm going to commit the patch.

I noticed today that given the actual C++11 the error messages we provide:

     "constexpr constructor does not have empty body"

and:

     "body of constexpr function ‘XXX’ not a return-statement"

are rather outdated and misleading. In principle we should probably also 
provide more fine grained error messages, but if you have suggestions 
for less misleading catch all, I volunteer to do the change and adjust 
the testcases...

Also, I have been thinking that it would probably make sense to move 
constexpr-related code to a separate cp/constexpr.c: what do you think? 
Functions with *constexpr* in the name, the various cxx_eval_* and the 
various potential_constant_* would qualify, I think.

Paolo.
Jason Merrill Oct. 10, 2014, 3:15 a.m. UTC | #6 
On 10/09/2014 11:15 AM, Paolo Carlini wrote:
> I noticed today that given the actual C++11 the error messages we provide:
>
>      "constexpr constructor does not have empty body"
>
> and:
>
>      "body of constexpr function ‘XXX’ not a return-statement"
>
> are rather outdated and misleading. In principle we should probably also
> provide more fine grained error messages, but if you have suggestions
> for less misleading catch all, I volunteer to do the change and adjust
> the testcases...

I don't know that there's much room for improvement in a catch-all 
message, though I'm open to suggestions.  Better to give fine grained 
errors, as you say.

> Also, I have been thinking that it would probably make sense to move
> constexpr-related code to a separate cp/constexpr.c: what do you think?

Yes, I've been thinking about that, too.

Jason
diff mbox

Patch

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 216010)
+++ cp/cp-tree.h	(working copy)
@@ -5822,7 +5822,7 @@  extern void finish_handler			(tree);
 extern void finish_cleanup			(tree, tree);
 extern bool literal_type_p (tree);
 extern tree register_constexpr_fundef (tree, tree);
-extern bool check_constexpr_ctor_body (tree, tree);
+extern bool check_constexpr_ctor_body (tree, tree, bool);
 extern tree ensure_literal_type_for_constexpr_object (tree);
 extern bool potential_constant_expression (tree);
 extern bool potential_rvalue_constant_expression (tree);
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 216010)
+++ cp/parser.c	(working copy)
@@ -9891,7 +9891,7 @@  cp_parser_compound_statement (cp_parser *parser, t
   if (!cp_parser_require (parser, CPP_OPEN_BRACE, RT_OPEN_BRACE))
     return error_mark_node;
   if (DECL_DECLARED_CONSTEXPR_P (current_function_decl)
-      && !function_body)
+      && !function_body && cxx_dialect < cxx14)
     pedwarn (input_location, OPT_Wpedantic,
 	     "compound-statement in constexpr function");
   /* Begin the compound-statement.  */
@@ -19015,7 +19015,7 @@  cp_parser_ctor_initializer_opt_and_function_body (
   /* Parse the function-body.  */
   cp_parser_function_body (parser, in_function_try_block);
   if (check_body_p)
-    check_constexpr_ctor_body (last, list);
+    check_constexpr_ctor_body (last, list, /*complain=*/true);
   /* Finish the function body.  */
   finish_function_body (body);
 
Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 216010)
+++ cp/semantics.c	(working copy)
@@ -7779,8 +7779,12 @@  build_data_member_initialization (tree t, vec<cons
 	 wrong type, but cxx_eval_constant_expression doesn't care.  */
       init = break_out_target_exprs (t);
     }
-  else if (TREE_CODE (t) == DECL_EXPR)
-    /* Declaring a temporary, don't add it to the CONSTRUCTOR.  */
+  else if (TREE_CODE (t) == BIND_EXPR)
+    return build_data_member_initialization (BIND_EXPR_BODY (t), vec);
+  else if (TREE_CODE (t) == DECL_EXPR
+	   || TREE_CODE (t) == USING_STMT)
+    /* Declaring a temporary, don't add it to the CONSTRUCTOR.
+       Likewise for using directives.  */
     return true;
   else
     gcc_unreachable ();
@@ -7833,7 +7837,7 @@  build_data_member_initialization (tree t, vec<cons
   return true;
 }
 
-/* Subroutine of check_constexpr_ctor_body and massage_constexpr_body.
+/* Subroutine of check_constexpr_ctor_body_1 and constexpr_fn_retval.
    In C++11 mode checks that the TYPE_DECLs in the BIND_EXPR_VARS of a 
    BIND_EXPR conform to 7.1.5/3/4 on typedef and alias declarations.  */
 
@@ -7852,11 +7856,45 @@  check_constexpr_bind_expr_vars (tree t)
   return true;
 }
 
+/* Subroutine of check_constexpr_ctor_body.  */
+
+static bool
+check_constexpr_ctor_body_1 (tree last, tree list)
+{
+  switch (TREE_CODE (list))
+    {
+    case DECL_EXPR:
+      if (TREE_CODE (DECL_EXPR_DECL (list)) == USING_DECL)
+	return true;
+      if (cxx_dialect >= cxx14)
+	return true;
+      return false;
+
+    case CLEANUP_POINT_EXPR:
+      return check_constexpr_ctor_body (last, TREE_OPERAND (list, 0),
+					/*complain=*/false);
+
+    case BIND_EXPR:
+       if (!check_constexpr_bind_expr_vars (list)
+	   || !check_constexpr_ctor_body (last, BIND_EXPR_BODY (list),
+					  /*complain=*/false))
+	 return false;
+       return true;
+
+    case USING_STMT:
+    case STATIC_ASSERT:
+      return true;
+
+    default:
+      return false;
+    }
+}
+
 /* Make sure that there are no statements after LAST in the constructor
    body represented by LIST.  */
 
 bool
-check_constexpr_ctor_body (tree last, tree list)
+check_constexpr_ctor_body (tree last, tree list, bool complain)
 {
   bool ok = true;
   if (TREE_CODE (list) == STATEMENT_LIST)
@@ -7867,20 +7905,8 @@  bool
 	  tree t = tsi_stmt (i);
 	  if (t == last)
 	    break;
-	  if (TREE_CODE (t) == BIND_EXPR)
+	  if (!check_constexpr_ctor_body_1 (last, t))
 	    {
-	      if (!check_constexpr_bind_expr_vars (t))
-		{
-		  ok = false;
-		  break;
-		}
-	      if (!check_constexpr_ctor_body (last, BIND_EXPR_BODY (t)))
-		return false;
-	      else
-		continue;
-	    }
-	  if (TREE_CODE (t) != STATIC_ASSERT)
-	    {
 	      ok = false;
 	      break;
 	    }
@@ -7887,11 +7913,12 @@  bool
 	}
     }
   else if (list != last
-	   && TREE_CODE (list) != STATIC_ASSERT)
+	   && !check_constexpr_ctor_body_1 (last, list))
     ok = false;
   if (!ok)
     {
-      error ("constexpr constructor does not have empty body");
+      if (complain)
+	error ("constexpr constructor does not have empty body");
       DECL_DECLARED_CONSTEXPR_P (current_function_decl) = false;
     }
   return ok;
@@ -7981,8 +8008,6 @@  build_constexpr_constructor_member_initializers (t
 	     "a function-try-block");
       return error_mark_node;
     }
-  else if (TREE_CODE (body) == BIND_EXPR)
-    ok = build_data_member_initialization (BIND_EXPR_BODY (body), &vec);
   else if (EXPR_P (body))
     ok = build_data_member_initialization (body, &vec);
   else
@@ -8050,6 +8075,11 @@  constexpr_fn_retval (tree body)
     case CLEANUP_POINT_EXPR:
       return constexpr_fn_retval (TREE_OPERAND (body, 0));
 
+    case BIND_EXPR:
+      if (!check_constexpr_bind_expr_vars (body))
+	return error_mark_node;
+      return constexpr_fn_retval (BIND_EXPR_BODY (body));
+
     case USING_STMT:
       return NULL_TREE;
 
@@ -8074,9 +8104,6 @@  massage_constexpr_body (tree fun, tree body)
         body = EH_SPEC_STMTS (body);
       if (TREE_CODE (body) == MUST_NOT_THROW_EXPR)
 	body = TREE_OPERAND (body, 0);
-      if (TREE_CODE (body) == BIND_EXPR
-	  && check_constexpr_bind_expr_vars (body))
-	body = BIND_EXPR_BODY (body);
       body = constexpr_fn_retval (body);
     }
   return body;
Index: testsuite/g++.dg/cpp0x/constexpr-compound.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-compound.C	(revision 216010)
+++ testsuite/g++.dg/cpp0x/constexpr-compound.C	(working copy)
@@ -2,8 +2,8 @@ 
 
 constexpr int f()
 {
-  {				// { dg-error "" }
+  {				// { dg-error "compound-statement" "" { target { c++11_only } } }
     return 1;
   }
-  { }				// { dg-error "" }
+  { }				// { dg-error "compound-statement" "" { target { c++11_only } } }
 }
Index: testsuite/g++.dg/cpp0x/constexpr-using3.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-using3.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/constexpr-using3.C	(working copy)
@@ -0,0 +1,29 @@ 
+// { dg-do compile { target c++11 } }
+// { dg-options "-pedantic" }
+
+namespace ns { typedef int T; }
+
+constexpr int Test1(int x) { using ns::T; typedef T U; return U(x); }
+constexpr int Test2(int x) { using namespace ns; typedef T U; return U(x); }
+constexpr int Test3(int x) { { using ns::T; typedef T U; return U(x); } }  // { dg-warning "compound-statement" "" { target { c++11_only } } }
+constexpr int Test4(int x) { { using namespace ns; typedef T U; return T(x); } }  // { dg-warning "compound-statement" "" { target { c++11_only } } }
+
+struct S1
+{
+  constexpr S1() { using ns::T; typedef T U; }
+};
+
+struct S2
+{
+  constexpr S2() { using namespace ns; typedef T U; }
+};
+
+struct S3
+{
+  constexpr S3() { { using ns::T; typedef T U; } }  // { dg-warning "compound-statement" "" { target { c++11_only } } }
+};
+
+struct S4
+{
+  constexpr S4() { { using namespace ns; typedef T U; } }  // { dg-warning "compound-statement" "" { target { c++11_only } } }
+};
Index: testsuite/g++.dg/cpp1y/constexpr-local-compound1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-local-compound1.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/constexpr-local-compound1.C	(working copy)
@@ -0,0 +1,14 @@ 
+// { dg-do compile { target c++14 } }
+
+#define SA(X) static_assert((X),#X)
+
+constexpr int f(int i) { { int j = i+1; return j; } }
+
+constexpr int i = f(41);
+
+struct S
+{
+  constexpr S() { { constexpr int j = 17; SA(j == 17); } }
+};
+
+SA(i==42);
Index: testsuite/g++.dg/cpp1y/constexpr-local1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-local1.C	(revision 216010)
+++ testsuite/g++.dg/cpp1y/constexpr-local1.C	(working copy)
@@ -1,9 +1,14 @@ 
 // { dg-do compile { target c++14 } }
 
+#define SA(X) static_assert((X),#X)
+
 constexpr int f(int i) { int j = i+1; return j; }
 
 constexpr int i = f(41);
 
-#define SA(X) static_assert((X),#X)
+struct S
+{
+  constexpr S() { constexpr int j = 17; SA(j == 17); }
+};
 
 SA(i==42);
Index: testsuite/g++.dg/cpp1y/constexpr-type-def-compound1.C
===================================================================
--- testsuite/g++.dg/cpp1y/constexpr-type-def-compound1.C	(revision 0)
+++ testsuite/g++.dg/cpp1y/constexpr-type-def-compound1.C	(working copy)
@@ -0,0 +1,60 @@ 
+// PR c++/55250
+// { dg-do compile { target c++14 } }
+
+#define SA(X) static_assert((X),#X)
+
+constexpr int Test1(int x) { { enum E { y = 1 }; return x + y; } }
+
+constexpr int Test2(int x) { { struct T { constexpr operator int() { return 1; } }; return x + T(); } }
+
+constexpr int Test3(int x) { { typedef enum E { y = 1 } EE; return x + EE::y; } }
+
+constexpr int Test4(int x) { { typedef struct T { constexpr operator int() { return 1; } } TT; return x + TT(); } }
+
+constexpr int Test5(int x) { { using EE = enum E { y = 1 }; return x + EE::y; } }
+
+constexpr int Test6(int x) { { using TT = struct T { constexpr operator int() { return 1; } }; return x + TT(); } }
+
+SA(Test1(2) == 3);
+SA(Test2(2) == 3);
+SA(Test3(2) == 3);
+SA(Test4(2) == 3);
+SA(Test5(2) == 3);
+SA(Test6(2) == 3);
+
+struct S1
+{
+  constexpr S1() { { enum E { y = 1 }; SA(y == 1); } }
+};
+
+struct S2
+{
+  constexpr S2() { { struct T { constexpr operator int() { return 1; } }; SA(T() == 1); } }
+};
+
+struct S3
+{
+  constexpr S3() { { typedef enum E { y = 1} EE; SA(EE::y == 1); } }
+};
+
+struct S4
+{
+  constexpr S4() { { typedef struct T { constexpr operator int() { return 1; } } TT; SA(TT() == 1); } }
+};
+
+struct S5
+{
+  constexpr S5() { { using EE = enum E { y = 1}; SA(EE::y == 1); } }
+};
+
+struct S6
+{
+  constexpr S6() { { using TT = struct T { constexpr operator int() { return 1; } }; SA(TT() == 1); } }
+};
+
+S1 s1;
+S2 s2;
+S3 s3;
+S4 s4;
+S5 s5;
+S6 s6;