[v3,2/2] C++: Support constexpr strings for asm statements

Some programing styles use a lot of inline assembler, and it is common
to use very complex preprocessor macros to generate the assembler
strings for the asm statements. In C++ there would be a typesafe alternative
using templates and constexpr to generate the assembler strings, but
unfortunately the asm statement requires plain string literals, so this
doesn't work.

This patch modifies the C++ parser to accept strings generated by
constexpr instead of just plain strings. This requires new syntax
because e.g. asm("..." : "r" (expr)) would be ambigious with a function
call. I chose () to make it unique. For example now you can write

constexpr const char *genasm() { return "insn"; }
constexpr const char *genconstraint() { return "r"; }

	asm(genasm() :: (genconstraint()) (input));

The constexpr strings are allowed for the asm template, the
constraints and the clobbers (every time current asm accepts a string)

This version allows the same constexprs as C++26 static_assert,
following Jakub's suggestion.

The drawback of this scheme is that the constexpr doesn't have
full control over the input/output/clobber lists, but that can be
usually handled with a switch statement.  One could imagine
more flexible ways to handle that, for example supporting constexpr
vectors for the clobber list, or similar. But even without
that it is already useful.

Bootstrapped and full test on x86_64-linux.

gcc/c-family/ChangeLog:

	* c-cppbuiltin.cc (c_cpp_builtins): Define __GXX_CONSTEXPR_ASM__

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_asm_string_expression): New function
        to handle constexpr strings for asm.
	(cp_parser_asm_definition): Use cp_parser_asm_string_expression.
	(cp_parser_yield_expression): Dito.
	(cp_parser_asm_specification_opt): Dito.
	(cp_parser_asm_operand_list): Dito.
	(cp_parser_asm_clobber_list): Dito.

gcc/ChangeLog:

	* doc/extend.texi: Document constexpr asm.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/asm11.C: Adjust to new error message.
	* g++.dg/ext/asm9.C: Dito.
	* g++.dg/parse/asm1.C: Dito.
	* g++.dg/parse/asm2.C: Dito.
	* g++.dg/parse/asm3.C: Dito.
	* g++.dg/cpp1z/constexpr-asm-1.C: New test.
	* g++.dg/cpp1z/constexpr-asm-2.C: New test.
	* g++.dg/cpp1z/constexpr-asm-3.C: New test.
---
 gcc/c-family/c-cppbuiltin.cc                 |  5 +-
 gcc/cp/parser.cc                             | 86 ++++++++++++++------
 gcc/doc/extend.texi                          | 35 ++++++--
 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C | 30 +++++++
 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C | 21 +++++
 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C | 31 +++++++
 gcc/testsuite/g++.dg/ext/asm11.C             | 22 ++---
 gcc/testsuite/g++.dg/ext/asm9.C              |  3 +-
 gcc/testsuite/g++.dg/parse/asm1.C            |  1 +
 gcc/testsuite/g++.dg/parse/asm2.C            |  1 +
 gcc/testsuite/g++.dg/parse/asm3.C            |  1 +
 11 files changed, 194 insertions(+), 42 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C

