C++ PATCH for c++/91346 - Implement C++2a P1668R1, allow unevaluated asm in constexpr

This patch implements another C++2a feature, P1668R1: Permit unevaluated inline
asm in constexpr functions.

It's really straightforward so not much to say; we need to allow ASM_EXPRs in
potential_constant_expression_1, but since only unevaluated asm is allowed,
cxx_eval_constant_expression must give an error when it encounters an ASM_EXPR.

I've improved location for "asm", so that the error points to "asm" rather than
to ";".

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2019-08-06  Marek Polacek  <polacek@redhat.com>

	PR c++/91346 - Implement P1668R1, allow unevaluated asm in constexpr.
	* constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
	(potential_constant_expression_1) <case ASM_EXPR>: Allow in C++2a, give
	an error otherwise.
	* cp-tree.h (finish_asm_stmt): Adjust.
	* parser.c (cp_parser_asm_definition): Grab the locaion of "asm" and
	use it.  Adjust an error message.  Allow asm in C++2a.
	* pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
	* semantics.c (finish_asm_stmt): New location_t parameter.  Use it.

	* g++.dg/cpp2a/inline-asm1.C: New test.
	* g++.dg/cpp2a/inline-asm2.C: New test.
	* g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.

Let's downgrade the errors in earlier standard modes to pedwarn. Ok with
that change.

On Tue, Aug 6, 2019, 10:28 AM Marek Polacek <polacek@redhat.com> wrote:

> This patch implements another C++2a feature, P1668R1: Permit unevaluated
> inline
> asm in constexpr functions.
>
> It's really straightforward so not much to say; we need to allow ASM_EXPRs
> in
> potential_constant_expression_1, but since only unevaluated asm is allowed,
> cxx_eval_constant_expression must give an error when it encounters an
> ASM_EXPR.
>
> I've improved location for "asm", so that the error points to "asm" rather
> than
> to ";".
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2019-08-06  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/91346 - Implement P1668R1, allow unevaluated asm in
> constexpr.
>         * constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
>         (potential_constant_expression_1) <case ASM_EXPR>: Allow in C++2a,
> give
>         an error otherwise.
>         * cp-tree.h (finish_asm_stmt): Adjust.
>         * parser.c (cp_parser_asm_definition): Grab the locaion of "asm"
> and
>         use it.  Adjust an error message.  Allow asm in C++2a.
>         * pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
>         * semantics.c (finish_asm_stmt): New location_t parameter.  Use it.
>
>         * g++.dg/cpp2a/inline-asm1.C: New test.
>         * g++.dg/cpp2a/inline-asm2.C: New test.
>         * g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 36a66337433..d45e65df400 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const constexpr_ctx
> *ctx, tree t,
>        r = void_node;
>        break;
>
> +    case ASM_EXPR:
> +      if (!ctx->quiet)
> +       {
> +         error_at (cp_expr_loc_or_input_loc (t),
> +                   "inline assembly is not a constant expression");
> +         inform (cp_expr_loc_or_input_loc (t),
> +                 "only unevaluated inline assembly is allowed in a "
> +                 "%<constexpr%> function in C++2a");
> +       }
> +      *non_constant_p = true;
> +      return t;
> +
>      default:
>        if (STATEMENT_CODE_P (TREE_CODE (t)))
>         {
> @@ -6469,13 +6481,22 @@ potential_constant_expression_1 (tree t, bool
> want_rval, bool strict, bool now,
>        /* GCC internal stuff.  */
>      case VA_ARG_EXPR:
>      case TRANSACTION_EXPR:
> -    case ASM_EXPR:
>      case AT_ENCODE_EXPR:
>      fail:
>        if (flags & tf_error)
>         error_at (loc, "expression %qE is not a constant expression", t);
>        return false;
>
> +    case ASM_EXPR:
> +      if (cxx_dialect >= cxx2a)
> +       /* In C++2a, unevaluated inline assembly is permitted in constexpr
> +          functions.  */
> +       return true;
> +      else if (flags & tf_error)
> +       error_at (loc, "inline assembly is not allowed in %<constexpr%> "
> +                 "functions before C++2a");
> +      return false;
> +
>      case OBJ_TYPE_REF:
>        if (cxx_dialect >= cxx2a)
>         /* In C++2a virtual calls can be constexpr, don't give up yet.  */
> diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
> index d4e67cdfd96..72ee1d61e97 100644
> --- gcc/cp/cp-tree.h
> +++ gcc/cp/cp-tree.h
> @@ -7052,8 +7052,8 @@ enum {
>  extern tree begin_compound_stmt                        (unsigned int);
>
>  extern void finish_compound_stmt               (tree);
> -extern tree finish_asm_stmt                    (int, tree, tree, tree,
> tree,
> -                                                tree, bool);
> +extern tree finish_asm_stmt                    (location_t, int, tree,
> tree,
> +                                                tree, tree, tree, bool);
>  extern tree finish_label_stmt                  (tree);
>  extern void finish_label_decl                  (tree);
>  extern cp_expr finish_parenthesized_expr       (cp_expr);
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 79da7b52eb9..2d0e6a0c931 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -19817,14 +19817,19 @@ cp_parser_asm_definition (cp_parser* parser)
>    bool invalid_inputs_p = false;
>    bool invalid_outputs_p = false;
>    required_token missing = RT_NONE;
> +  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
>
>    /* Look for the `asm' keyword.  */
>    cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
>
>    if (parser->in_function_body
> -      && DECL_DECLARED_CONSTEXPR_P (current_function_decl))
> +      && DECL_DECLARED_CONSTEXPR_P (current_function_decl)
> +      /* In C++2a, unevaluated inline assembly is permitted in constexpr
> +        functions.  */
> +      && (cxx_dialect < cxx2a))
>      {
> -      error ("%<asm%> in %<constexpr%> function");
> +      error_at (asm_loc, "%<asm%> in %<constexpr%> function only
> available "
> +               "with %<-std=c++2a%> or %<-std=gnu++2a%>");
>        cp_function_chain->invalid_constexpr = true;
>      }
>
> @@ -20032,7 +20037,7 @@ cp_parser_asm_definition (cp_parser* parser)
>        /* Create the ASM_EXPR.  */
>        if (parser->in_function_body)
>         {
> -         asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
> +         asm_stmt = finish_asm_stmt (asm_loc, volatile_p, string, outputs,
>                                       inputs, clobbers, labels, inline_p);
>           /* If the extended syntax was not used, mark the ASM_EXPR.  */
>           if (!extended_p)
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index 903e589b663..85cff96c1b6 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -17394,8 +17394,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
> complain, tree in_decl,
>                                                   complain, in_decl);
>         tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
>                                                 complain, in_decl);
> -       tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
> -                              clobbers, labels, ASM_INLINE_P (t));
> +       tmp = finish_asm_stmt (EXPR_LOCATION (t), ASM_VOLATILE_P (t),
> string,
> +                              outputs, inputs, clobbers, labels,
> +                              ASM_INLINE_P (t));
>         tree asm_expr = tmp;
>         if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
>           asm_expr = TREE_OPERAND (asm_expr, 0);
> diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> index fa6962454bf..83dc122e0bd 100644
> --- gcc/cp/semantics.c
> +++ gcc/cp/semantics.c
> @@ -1484,8 +1484,9 @@ finish_compound_stmt (tree stmt)
>     considered volatile, and whether it is asm inline.  */
>
>  tree
> -finish_asm_stmt (int volatile_p, tree string, tree output_operands,
> -                tree input_operands, tree clobbers, tree labels, bool
> inline_p)
> +finish_asm_stmt (location_t loc, int volatile_p, tree string,
> +                tree output_operands, tree input_operands, tree clobbers,
> +                tree labels, bool inline_p)
>  {
>    tree r;
>    tree t;
> @@ -1532,7 +1533,7 @@ finish_asm_stmt (int volatile_p, tree string, tree
> output_operands,
>                      effectively const.  */
>                   || (CLASS_TYPE_P (TREE_TYPE (operand))
>                       && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
> -           cxx_readonly_error (input_location, operand, lv_asm);
> +           cxx_readonly_error (loc, operand, lv_asm);
>
>           tree *op = &operand;
>           while (TREE_CODE (*op) == COMPOUND_EXPR)
> @@ -1585,8 +1586,9 @@ finish_asm_stmt (int volatile_p, tree string, tree
> output_operands,
>              resolve the overloading.  */
>           if (TREE_TYPE (operand) == unknown_type_node)
>             {
> -             error ("type of %<asm%> operand %qE could not be determined",
> -                    TREE_VALUE (t));
> +             error_at (loc,
> +                       "type of %<asm%> operand %qE could not be
> determined",
> +                       TREE_VALUE (t));
>               operand = error_mark_node;
>             }
>
> @@ -1634,7 +1636,7 @@ finish_asm_stmt (int volatile_p, tree string, tree
> output_operands,
>         }
>      }
>
> -  r = build_stmt (input_location, ASM_EXPR, string,
> +  r = build_stmt (loc, ASM_EXPR, string,
>                   output_operands, input_operands,
>                   clobbers, labels);
>    ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
> diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> index ae3dcc69cf0..d82dbada1bf 100644
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> @@ -7,7 +7,7 @@ constexpr int f(int i) {
>    thread_local int l = i;      // { dg-error "thread_local" }
>    goto foo;                    // { dg-error "goto" }
>   foo:
> -  asm("foo");                  // { dg-error "asm" }
> +  asm("foo");                  // { dg-error "asm" "" { target c++17_down
> } }
>    int k;                       // { dg-error "uninitialized" }
>    A a;                         // { dg-error "non-literal" }
>    return i;
> diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
> gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
> new file mode 100644
> index 00000000000..9ac588f0ecc
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
> @@ -0,0 +1,12 @@
> +// P1668R1: Permit unevaluated inline asm in constexpr functions
> +// PR c++/91346
> +// { dg-do compile { target c++11 } }
> +
> +constexpr int
> +foo (int a, int b)
> +{
> +  if (__builtin_is_constant_evaluated ())
> +    return a + b;
> +  asm (""); // { dg-error ".asm. in .constexpr. function only available
> with" "" { target c++17_down } }
> +  return a;
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
> gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
> new file mode 100644
> index 00000000000..6038c111eb0
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
> @@ -0,0 +1,17 @@
> +// P1668R1: Permit unevaluated inline asm in constexpr functions
> +// PR c++/91346
> +// { dg-do compile { target c++2a } }
> +
> +constexpr int
> +foo (bool b)
> +{
> +  if (b)
> +    return 42;
> +  asm (""); // { dg-error "inline assembly is not a constant expression" }
> +// { dg-message "only unevaluated inline assembly" "" { target *-*-* }
> .-1 }
> +  return -1;
> +}
> +
> +constexpr int i = foo (true);
> +static_assert(i == 42, "");
> +constexpr int j = foo (false); // { dg-message "in .constexpr. expansion
> of" }
>

On Tue, Aug 06, 2019 at 09:25:45PM -0400, Jason Merrill wrote:
> Let's downgrade the errors in earlier standard modes to pedwarn. Ok with
> that change.

Works for me, here's what I'll apply once it passes testing.

I removed the diagnostic in potential_constant_expression_1/ASM_EXPR so that
we don't generate duplicate pedwarns for the same thing.  Hope that's OK.

2019-08-07  Marek Polacek  <polacek@redhat.com>

	PR c++/91346 - Implement P1668R1, allow unevaluated asm in constexpr.
	* constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
	(potential_constant_expression_1) <case ASM_EXPR>: Allow.
	* cp-tree.h (finish_asm_stmt): Adjust.
	* parser.c (cp_parser_asm_definition): Grab the locaion of "asm" and
	use it.  Change an error to a pedwarn.  Allow asm in C++2a, warn
	otherwise.
	* pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
	* semantics.c (finish_asm_stmt): New location_t parameter.  Use it.

	* g++.dg/cpp2a/inline-asm1.C: New test.
	* g++.dg/cpp2a/inline-asm2.C: New test.
	* g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 36a66337433..e86b0789b84 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       r = void_node;
       break;
 
+    case ASM_EXPR:
+      if (!ctx->quiet)
+	{
+	  error_at (cp_expr_loc_or_input_loc (t),
+		    "inline assembly is not a constant expression");
+	  inform (cp_expr_loc_or_input_loc (t),
+		  "only unevaluated inline assembly is allowed in a "
+		  "%<constexpr%> function in C++2a");
+	}
+      *non_constant_p = true;
+      return t;
+
     default:
       if (STATEMENT_CODE_P (TREE_CODE (t)))
 	{
@@ -6469,13 +6481,18 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
       /* GCC internal stuff.  */
     case VA_ARG_EXPR:
     case TRANSACTION_EXPR:
-    case ASM_EXPR:
     case AT_ENCODE_EXPR:
     fail:
       if (flags & tf_error)
 	error_at (loc, "expression %qE is not a constant expression", t);
       return false;
 
+    case ASM_EXPR:
+      /* In C++2a, unevaluated inline assembly is permitted in constexpr
+	 functions.  If it's used in earlier standard modes, we pedwarn in
+	 cp_parser_asm_definition.  */
+      return true;
+
     case OBJ_TYPE_REF:
       if (cxx_dialect >= cxx2a)
 	/* In C++2a virtual calls can be constexpr, don't give up yet.  */
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index d4e67cdfd96..72ee1d61e97 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -7052,8 +7052,8 @@ enum {
 extern tree begin_compound_stmt			(unsigned int);
 
 extern void finish_compound_stmt		(tree);
-extern tree finish_asm_stmt			(int, tree, tree, tree, tree,
-						 tree, bool);
+extern tree finish_asm_stmt			(location_t, int, tree, tree,
+						 tree, tree, tree, bool);
 extern tree finish_label_stmt			(tree);
 extern void finish_label_decl			(tree);
 extern cp_expr finish_parenthesized_expr	(cp_expr);
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 4d07a6a3011..ccf89f0856f 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -19817,16 +19817,18 @@ cp_parser_asm_definition (cp_parser* parser)
   bool invalid_inputs_p = false;
   bool invalid_outputs_p = false;
   required_token missing = RT_NONE;
+  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
 
   /* Look for the `asm' keyword.  */
   cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
 
+  /* In C++2a, unevaluated inline assembly is permitted in constexpr
+     functions.  */
   if (parser->in_function_body
-      && DECL_DECLARED_CONSTEXPR_P (current_function_decl))
-    {
-      error ("%<asm%> in %<constexpr%> function");
-      cp_function_chain->invalid_constexpr = true;
-    }
+      && DECL_DECLARED_CONSTEXPR_P (current_function_decl)
+      && (cxx_dialect < cxx2a))
+    pedwarn (asm_loc, 0, "%<asm%> in %<constexpr%> function only available "
+	     "with %<-std=c++2a%> or %<-std=gnu++2a%>");
 
   /* Handle the asm-qualifier-list.  */
   location_t volatile_loc = UNKNOWN_LOCATION;
@@ -20032,7 +20034,7 @@ cp_parser_asm_definition (cp_parser* parser)
       /* Create the ASM_EXPR.  */
       if (parser->in_function_body)
 	{
-	  asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
+	  asm_stmt = finish_asm_stmt (asm_loc, volatile_p, string, outputs,
 				      inputs, clobbers, labels, inline_p);
 	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
 	  if (!extended_p)
diff --git gcc/cp/pt.c gcc/cp/pt.c
index b1ad99d1481..b03968febb4 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -17396,8 +17396,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
 	 					  complain, in_decl);
 	tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
 						complain, in_decl);
-	tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
-			       clobbers, labels, ASM_INLINE_P (t));
+	tmp = finish_asm_stmt (EXPR_LOCATION (t), ASM_VOLATILE_P (t), string,
+			       outputs, inputs, clobbers, labels,
+			       ASM_INLINE_P (t));
 	tree asm_expr = tmp;
 	if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
 	  asm_expr = TREE_OPERAND (asm_expr, 0);
diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index 77e7a6dced2..8fe632f2239 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -1484,8 +1484,9 @@ finish_compound_stmt (tree stmt)
    considered volatile, and whether it is asm inline.  */
 
 tree
-finish_asm_stmt (int volatile_p, tree string, tree output_operands,
-		 tree input_operands, tree clobbers, tree labels, bool inline_p)
+finish_asm_stmt (location_t loc, int volatile_p, tree string,
+		 tree output_operands, tree input_operands, tree clobbers,
+		 tree labels, bool inline_p)
 {
   tree r;
   tree t;
@@ -1532,7 +1533,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
 		     effectively const.  */
 		  || (CLASS_TYPE_P (TREE_TYPE (operand))
 		      && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
-	    cxx_readonly_error (input_location, operand, lv_asm);
+	    cxx_readonly_error (loc, operand, lv_asm);
 
 	  tree *op = &operand;
 	  while (TREE_CODE (*op) == COMPOUND_EXPR)
@@ -1585,8 +1586,9 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
 	     resolve the overloading.  */
 	  if (TREE_TYPE (operand) == unknown_type_node)
 	    {
-	      error ("type of %<asm%> operand %qE could not be determined",
-		     TREE_VALUE (t));
+	      error_at (loc,
+			"type of %<asm%> operand %qE could not be determined",
+			TREE_VALUE (t));
 	      operand = error_mark_node;
 	    }
 
@@ -1634,7 +1636,7 @@ finish_asm_stmt (int volatile_p, tree string, tree output_operands,
 	}
     }
 
-  r = build_stmt (input_location, ASM_EXPR, string,
+  r = build_stmt (loc, ASM_EXPR, string,
 		  output_operands, input_operands,
 		  clobbers, labels);
   ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
index ae3dcc69cf0..d82dbada1bf 100644
--- gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
@@ -7,7 +7,7 @@ constexpr int f(int i) {
   thread_local int l = i;	// { dg-error "thread_local" }
   goto foo;			// { dg-error "goto" }
  foo:
-  asm("foo");			// { dg-error "asm" }
+  asm("foo");			// { dg-error "asm" "" { target c++17_down } }
   int k;			// { dg-error "uninitialized" }
   A a;				// { dg-error "non-literal" }
   return i;
diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm1.C gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
new file mode 100644
index 00000000000..a7835c7199e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
@@ -0,0 +1,13 @@
+// P1668R1: Permit unevaluated inline asm in constexpr functions
+// PR c++/91346
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+constexpr int
+foo (int a, int b)
+{
+  if (__builtin_is_constant_evaluated ())
+    return a + b;
+  asm (""); // { dg-warning ".asm. in .constexpr. function only available with" "" { target c++17_down } }
+  return a;
+}
diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm2.C gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
new file mode 100644
index 00000000000..6038c111eb0
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
@@ -0,0 +1,17 @@
+// P1668R1: Permit unevaluated inline asm in constexpr functions
+// PR c++/91346
+// { dg-do compile { target c++2a } }
+
+constexpr int
+foo (bool b)
+{
+  if (b)
+    return 42;
+  asm (""); // { dg-error "inline assembly is not a constant expression" }
+// { dg-message "only unevaluated inline assembly" "" { target *-*-* } .-1 }
+  return -1;
+}
+
+constexpr int i = foo (true);
+static_assert(i == 42, "");
+constexpr int j = foo (false); // { dg-message "in .constexpr. expansion of" }

On Wed, Aug 7, 2019, 9:11 AM Marek Polacek <polacek@redhat.com> wrote:

> On Tue, Aug 06, 2019 at 09:25:45PM -0400, Jason Merrill wrote:
> > Let's downgrade the errors in earlier standard modes to pedwarn. Ok with
> > that change.
>
> Works for me, here's what I'll apply once it passes testing.
>
> I removed the diagnostic in potential_constant_expression_1/ASM_EXPR so
> that
> we don't generate duplicate pedwarns for the same thing.  Hope that's OK.
>
> 2019-08-07  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/91346 - Implement P1668R1, allow unevaluated asm in
> constexpr.
>         * constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
>         (potential_constant_expression_1) <case ASM_EXPR>: Allow.
>         * cp-tree.h (finish_asm_stmt): Adjust.
>         * parser.c (cp_parser_asm_definition): Grab the locaion of "asm"
> and
>         use it.  Change an error to a pedwarn.  Allow asm in C++2a, warn
>         otherwise.
>         * pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
>         * semantics.c (finish_asm_stmt): New location_t parameter.  Use it.
>
>         * g++.dg/cpp2a/inline-asm1.C: New test.
>         * g++.dg/cpp2a/inline-asm2.C: New test.
>         * g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 36a66337433..e86b0789b84 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const constexpr_ctx
> *ctx, tree t,
>        r = void_node;
>        break;
>
> +    case ASM_EXPR:
> +      if (!ctx->quiet)
> +       {
> +         error_at (cp_expr_loc_or_input_loc (t),
> +                   "inline assembly is not a constant expression");
> +         inform (cp_expr_loc_or_input_loc (t),
> +                 "only unevaluated inline assembly is allowed in a "
> +                 "%<constexpr%> function in C++2a");
> +       }
> +      *non_constant_p = true;
> +      return t;
> +
>      default:
>        if (STATEMENT_CODE_P (TREE_CODE (t)))
>         {
> @@ -6469,13 +6481,18 @@ potential_constant_expression_1 (tree t, bool
> want_rval, bool strict, bool now,
>        /* GCC internal stuff.  */
>      case VA_ARG_EXPR:
>      case TRANSACTION_EXPR:
> -    case ASM_EXPR:
>      case AT_ENCODE_EXPR:
>      fail:
>        if (flags & tf_error)
>         error_at (loc, "expression %qE is not a constant expression", t);
>        return false;
>
> +    case ASM_EXPR:
> +      /* In C++2a, unevaluated inline assembly is permitted in constexpr
> +        functions.  If it's used in earlier standard modes, we pedwarn in
> +        cp_parser_asm_definition.  */
> +      return true;
>

Actually, do we need this change? If it's (possibly) unevaluated, we
shouldn't get here.

     case OBJ_TYPE_REF:
>        if (cxx_dialect >= cxx2a)
>         /* In C++2a virtual calls can be constexpr, don't give up yet.  */
> diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
> index d4e67cdfd96..72ee1d61e97 100644
> --- gcc/cp/cp-tree.h
> +++ gcc/cp/cp-tree.h
> @@ -7052,8 +7052,8 @@ enum {
>  extern tree begin_compound_stmt                        (unsigned int);
>
>  extern void finish_compound_stmt               (tree);
> -extern tree finish_asm_stmt                    (int, tree, tree, tree,
> tree,
> -                                                tree, bool);
> +extern tree finish_asm_stmt                    (location_t, int, tree,
> tree,
> +                                                tree, tree, tree, bool);
>  extern tree finish_label_stmt                  (tree);
>  extern void finish_label_decl                  (tree);
>  extern cp_expr finish_parenthesized_expr       (cp_expr);
> diff --git gcc/cp/parser.c gcc/cp/parser.c
> index 4d07a6a3011..ccf89f0856f 100644
> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -19817,16 +19817,18 @@ cp_parser_asm_definition (cp_parser* parser)
>    bool invalid_inputs_p = false;
>    bool invalid_outputs_p = false;
>    required_token missing = RT_NONE;
> +  location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
>
>    /* Look for the `asm' keyword.  */
>    cp_parser_require_keyword (parser, RID_ASM, RT_ASM);
>
> +  /* In C++2a, unevaluated inline assembly is permitted in constexpr
> +     functions.  */
>    if (parser->in_function_body
> -      && DECL_DECLARED_CONSTEXPR_P (current_function_decl))
> -    {
> -      error ("%<asm%> in %<constexpr%> function");
> -      cp_function_chain->invalid_constexpr = true;
> -    }
> +      && DECL_DECLARED_CONSTEXPR_P (current_function_decl)
> +      && (cxx_dialect < cxx2a))
> +    pedwarn (asm_loc, 0, "%<asm%> in %<constexpr%> function only
> available "
> +            "with %<-std=c++2a%> or %<-std=gnu++2a%>");
>
>    /* Handle the asm-qualifier-list.  */
>    location_t volatile_loc = UNKNOWN_LOCATION;
> @@ -20032,7 +20034,7 @@ cp_parser_asm_definition (cp_parser* parser)
>        /* Create the ASM_EXPR.  */
>        if (parser->in_function_body)
>         {
> -         asm_stmt = finish_asm_stmt (volatile_p, string, outputs,
> +         asm_stmt = finish_asm_stmt (asm_loc, volatile_p, string, outputs,
>                                       inputs, clobbers, labels, inline_p);
>           /* If the extended syntax was not used, mark the ASM_EXPR.  */
>           if (!extended_p)
> diff --git gcc/cp/pt.c gcc/cp/pt.c
> index b1ad99d1481..b03968febb4 100644
> --- gcc/cp/pt.c
> +++ gcc/cp/pt.c
> @@ -17396,8 +17396,9 @@ tsubst_expr (tree t, tree args, tsubst_flags_t
> complain, tree in_decl,
>                                                   complain, in_decl);
>         tree labels = tsubst_copy_asm_operands (ASM_LABELS (t), args,
>                                                 complain, in_decl);
> -       tmp = finish_asm_stmt (ASM_VOLATILE_P (t), string, outputs, inputs,
> -                              clobbers, labels, ASM_INLINE_P (t));
> +       tmp = finish_asm_stmt (EXPR_LOCATION (t), ASM_VOLATILE_P (t),
> string,
> +                              outputs, inputs, clobbers, labels,
> +                              ASM_INLINE_P (t));
>         tree asm_expr = tmp;
>         if (TREE_CODE (asm_expr) == CLEANUP_POINT_EXPR)
>           asm_expr = TREE_OPERAND (asm_expr, 0);
> diff --git gcc/cp/semantics.c gcc/cp/semantics.c
> index 77e7a6dced2..8fe632f2239 100644
> --- gcc/cp/semantics.c
> +++ gcc/cp/semantics.c
> @@ -1484,8 +1484,9 @@ finish_compound_stmt (tree stmt)
>     considered volatile, and whether it is asm inline.  */
>
>  tree
> -finish_asm_stmt (int volatile_p, tree string, tree output_operands,
> -                tree input_operands, tree clobbers, tree labels, bool
> inline_p)
> +finish_asm_stmt (location_t loc, int volatile_p, tree string,
> +                tree output_operands, tree input_operands, tree clobbers,
> +                tree labels, bool inline_p)
>  {
>    tree r;
>    tree t;
> @@ -1532,7 +1533,7 @@ finish_asm_stmt (int volatile_p, tree string, tree
> output_operands,
>                      effectively const.  */
>                   || (CLASS_TYPE_P (TREE_TYPE (operand))
>                       && C_TYPE_FIELDS_READONLY (TREE_TYPE (operand)))))
> -           cxx_readonly_error (input_location, operand, lv_asm);
> +           cxx_readonly_error (loc, operand, lv_asm);
>
>           tree *op = &operand;
>           while (TREE_CODE (*op) == COMPOUND_EXPR)
> @@ -1585,8 +1586,9 @@ finish_asm_stmt (int volatile_p, tree string, tree
> output_operands,
>              resolve the overloading.  */
>           if (TREE_TYPE (operand) == unknown_type_node)
>             {
> -             error ("type of %<asm%> operand %qE could not be determined",
> -                    TREE_VALUE (t));
> +             error_at (loc,
> +                       "type of %<asm%> operand %qE could not be
> determined",
> +                       TREE_VALUE (t));
>               operand = error_mark_node;
>             }
>
> @@ -1634,7 +1636,7 @@ finish_asm_stmt (int volatile_p, tree string, tree
> output_operands,
>         }
>      }
>
> -  r = build_stmt (input_location, ASM_EXPR, string,
> +  r = build_stmt (loc, ASM_EXPR, string,
>                   output_operands, input_operands,
>                   clobbers, labels);
>    ASM_VOLATILE_P (r) = volatile_p || noutputs == 0;
> diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> index ae3dcc69cf0..d82dbada1bf 100644
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-neg1.C
> @@ -7,7 +7,7 @@ constexpr int f(int i) {
>    thread_local int l = i;      // { dg-error "thread_local" }
>    goto foo;                    // { dg-error "goto" }
>   foo:
> -  asm("foo");                  // { dg-error "asm" }
> +  asm("foo");                  // { dg-error "asm" "" { target c++17_down
> } }
>    int k;                       // { dg-error "uninitialized" }
>    A a;                         // { dg-error "non-literal" }
>    return i;
> diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
> gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
> new file mode 100644
> index 00000000000..a7835c7199e
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/inline-asm1.C
> @@ -0,0 +1,13 @@
> +// P1668R1: Permit unevaluated inline asm in constexpr functions
> +// PR c++/91346
> +// { dg-do compile { target c++14 } }
> +// { dg-options "" }
> +
> +constexpr int
> +foo (int a, int b)
> +{
> +  if (__builtin_is_constant_evaluated ())
> +    return a + b;
> +  asm (""); // { dg-warning ".asm. in .constexpr. function only available
> with" "" { target c++17_down } }
> +  return a;
> +}
> diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
> gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
> new file mode 100644
> index 00000000000..6038c111eb0
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp2a/inline-asm2.C
> @@ -0,0 +1,17 @@
> +// P1668R1: Permit unevaluated inline asm in constexpr functions
> +// PR c++/91346
> +// { dg-do compile { target c++2a } }
> +
> +constexpr int
> +foo (bool b)
> +{
> +  if (b)
> +    return 42;
> +  asm (""); // { dg-error "inline assembly is not a constant expression" }
> +// { dg-message "only unevaluated inline assembly" "" { target *-*-* }
> .-1 }
> +  return -1;
> +}
> +
> +constexpr int i = foo (true);
> +static_assert(i == 42, "");
> +constexpr int j = foo (false); // { dg-message "in .constexpr. expansion
> of" }
>

On Wed, Aug 07, 2019 at 12:23:25PM -0400, Jason Merrill wrote:
> On Wed, Aug 7, 2019, 9:11 AM Marek Polacek <polacek@redhat.com> wrote:
> 
> > On Tue, Aug 06, 2019 at 09:25:45PM -0400, Jason Merrill wrote:
> > > Let's downgrade the errors in earlier standard modes to pedwarn. Ok with
> > > that change.
> >
> > Works for me, here's what I'll apply once it passes testing.
> >
> > I removed the diagnostic in potential_constant_expression_1/ASM_EXPR so
> > that
> > we don't generate duplicate pedwarns for the same thing.  Hope that's OK.
> >
> > 2019-08-07  Marek Polacek  <polacek@redhat.com>
> >
> >         PR c++/91346 - Implement P1668R1, allow unevaluated asm in
> > constexpr.
> >         * constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
> >         (potential_constant_expression_1) <case ASM_EXPR>: Allow.
> >         * cp-tree.h (finish_asm_stmt): Adjust.
> >         * parser.c (cp_parser_asm_definition): Grab the locaion of "asm"
> > and
> >         use it.  Change an error to a pedwarn.  Allow asm in C++2a, warn
> >         otherwise.
> >         * pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
> >         * semantics.c (finish_asm_stmt): New location_t parameter.  Use it.
> >
> >         * g++.dg/cpp2a/inline-asm1.C: New test.
> >         * g++.dg/cpp2a/inline-asm2.C: New test.
> >         * g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.
> >
> > diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> > index 36a66337433..e86b0789b84 100644
> > --- gcc/cp/constexpr.c
> > +++ gcc/cp/constexpr.c
> > @@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const constexpr_ctx
> > *ctx, tree t,
> >        r = void_node;
> >        break;
> >
> > +    case ASM_EXPR:
> > +      if (!ctx->quiet)
> > +       {
> > +         error_at (cp_expr_loc_or_input_loc (t),
> > +                   "inline assembly is not a constant expression");
> > +         inform (cp_expr_loc_or_input_loc (t),
> > +                 "only unevaluated inline assembly is allowed in a "
> > +                 "%<constexpr%> function in C++2a");
> > +       }
> > +      *non_constant_p = true;
> > +      return t;
> > +
> >      default:
> >        if (STATEMENT_CODE_P (TREE_CODE (t)))
> >         {
> > @@ -6469,13 +6481,18 @@ potential_constant_expression_1 (tree t, bool
> > want_rval, bool strict, bool now,
> >        /* GCC internal stuff.  */
> >      case VA_ARG_EXPR:
> >      case TRANSACTION_EXPR:
> > -    case ASM_EXPR:
> >      case AT_ENCODE_EXPR:
> >      fail:
> >        if (flags & tf_error)
> >         error_at (loc, "expression %qE is not a constant expression", t);
> >        return false;
> >
> > +    case ASM_EXPR:
> > +      /* In C++2a, unevaluated inline assembly is permitted in constexpr
> > +        functions.  If it's used in earlier standard modes, we pedwarn in
> > +        cp_parser_asm_definition.  */
> > +      return true;
> >
> 
> Actually, do we need this change? If it's (possibly) unevaluated, we
> shouldn't get here.

We can get here when using asm() in ({ }) like this (ugh):

constexpr int
foo (bool b)
{
  if (b)
   {
     constexpr int i = ({ asm(""); 42; });
     return i;
    }
  else
    return 42;
}
static_assert(foo(false) == 42, "");

With the current state of potential_constant_expression_1, we generate

inline-asm3.C: In function ‘constexpr int foo(bool)’:
inline-asm3.C:10:27: error: inline assembly is not a constant expression
   10 |      constexpr int i = ({ asm(""); 42; });
      |                           ^~~
inline-asm3.C:10:27: note: only unevaluated inline assembly is allowed in a ‘constexpr’ function in C++2a

which I thought was better than what we emit with the hunk revered:

inline-asm3.C: In function ‘constexpr int foo(bool)’:
inline-asm3.C:10:27: error: expression ‘<statement>’ is not a constant expression
   10 |      constexpr int i = ({ asm(""); 42; });
      |                           ^~~

But I'm happy to revert that hunk if you want.

Marek

On Wed, Aug 7, 2019, 2:35 PM Marek Polacek <polacek@redhat.com> wrote:

> On Wed, Aug 07, 2019 at 12:23:25PM -0400, Jason Merrill wrote:
> > On Wed, Aug 7, 2019, 9:11 AM Marek Polacek <polacek@redhat.com> wrote:
> >
> > > On Tue, Aug 06, 2019 at 09:25:45PM -0400, Jason Merrill wrote:
> > > > Let's downgrade the errors in earlier standard modes to pedwarn. Ok
> with
> > > > that change.
> > >
> > > Works for me, here's what I'll apply once it passes testing.
> > >
> > > I removed the diagnostic in potential_constant_expression_1/ASM_EXPR so
> > > that
> > > we don't generate duplicate pedwarns for the same thing.  Hope that's
> OK.
> > >
> > > 2019-08-07  Marek Polacek  <polacek@redhat.com>
> > >
> > >         PR c++/91346 - Implement P1668R1, allow unevaluated asm in
> > > constexpr.
> > >         * constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
> > >         (potential_constant_expression_1) <case ASM_EXPR>: Allow.
> > >         * cp-tree.h (finish_asm_stmt): Adjust.
> > >         * parser.c (cp_parser_asm_definition): Grab the locaion of
> "asm"
> > > and
> > >         use it.  Change an error to a pedwarn.  Allow asm in C++2a,
> warn
> > >         otherwise.
> > >         * pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
> > >         * semantics.c (finish_asm_stmt): New location_t parameter.
> Use it.
> > >
> > >         * g++.dg/cpp2a/inline-asm1.C: New test.
> > >         * g++.dg/cpp2a/inline-asm2.C: New test.
> > >         * g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.
> > >
> > > diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> > > index 36a66337433..e86b0789b84 100644
> > > --- gcc/cp/constexpr.c
> > > +++ gcc/cp/constexpr.c
> > > @@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const
> constexpr_ctx
> > > *ctx, tree t,
> > >        r = void_node;
> > >        break;
> > >
> > > +    case ASM_EXPR:
> > > +      if (!ctx->quiet)
> > > +       {
> > > +         error_at (cp_expr_loc_or_input_loc (t),
> > > +                   "inline assembly is not a constant expression");
> > > +         inform (cp_expr_loc_or_input_loc (t),
> > > +                 "only unevaluated inline assembly is allowed in a "
> > > +                 "%<constexpr%> function in C++2a");
> > > +       }
> > > +      *non_constant_p = true;
> > > +      return t;
> > > +
> > >      default:
> > >        if (STATEMENT_CODE_P (TREE_CODE (t)))
> > >         {
> > > @@ -6469,13 +6481,18 @@ potential_constant_expression_1 (tree t, bool
> > > want_rval, bool strict, bool now,
> > >        /* GCC internal stuff.  */
> > >      case VA_ARG_EXPR:
> > >      case TRANSACTION_EXPR:
> > > -    case ASM_EXPR:
> > >      case AT_ENCODE_EXPR:
> > >      fail:
> > >        if (flags & tf_error)
> > >         error_at (loc, "expression %qE is not a constant expression",
> t);
> > >        return false;
> > >
> > > +    case ASM_EXPR:
> > > +      /* In C++2a, unevaluated inline assembly is permitted in
> constexpr
> > > +        functions.  If it's used in earlier standard modes, we
> pedwarn in
> > > +        cp_parser_asm_definition.  */
> > > +      return true;
> > >
> >
> > Actually, do we need this change? If it's (possibly) unevaluated, we
> > shouldn't get here.
>
> We can get here when using asm() in ({ }) like this (ugh):
>
> constexpr int
> foo (bool b)
> {
>   if (b)
>    {
>      constexpr int i = ({ asm(""); 42; });
>      return i;
>     }
>   else
>     return 42;
> }
> static_assert(foo(false) == 42, "");
>
> With the current state of potential_constant_expression_1, we generate
>
> inline-asm3.C: In function ‘constexpr int foo(bool)’:
> inline-asm3.C:10:27: error: inline assembly is not a constant expression
>    10 |      constexpr int i = ({ asm(""); 42; });
>       |                           ^~~
> inline-asm3.C:10:27: note: only unevaluated inline assembly is allowed in
> a ‘constexpr’ function in C++2a
>
> which I thought was better than what we emit with the hunk revered:
>
> inline-asm3.C: In function ‘constexpr int foo(bool)’:
> inline-asm3.C:10:27: error: expression ‘<statement>’ is not a constant
> expression
>    10 |      constexpr int i = ({ asm(""); 42; });
>       |                           ^~~
>
> But I'm happy to revert that hunk if you want.
>

Sure, perhaps we could share the error message between the two functions.
In general it's better to reject something in potential_... if we can.

>

On Wed, Aug 07, 2019 at 03:38:02PM -0400, Jason Merrill wrote:
> On Wed, Aug 7, 2019, 2:35 PM Marek Polacek <polacek@redhat.com> wrote:
> 
> > On Wed, Aug 07, 2019 at 12:23:25PM -0400, Jason Merrill wrote:
> > > On Wed, Aug 7, 2019, 9:11 AM Marek Polacek <polacek@redhat.com> wrote:
> > >
> > > > On Tue, Aug 06, 2019 at 09:25:45PM -0400, Jason Merrill wrote:
> > > > > Let's downgrade the errors in earlier standard modes to pedwarn. Ok
> > with
> > > > > that change.
> > > >
> > > > Works for me, here's what I'll apply once it passes testing.
> > > >
> > > > I removed the diagnostic in potential_constant_expression_1/ASM_EXPR so
> > > > that
> > > > we don't generate duplicate pedwarns for the same thing.  Hope that's
> > OK.
> > > >
> > > > 2019-08-07  Marek Polacek  <polacek@redhat.com>
> > > >
> > > >         PR c++/91346 - Implement P1668R1, allow unevaluated asm in
> > > > constexpr.
> > > >         * constexpr.c (cxx_eval_constant_expression): Handle ASM_EXPR.
> > > >         (potential_constant_expression_1) <case ASM_EXPR>: Allow.
> > > >         * cp-tree.h (finish_asm_stmt): Adjust.
> > > >         * parser.c (cp_parser_asm_definition): Grab the locaion of
> > "asm"
> > > > and
> > > >         use it.  Change an error to a pedwarn.  Allow asm in C++2a,
> > warn
> > > >         otherwise.
> > > >         * pt.c (tsubst_expr): Pass a location down to finish_asm_stmt.
> > > >         * semantics.c (finish_asm_stmt): New location_t parameter.
> > Use it.
> > > >
> > > >         * g++.dg/cpp2a/inline-asm1.C: New test.
> > > >         * g++.dg/cpp2a/inline-asm2.C: New test.
> > > >         * g++.dg/cpp1y/constexpr-neg1.C: Adjust dg-error.
> > > >
> > > > diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> > > > index 36a66337433..e86b0789b84 100644
> > > > --- gcc/cp/constexpr.c
> > > > +++ gcc/cp/constexpr.c
> > > > @@ -5289,6 +5289,18 @@ cxx_eval_constant_expression (const
> > constexpr_ctx
> > > > *ctx, tree t,
> > > >        r = void_node;
> > > >        break;
> > > >
> > > > +    case ASM_EXPR:
> > > > +      if (!ctx->quiet)
> > > > +       {
> > > > +         error_at (cp_expr_loc_or_input_loc (t),
> > > > +                   "inline assembly is not a constant expression");
> > > > +         inform (cp_expr_loc_or_input_loc (t),
> > > > +                 "only unevaluated inline assembly is allowed in a "
> > > > +                 "%<constexpr%> function in C++2a");
> > > > +       }
> > > > +      *non_constant_p = true;
> > > > +      return t;
> > > > +
> > > >      default:
> > > >        if (STATEMENT_CODE_P (TREE_CODE (t)))
> > > >         {
> > > > @@ -6469,13 +6481,18 @@ potential_constant_expression_1 (tree t, bool
> > > > want_rval, bool strict, bool now,
> > > >        /* GCC internal stuff.  */
> > > >      case VA_ARG_EXPR:
> > > >      case TRANSACTION_EXPR:
> > > > -    case ASM_EXPR:
> > > >      case AT_ENCODE_EXPR:
> > > >      fail:
> > > >        if (flags & tf_error)
> > > >         error_at (loc, "expression %qE is not a constant expression",
> > t);
> > > >        return false;
> > > >
> > > > +    case ASM_EXPR:
> > > > +      /* In C++2a, unevaluated inline assembly is permitted in
> > constexpr
> > > > +        functions.  If it's used in earlier standard modes, we
> > pedwarn in
> > > > +        cp_parser_asm_definition.  */
> > > > +      return true;
> > > >
> > >
> > > Actually, do we need this change? If it's (possibly) unevaluated, we
> > > shouldn't get here.
> >
> > We can get here when using asm() in ({ }) like this (ugh):
> >
> > constexpr int
> > foo (bool b)
> > {
> >   if (b)
> >    {
> >      constexpr int i = ({ asm(""); 42; });
> >      return i;
> >     }
> >   else
> >     return 42;
> > }
> > static_assert(foo(false) == 42, "");
> >
> > With the current state of potential_constant_expression_1, we generate
> >
> > inline-asm3.C: In function ‘constexpr int foo(bool)’:
> > inline-asm3.C:10:27: error: inline assembly is not a constant expression
> >    10 |      constexpr int i = ({ asm(""); 42; });
> >       |                           ^~~
> > inline-asm3.C:10:27: note: only unevaluated inline assembly is allowed in
> > a ‘constexpr’ function in C++2a
> >
> > which I thought was better than what we emit with the hunk revered:
> >
> > inline-asm3.C: In function ‘constexpr int foo(bool)’:
> > inline-asm3.C:10:27: error: expression ‘<statement>’ is not a constant
> > expression
> >    10 |      constexpr int i = ({ asm(""); 42; });
> >       |                           ^~~
> >
> > But I'm happy to revert that hunk if you want.
> >
> 
> Sure, perhaps we could share the error message between the two functions.
> In general it's better to reject something in potential_... if we can.

Yeah, that makes sense.  How about this further tweak then?

2019-08-07  Marek Polacek  <polacek@redhat.com>

	* constexpr.c (inline_asm_in_constexpr_error): New.
	(cxx_eval_constant_expression) <case ASM_EXPR>: Call it.
	(potential_constant_expression_1) <case ASM_EXPR>: Likewise.

	* g++.dg/cpp2a/inline-asm3.C: New test.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index e86b0789b84..63c890560e0 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -4411,6 +4411,16 @@ lookup_placeholder (const constexpr_ctx *ctx, bool lval, tree type)
   return ob;
 }
 
+/* Complain about an attempt to evaluate inline assembly.  */
+
+static void
+inline_asm_in_constexpr_error (location_t loc)
+{
+  error_at (loc, "inline assembly is not a constant expression");
+  inform (loc, "only unevaluated inline assembly is allowed in a "
+	  "%<constexpr%> function in C++2a");
+}
+
 /* Attempt to reduce the expression T to a constant value.
    On failure, issue diagnostic and return error_mark_node.  */
 /* FIXME unify with c_fully_fold */
@@ -5291,13 +5301,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
 
     case ASM_EXPR:
       if (!ctx->quiet)
-	{
-	  error_at (cp_expr_loc_or_input_loc (t),
-		    "inline assembly is not a constant expression");
-	  inform (cp_expr_loc_or_input_loc (t),
-		  "only unevaluated inline assembly is allowed in a "
-		  "%<constexpr%> function in C++2a");
-	}
+	inline_asm_in_constexpr_error (cp_expr_loc_or_input_loc (t));
       *non_constant_p = true;
       return t;
 
@@ -6488,10 +6492,9 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
       return false;
 
     case ASM_EXPR:
-      /* In C++2a, unevaluated inline assembly is permitted in constexpr
-	 functions.  If it's used in earlier standard modes, we pedwarn in
-	 cp_parser_asm_definition.  */
-      return true;
+      if (flags & tf_error)
+	inline_asm_in_constexpr_error (loc);
+      return false;
 
     case OBJ_TYPE_REF:
       if (cxx_dialect >= cxx2a)
diff --git gcc/testsuite/g++.dg/cpp2a/inline-asm3.C gcc/testsuite/g++.dg/cpp2a/inline-asm3.C
new file mode 100644
index 00000000000..b636e3b2d87
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp2a/inline-asm3.C
@@ -0,0 +1,12 @@
+// P1668R1: Permit unevaluated inline asm in constexpr functions
+// { dg-do compile { target c++2a } }
+// { dg-additional-options "-Wno-pedantic" }
+
+constexpr int
+foo ()
+{
+ constexpr int i = ({ asm(""); 42; }); // { dg-error "inline assembly is not a constant expression" }
+ return i;
+}
+
+constexpr int i = foo ();

On Wed, Aug 07, 2019 at 04:30:29PM -0400, Marek Polacek wrote:
> +/* Complain about an attempt to evaluate inline assembly.  */
> +
> +static void
> +inline_asm_in_constexpr_error (location_t loc)
> +{
> +  error_at (loc, "inline assembly is not a constant expression");
> +  inform (loc, "only unevaluated inline assembly is allowed in a "
> +	  "%<constexpr%> function in C++2a");

Note, I think in this case you do want a
  auto_diagnostic_group d;
before the two diagnostic calls, whether you apply this patch or not,
because they are a diagnostic group together.  See PR84889.

	Jakub

On Wed, Aug 07, 2019 at 10:37:04PM +0200, Jakub Jelinek wrote:
> On Wed, Aug 07, 2019 at 04:30:29PM -0400, Marek Polacek wrote:
> > +/* Complain about an attempt to evaluate inline assembly.  */
> > +
> > +static void
> > +inline_asm_in_constexpr_error (location_t loc)
> > +{
> > +  error_at (loc, "inline assembly is not a constant expression");
> > +  inform (loc, "only unevaluated inline assembly is allowed in a "
> > +	  "%<constexpr%> function in C++2a");
> 
> Note, I think in this case you do want a
>   auto_diagnostic_group d;
> before the two diagnostic calls, whether you apply this patch or not,
> because they are a diagnostic group together.  See PR84889.

It's not like we talked about it just a while ago...  /facepalm

Will fix.

Marek

On 8/7/19 4:44 PM, Marek Polacek wrote:
> On Wed, Aug 07, 2019 at 10:37:04PM +0200, Jakub Jelinek wrote:
>> On Wed, Aug 07, 2019 at 04:30:29PM -0400, Marek Polacek wrote:
>>> +/* Complain about an attempt to evaluate inline assembly.  */
>>> +
>>> +static void
>>> +inline_asm_in_constexpr_error (location_t loc)
>>> +{
>>> +  error_at (loc, "inline assembly is not a constant expression");
>>> +  inform (loc, "only unevaluated inline assembly is allowed in a "
>>> +	  "%<constexpr%> function in C++2a");
>>
>> Note, I think in this case you do want a
>>    auto_diagnostic_group d;
>> before the two diagnostic calls, whether you apply this patch or not,
>> because they are a diagnostic group together.  See PR84889.
> 
> It's not like we talked about it just a while ago...  /facepalm
> 
> Will fix.

OK with that change.

Jason