On 6/5/24 00:45, Andi Kleen wrote:
> Some programing styles use a lot of inline assembler, and it is common
> to use very complex preprocessor macros to generate the assembler
> strings for the asm statements. In C++ there would be a typesafe alternative
> using templates and constexpr to generate the assembler strings, but
> unfortunately the asm statement requires plain string literals, so this
> doesn't work.
> 
> This patch modifies the C++ parser to accept strings generated by
> constexpr instead of just plain strings. This requires new syntax
> because e.g. asm("..." : "r" (expr)) would be ambigious with a function
> call. I chose () to make it unique. For example now you can write
> 
> constexpr const char *genasm() { return "insn"; }
> constexpr const char *genconstraint() { return "r"; }
> 
> 	asm(genasm() :: (genconstraint()) (input));
> 
> The constexpr strings are allowed for the asm template, the
> constraints and the clobbers (every time current asm accepts a string)
> 
> This version allows the same constexprs as C++26 static_assert,
> following Jakub's suggestion.
> 
> The drawback of this scheme is that the constexpr doesn't have
> full control over the input/output/clobber lists, but that can be
> usually handled with a switch statement.  One could imagine
> more flexible ways to handle that, for example supporting constexpr
> vectors for the clobber list, or similar. But even without
> that it is already useful.
> 
> Bootstrapped and full test on x86_64-linux.
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-cppbuiltin.cc (c_cpp_builtins): Define __GXX_CONSTEXPR_ASM__
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_asm_string_expression): New function
>          to handle constexpr strings for asm.
> 	(cp_parser_asm_definition): Use cp_parser_asm_string_expression.
> 	(cp_parser_yield_expression): Dito.
> 	(cp_parser_asm_specification_opt): Dito.
> 	(cp_parser_asm_operand_list): Dito.
> 	(cp_parser_asm_clobber_list): Dito.
> 
> gcc/ChangeLog:
> 
> 	* doc/extend.texi: Document constexpr asm.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/asm11.C: Adjust to new error message.
> 	* g++.dg/ext/asm9.C: Dito.
> 	* g++.dg/parse/asm1.C: Dito.
> 	* g++.dg/parse/asm2.C: Dito.
> 	* g++.dg/parse/asm3.C: Dito.
> 	* g++.dg/cpp1z/constexpr-asm-1.C: New test.
> 	* g++.dg/cpp1z/constexpr-asm-2.C: New test.
> 	* g++.dg/cpp1z/constexpr-asm-3.C: New test.
> ---
>   gcc/c-family/c-cppbuiltin.cc                 |  5 +-
>   gcc/cp/parser.cc                             | 86 ++++++++++++++------
>   gcc/doc/extend.texi                          | 35 ++++++--
>   gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C | 30 +++++++
>   gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C | 21 +++++
>   gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C | 31 +++++++
>   gcc/testsuite/g++.dg/ext/asm11.C             | 22 ++---
>   gcc/testsuite/g++.dg/ext/asm9.C              |  3 +-
>   gcc/testsuite/g++.dg/parse/asm1.C            |  1 +
>   gcc/testsuite/g++.dg/parse/asm2.C            |  1 +
>   gcc/testsuite/g++.dg/parse/asm3.C            |  1 +
>   11 files changed, 194 insertions(+), 42 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C
> 
> diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc
> index d9b84a0f1b97..dfd8f6f0c485 100644
> --- a/gcc/c-family/c-cppbuiltin.cc
> +++ b/gcc/c-family/c-cppbuiltin.cc
> @@ -954,7 +954,10 @@ c_cpp_builtins (cpp_reader *pfile)
>   	}
>   
>         if (cxx_dialect >= cxx11)
> -        cpp_define (pfile, "__GXX_EXPERIMENTAL_CXX0X__");
> +	{
> +	  cpp_define (pfile, "__GXX_EXPERIMENTAL_CXX0X__");
> +	  cpp_define (pfile, "__GXX_CONSTEXPR_ASM__");
> +	}
>   
>         /* Binary literals have been allowed in g++ before C++11
>   	 and were standardized for C++14.  */
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 779625144db4..1a1baa814373 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -22824,6 +22824,53 @@ cp_parser_using_directive (cp_parser* parser)
>     cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
>   }
>   
> +/* Parse a string literal or constant expression yielding a string.
> +   The constant expression uses extra parens to avoid ambiguity with "x" (expr).
> +   WHAT is an identifier for error messages.
> +
> +   asm-string-expr:
> +     string-literal
> +     ( constant-expr ) */
> +
> +static tree
> +cp_parser_asm_string_expression (cp_parser *parser)
> +{
> +  cp_token *tok = cp_lexer_peek_token (parser->lexer);
> +
> +  if (tok->type == CPP_OPEN_PAREN)
> +    {
> +      matching_parens parens;
> +      parens.consume_open (parser);
> +      tree string = cp_parser_constant_expression (parser);
> +      if (string != error_mark_node)
> +	string = cxx_constant_value (string, tf_error);
> +      if (TREE_CODE (string) == NOP_EXPR)
> +	string = TREE_OPERAND (string, 0);
> +      if (TREE_CODE (string) == ADDR_EXPR
> +	  && TREE_CODE (TREE_OPERAND (string, 0)) == STRING_CST)
> +	string = TREE_OPERAND (string, 0);
> +      if (TREE_CODE (string) == VIEW_CONVERT_EXPR)
> +	string = TREE_OPERAND (string, 0);

What in the testcases needs this wrapper stripping?

> +      cexpr_str cstr (string);
> +      if (!cstr.type_check (tok->location))
> +	return error_mark_node;

> +      const char *msg;
> +      int len;
> +      if (!cstr.extract (tok->location, msg, len))
> +	return error_mark_node;
> +      parens.require_close (parser);
> +      string = build_string (len, msg);

mayb cexpr_str should have a function to return a STRING_CST?

> +      return string;
> +    }
> +  else if (!cp_parser_is_string_literal (tok))
> +    {
> +      error_at (tok->location,
> +		"expected string-literal or constexpr in brackets");
> +      return error_mark_node;
> +    }
> +  return cp_parser_string_literal (parser, false, false);
> +}
> +
>   /* Parse an asm-definition.
>   
>     asm-qualifier:
> @@ -22836,19 +22883,19 @@ cp_parser_using_directive (cp_parser* parser)
>       asm-qualifier-list asm-qualifier
>   
>      asm-definition:
> -     asm ( string-literal ) ;
> +     asm ( constant-expr ) ;
>   
>      GNU Extension:
>   
>      asm-definition:
> -     asm asm-qualifier-list [opt] ( string-literal ) ;
> -     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt] ) ;
> -     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt]
> +     asm asm-qualifier-list [opt] ( asm-string-expr ) ;
> +     asm asm-qualifier-list [opt] ( asm-string-expr : asm-operand-list [opt] ) ;
> +     asm asm-qualifier-list [opt] ( asm-string-expr : asm-operand-list [opt]
>   				    : asm-operand-list [opt] ) ;
> -     asm asm-qualifier-list [opt] ( string-literal : asm-operand-list [opt]
> +     asm asm-qualifier-list [opt] ( asm-string-expr : asm-operand-list [opt]
>   				    : asm-operand-list [opt]
>   			  : asm-clobber-list [opt] ) ;
> -     asm asm-qualifier-list [opt] ( string-literal : : asm-operand-list [opt]
> +     asm asm-qualifier-list [opt] ( asm-string-expr : : asm-operand-list [opt]
>   				    : asm-clobber-list [opt]
>   				    : asm-goto-list ) ;
>   
> @@ -22967,8 +23014,7 @@ cp_parser_asm_definition (cp_parser* parser)
>     if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
>       return;
>     /* Look for the string.  */
> -  tree string = cp_parser_string_literal (parser, /*translate=*/false,
> -					  /*wide_ok=*/false);
> +  tree string = cp_parser_asm_string_expression (parser);
>     if (string == error_mark_node)
>       {
>         cp_parser_skip_to_closing_parenthesis (parser, true, false,
> @@ -29627,7 +29673,7 @@ cp_parser_yield_expression (cp_parser* parser)
>   /* Parse an (optional) asm-specification.
>   
>      asm-specification:
> -     asm ( string-literal )
> +     asm ( asm-string-expr )
>   
>      If the asm-specification is present, returns a STRING_CST
>      corresponding to the string-literal.  Otherwise, returns
> @@ -29650,9 +29696,7 @@ cp_parser_asm_specification_opt (cp_parser* parser)
>     parens.require_open (parser);
>   
>     /* Look for the string-literal.  */
> -  tree asm_specification = cp_parser_string_literal (parser,
> -						     /*translate=*/false,
> -						     /*wide_ok=*/false);
> +  tree asm_specification = cp_parser_asm_string_expression (parser);
>   
>     /* Look for the `)'.  */
>     parens.require_close (parser);
> @@ -29667,8 +29711,8 @@ cp_parser_asm_specification_opt (cp_parser* parser)
>        asm-operand-list , asm-operand
>   
>      asm-operand:
> -     string-literal ( expression )
> -     [ string-literal ] string-literal ( expression )
> +     asm-string-expr ( expression )
> +     [ asm-string-expr ] asm-string-expr ( expression )
>   
>      Returns a TREE_LIST representing the operands.  The TREE_VALUE of
>      each node is the expression.  The TREE_PURPOSE is itself a
> @@ -29701,10 +29745,8 @@ cp_parser_asm_operand_list (cp_parser* parser)
>   	}
>         else
>   	name = NULL_TREE;
> -      /* Look for the string-literal.  */
> -      tree string_literal = cp_parser_string_literal (parser,
> -						      /*translate=*/false,
> -						      /*wide_ok=*/false);
> +      /* Look for the string.  */
> +      tree string_literal = cp_parser_asm_string_expression (parser);
>   
>         /* Look for the `('.  */
>         matching_parens parens;
> @@ -29737,8 +29779,8 @@ cp_parser_asm_operand_list (cp_parser* parser)
>   /* Parse an asm-clobber-list.
>   
>      asm-clobber-list:
> -     string-literal
> -     asm-clobber-list , string-literal
> +     const-expression
> +     asm-clobber-list , const-expression
>   
>      Returns a TREE_LIST, indicating the clobbers in the order that they
>      appeared.  The TREE_VALUE of each node is a STRING_CST.  */
> @@ -29751,9 +29793,7 @@ cp_parser_asm_clobber_list (cp_parser* parser)
>     while (true)
>       {
>         /* Look for the string literal.  */
> -      tree string_literal = cp_parser_string_literal (parser,
> -						      /*translate=*/false,
> -						      /*wide_ok=*/false);
> +      tree string_literal = cp_parser_asm_string_expression (parser);
>         /* Add it to the list.  */
>         clobbers = tree_cons (NULL_TREE, string_literal, clobbers);
>         /* If the next token is not a `,', then the list is
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index 799a36586dc9..17e26c5004c1 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -10700,14 +10700,30 @@ contain any instructions recognized by the assembler, including directives.
>   GCC does not parse the assembler instructions themselves and
>   does not know what they mean or even whether they are valid assembler input.
>   
> -You may place multiple assembler instructions together in a single @code{asm}
> -string, separated by the characters normally used in assembly code for the
> -system. A combination that works in most places is a newline to break the
> +You may place multiple assembler instructions together in a single @code{asm}
> +string, separated by the characters normally used in assembly code for the
> +system. A combination that works in most places is a newline to break the
>   line, plus a tab character (written as @samp{\n\t}).
> -Some assemblers allow semicolons as a line separator. However,
> -note that some assembler dialects use semicolons to start a comment.
> +Some assemblers allow semicolons as a line separator. However,
> +note that some assembler dialects use semicolons to start a comment.

Why is diff flagging the above as a change?  I don't see any difference.

>   @end table
>   
> +@node asm constexprs
> +With gnu++11 or later the string can also be a compile time constant expression
> +inside parens.  The constant expression can return a string or a container
> +with data and size members, following similar rules as C++26 @code{static_assert}
> +message. Any string is converted to the character set of the source code.
> +When this feature is available the @code{__GXX_CONSTEXPR_ASM__} cpp symbol is defined.
> +
> +@example
> +constexpr const char *genfoo() @{ return "foo"; @}
> +
> +void function()
> +@{
> +  asm((genfoo()));
> +@}
> +@end example
> +
>   @subsubheading Remarks
>   Using extended @code{asm} (@pxref{Extended Asm}) typically produces
>   smaller, safer, and more efficient code, and in most cases it is a
> @@ -10850,20 +10866,27 @@ perform a jump to one of the labels listed in the @var{GotoLabels}.
>   @item AssemblerTemplate
>   This is a literal string that is the template for the assembler code. It is a
>   combination of fixed text and tokens that refer to the input, output,
> -and goto parameters. @xref{AssemblerTemplate}.
> +and goto parameters. @xref{AssemblerTemplate}. With gnu++11 or later it can
> +also be a constant expression inside parens (see @ref{asm constexprs}).
>   
>   @item OutputOperands
>   A comma-separated list of the C variables modified by the instructions in the
>   @var{AssemblerTemplate}.  An empty list is permitted.  @xref{OutputOperands}.
> +With gnu++11 or later the strings can also be constant expressions inside parens
> +(see @ref{asm constexprs})
>   
>   @item InputOperands
>   A comma-separated list of C expressions read by the instructions in the
>   @var{AssemblerTemplate}.  An empty list is permitted.  @xref{InputOperands}.
> +With gnu++11 or later the strings can also be constant expressions inside parens
> +(see @ref{asm constexprs})
>   
>   @item Clobbers
>   A comma-separated list of registers or other values changed by the
>   @var{AssemblerTemplate}, beyond those listed as outputs.
>   An empty list is permitted.  @xref{Clobbers and Scratch Registers}.
> +With gnu++11 or later the strings can also be constant expressions inside parens
> +(see @ref{asm constexprs})
>   
>   @item GotoLabels
>   When you are using the @code{goto} form of @code{asm}, this section contains
> diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C
> new file mode 100644
> index 000000000000..7cc6b37d6208
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C
> @@ -0,0 +1,30 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu++11" } */
> +
> +constexpr const char *genfoo ()
> +{
> +  return "foo %1,%0";
> +}
> +
> +constexpr const char *genoutput ()
> +{
> +  return "=r";
> +}
> +
> +constexpr const char *geninput ()
> +{
> +  return "r";
> +}
> +
> +constexpr const char *genclobber ()
> +{
> +  return "memory";
> +}
> +
> +void f()
> +{
> +  int a;
> +  asm((genfoo ()) : (genoutput ()) (a) : (geninput ()) (1) : (genclobber ()));
> +}
> +
> +/* { dg-final { scan-assembler "foo" } } */
> diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C
> new file mode 100644
> index 000000000000..7d0eb590afbe
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-2.C
> @@ -0,0 +1,21 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu++11" } */
> +
> +using size_t = typeof(sizeof(0));
> +template <typename T, size_t N>
> +struct array {
> +  constexpr size_t size () const { return N; }
> +  constexpr const T *data () const { return a; }
> +  const T a[N];
> +};
> +
> +void f()
> +{
> +  int a;
> +  asm((array<char, 3> {'f','o','o'}) :
> +      (array<char, 2>{'=','r'})  (a) :
> +      (array<char, 1>{'r'}) (1) :
> +      (array<char, 6>{'m','e','m','o','r','y'}));
> +}
> +
> +/* { dg-final { scan-assembler "foo" } } */
> diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C b/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C
> new file mode 100644
> index 000000000000..d33631876bdc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-3.C
> @@ -0,0 +1,31 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu++11" } */
> +
> +constexpr const char *genfoo ()
> +{
> +  return "foo %1,%0";
> +}
> +
> +constexpr const char *genoutput ()
> +{
> +  return "=r";
> +}
> +
> +constexpr const char *geninput ()
> +{
> +  return "r";
> +}
> +
> +constexpr const char *genclobber ()
> +{
> +  return "memory";
> +}
> +
> +void f()
> +{
> +  int a;
> +  asm(genfoo () : /* { dg-error "expected string-literal or constexpr in brackets" } */
> +      genoutput() (a) :
> +      geninput() (1) :
> +      genclobber());
> +}
> diff --git a/gcc/testsuite/g++.dg/ext/asm11.C b/gcc/testsuite/g++.dg/ext/asm11.C
> index 7939aacc0f42..97a299a7ecb2 100644
> --- a/gcc/testsuite/g++.dg/ext/asm11.C
> +++ b/gcc/testsuite/g++.dg/ext/asm11.C
> @@ -6,15 +6,15 @@ void
>   foo ()
>   {
>     int i;
> -  asm ();		  // { dg-error "expected string-literal before" }
> -  asm (1);		  // { dg-error "expected string-literal before" }
> -  asm (int);		  // { dg-error "expected string-literal before" }
> -  asm (: "=r" (i));	  // { dg-error "expected string-literal before" }
> -  asm (1 : "=r" (i));	  // { dg-error "expected string-literal before" }
> -  asm (int : "=r" (i));	  // { dg-error "expected string-literal before" }
> -  asm (: : "r" (i));	  // { dg-error "expected string-literal before" }
> -  asm (1 : : "r" (i));	  // { dg-error "expected string-literal before" }
> -  asm (int : : "r" (i));  // { dg-error "expected string-literal before" }
> -  asm (: : : "memory");	  // { dg-error "expected string-literal before" }
> -  asm (1 : : : "memory"); // { dg-error "expected string-literal before" }
> +  asm ();		  // { dg-error "expected string-literal" }
> +  asm (1);		  // { dg-error "expected string-literal" }
> +  asm (int);		  // { dg-error "expected string-literal" }
> +  asm (: "=r" (i));	  // { dg-error "expected string-literal" }
> +  asm (1 : "=r" (i));	  // { dg-error "expected string-literal" }
> +  asm (int : "=r" (i));	  // { dg-error "expected string-literal" }
> +  asm (: : "r" (i));	  // { dg-error "expected string-literal" }
> +  asm (1 : : "r" (i));	  // { dg-error "expected string-literal" }
> +  asm (int : : "r" (i));  // { dg-error "expected string-literal" }
> +  asm (: : : "memory");	  // { dg-error "expected string-literal" }
> +  asm (1 : : : "memory"); // { dg-error "expected string-literal" }
>   }
> diff --git a/gcc/testsuite/g++.dg/ext/asm9.C b/gcc/testsuite/g++.dg/ext/asm9.C
> index 9daa01bbf5f9..3bce845c97a0 100644
> --- a/gcc/testsuite/g++.dg/ext/asm9.C
> +++ b/gcc/testsuite/g++.dg/ext/asm9.C
> @@ -3,5 +3,6 @@
>   
>   void foo()
>   {
> -  asm("" ::: X); // { dg-error "before" }
> +  asm("" ::: X); // { dg-error "string-literal" }
> +		 // { dg-error "before" "" { target *-*-* } .-1 }
>   }
> diff --git a/gcc/testsuite/g++.dg/parse/asm1.C b/gcc/testsuite/g++.dg/parse/asm1.C
> index 009ffbd3ad35..9ce24b564dce 100644
> --- a/gcc/testsuite/g++.dg/parse/asm1.C
> +++ b/gcc/testsuite/g++.dg/parse/asm1.C
> @@ -3,4 +3,5 @@
>   void foo()
>   {
>     asm("" : 0);  // { dg-error "numeric constant|token" }
> +		// { dg-error "string-literal" "" { target *-*-* } .-1 }
>   }
> diff --git a/gcc/testsuite/g++.dg/parse/asm2.C b/gcc/testsuite/g++.dg/parse/asm2.C
> index 09924c9c7c31..d4e1c6e80de5 100644
> --- a/gcc/testsuite/g++.dg/parse/asm2.C
> +++ b/gcc/testsuite/g++.dg/parse/asm2.C
> @@ -3,4 +3,5 @@
>   void foo()
>   {
>     asm("" :: 0);  // { dg-error "numeric constant|token" }
> +		 // { dg-error "string-literal" "" { target *-*-* } .-1 }
>   }
> diff --git a/gcc/testsuite/g++.dg/parse/asm3.C b/gcc/testsuite/g++.dg/parse/asm3.C
> index 91f19e48cd5a..dccee5ac6813 100644
> --- a/gcc/testsuite/g++.dg/parse/asm3.C
> +++ b/gcc/testsuite/g++.dg/parse/asm3.C
> @@ -3,4 +3,5 @@
>   void foo()
>   {
>     asm ("%[x]" : [0](x));  // { dg-error "numeric constant|token" }
> +			  // { dg-error "string-literal" "" { target *-*-* } .-1 }
>   }

Hi Jason,

Sorry I must have misunderstood you. I thought the patch was already
approved earlier and I did commit. I can revert or do additional
changes.

On Tue, Jun 11, 2024 at 04:31:30PM -0400, Jason Merrill wrote:
> > +  if (tok->type == CPP_OPEN_PAREN)
> > +    {
> > +      matching_parens parens;
> > +      parens.consume_open (parser);
> > +      tree string = cp_parser_constant_expression (parser);
> > +      if (string != error_mark_node)
> > +	string = cxx_constant_value (string, tf_error);
> > +      if (TREE_CODE (string) == NOP_EXPR)
> > +	string = TREE_OPERAND (string, 0);
> > +      if (TREE_CODE (string) == ADDR_EXPR
> > +	  && TREE_CODE (TREE_OPERAND (string, 0)) == STRING_CST)
> > +	string = TREE_OPERAND (string, 0);
> > +      if (TREE_CODE (string) == VIEW_CONVERT_EXPR)
> > +	string = TREE_OPERAND (string, 0);
> 
> What in the testcases needs this wrapper stripping?

Without the stripping I get

/home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C: In
function 'void f()':
/home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C:27:16:
error: request for member 'size' in '(const char*)"foo %1,%0"', which is
of non-class type 'const char*'
/home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C:27:7:
error: constexpr string must be a string literal or object with 'size'
and 'data' members
/home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C:27:19:
error: expected primary-expression before ':' token
compiler exited with status 1

presumably because it fails this test:


  if (TREE_CODE (message) != STRING_CST
        && !type_dependent_expression_p (message))

> > +      cexpr_str cstr (string);
> > +      if (!cstr.type_check (tok->location))
> > +	return error_mark_node;
> 
> > +      const char *msg;
> > +      int len;
> > +      if (!cstr.extract (tok->location, msg, len))
> > +	return error_mark_node;
> > +      parens.require_close (parser);
> > +      string = build_string (len, msg);
> 
> mayb cexpr_str should have a function to return a STRING_CST?

Okay I can add that.

> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -10700,14 +10700,30 @@ contain any instructions recognized by the assembler, including directives.
> >   GCC does not parse the assembler instructions themselves and
> >   does not know what they mean or even whether they are valid assembler input.
> > -You may place multiple assembler instructions together in a single @code{asm}
> > -string, separated by the characters normally used in assembly code for the
> > -system. A combination that works in most places is a newline to break the
> > +You may place multiple assembler instructions together in a single @code{asm}
> > +string, separated by the characters normally used in assembly code for the
> > +system. A combination that works in most places is a newline to break the
> >   line, plus a tab character (written as @samp{\n\t}).
> > -Some assemblers allow semicolons as a line separator. However,
> > -note that some assembler dialects use semicolons to start a comment.
> > +Some assemblers allow semicolons as a line separator. However,
> > +note that some assembler dialects use semicolons to start a comment.
> 
> Why is diff flagging the above as a change?  I don't see any difference.

Strange, but I don't know. I don't see a white space change.

-Andi

On Tue, 2024-06-11 at 20:53 -0700, Andi Kleen wrote:
> > > -Some assemblers allow semicolons as a line separator. However,
> > > -note that some assembler dialects use semicolons to start a comment.
> > > +Some assemblers allow semicolons as a line separator. However,
> > > +note that some assembler dialects use semicolons to start a comment.
> > 
> > Why is diff flagging the above as a change?  I don't see any difference.
> 
> Strange, but I don't know. I don't see a white space change.

There was one trailing white space in either lines, and your commit has
removed the spaces.  But those spaces are completely missing in the
patch sent to gcc-patches.  Maybe your mail client has eaten them?

On 6/11/24 23:53, Andi Kleen wrote:
> 
> Sorry I must have misunderstood you. I thought the patch was already
> approved earlier and I did commit. I can revert or do additional
> changes.

I only meant to approve the refactoring patch, but no worries.

> On Tue, Jun 11, 2024 at 04:31:30PM -0400, Jason Merrill wrote:
>>> +  if (tok->type == CPP_OPEN_PAREN)
>>> +    {
>>> +      matching_parens parens;
>>> +      parens.consume_open (parser);
>>> +      tree string = cp_parser_constant_expression (parser);
>>> +      if (string != error_mark_node)
>>> +	string = cxx_constant_value (string, tf_error);
>>> +      if (TREE_CODE (string) == NOP_EXPR)
>>> +	string = TREE_OPERAND (string, 0);
>>> +      if (TREE_CODE (string) == ADDR_EXPR
>>> +	  && TREE_CODE (TREE_OPERAND (string, 0)) == STRING_CST)
>>> +	string = TREE_OPERAND (string, 0);
>>> +      if (TREE_CODE (string) == VIEW_CONVERT_EXPR)
>>> +	string = TREE_OPERAND (string, 0);
>>
>> What in the testcases needs this wrapper stripping?
> 
> Without the stripping I get
> 
> /home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C: In
> function 'void f()':
> /home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C:27:16:
> error: request for member 'size' in '(const char*)"foo %1,%0"', which is
> of non-class type 'const char*'
> /home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C:27:7:
> error: constexpr string must be a string literal or object with 'size'
> and 'data' members
> /home/ak/gcc/gcc/gcc/testsuite/g++.dg/cpp1z/constexpr-asm-1.C:27:19:
> error: expected primary-expression before ':' token
> compiler exited with status 1
> 
> presumably because it fails this test:
> 
>    if (TREE_CODE (message) != STRING_CST
>          && !type_dependent_expression_p (message))

Ah, yes, because you want to use a function returning const char *, 
which is not allowed for static_assert; rather, static_assert wants the 
function to return a wrapper class like std::string_view.  Only the 
return type needs to change; the return statement can still return a 
string-literal.

This extension relative to static_assert seems unnecessary to me.

> +		"expected string-literal or constexpr in brackets");

parentheses, not brackets.

Jason