diff mbox series

__VA_OPT__ fixes (PR preprocessor/83063, PR preprocessor/83708)

Message ID 20180110120407.GI1833@tucnak
State New
Headers show
Series __VA_OPT__ fixes (PR preprocessor/83063, PR preprocessor/83708) | expand

Commit Message

Jakub Jelinek Jan. 10, 2018, 12:04 p.m. UTC
Hi!

In libcpp, we have quite a lot of state on the token flags, some
related to the stuff that comes before the token (e.g.
PREV_FALLTHROUGH, PREV_WHITE and STRINGIFY_ARG), others related to the
stuff that comes after the token (e.g. PASTE_LEFT, SP_DIGRAPH, SP_PREV_WHITE).
Unfortunately, with the current __VA_OPT__ handling that information is
lost, because it is on the __VA_OPT__ or closing ) tokens that we are always
DROPing.

The following patch attempts to fix various issues, including some ICEs,
by introducing 3 new states, two of them are alternatives to INCLUDE used
for the very first token after __VA_OPT__( , where we want to take into
account also flags from the __VA_OPT__ token, and before the closing )
token where we want to use flags from the closing ) token.  Plus
PADDING which is used for the case where there are no varargs and __VA_OPT__
is supposed to fold into a placemarker, or for the case of __VA_OPT__(),
which is similar to that, in both cases we need to take into account in
those cases both flags from __VA_OPT__ and from the closing ).

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

This is just a partial fix, one thing this patch doesn't change is that
the standard says that __VA_OPT__ ( contents ) should be treated as
parameter, which means that #__VA_OPT__ ( contents ) should stringify it,
which we right now reject.  My preprocessor knowledge is too limited to
handle this right myself, including all the corner cases, e.g. one can have
#define f(x, ...) #__VA_OPT__(#x x ## x) etc..  I presume
m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
could be changed into:
m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE | STRINGIFY_ARG);
and when handling the PADDING result from update, we could just emit a 
"" token, but for INCLUDE_FIRST with this we'd need something complex,
probably a new routine similar to stringify_arg to some extent.

I've also cross-checked the libcpp implementation with this patch against
trunk clang which apparently also implements __VA_OPT__ now, on the
testcases included here the output is the same and on their
macro_vaopt_expand.cpp testcase, if I remove all tests that test
#__VA_OPT__ ( contents ) handling which we just reject now, there are still
some differences:
$ /usr/src/llvm/obj8/bin/clang++  -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/1
$ ~/src/gcc/obj20/gcc/cc1plus -quiet -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/2

	Jakub

Comments

Jakub Jelinek Jan. 17, 2018, 4:47 p.m. UTC | #1
Hi!

I'd like to ping this patch.
As I wrote, it isn't a full solution for all the __VA_OPT__ issues,
but it isn't even clear to me how exactly it should behave, but fixes
some ICEs and a couple of most important issues and shouldn't make things
worse, at least on the gcc and clang __VA_OPT__ testcases.

On Wed, Jan 10, 2018 at 01:04:07PM +0100, Jakub Jelinek wrote:
> In libcpp, we have quite a lot of state on the token flags, some
> related to the stuff that comes before the token (e.g.
> PREV_FALLTHROUGH, PREV_WHITE and STRINGIFY_ARG), others related to the
> stuff that comes after the token (e.g. PASTE_LEFT, SP_DIGRAPH, SP_PREV_WHITE).
> Unfortunately, with the current __VA_OPT__ handling that information is
> lost, because it is on the __VA_OPT__ or closing ) tokens that we are always
> DROPing.
> 
> The following patch attempts to fix various issues, including some ICEs,
> by introducing 3 new states, two of them are alternatives to INCLUDE used
> for the very first token after __VA_OPT__( , where we want to take into
> account also flags from the __VA_OPT__ token, and before the closing )
> token where we want to use flags from the closing ) token.  Plus
> PADDING which is used for the case where there are no varargs and __VA_OPT__
> is supposed to fold into a placemarker, or for the case of __VA_OPT__(),
> which is similar to that, in both cases we need to take into account in
> those cases both flags from __VA_OPT__ and from the closing ).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> This is just a partial fix, one thing this patch doesn't change is that
> the standard says that __VA_OPT__ ( contents ) should be treated as
> parameter, which means that #__VA_OPT__ ( contents ) should stringify it,
> which we right now reject.  My preprocessor knowledge is too limited to
> handle this right myself, including all the corner cases, e.g. one can have
> #define f(x, ...) #__VA_OPT__(#x x ## x) etc..  I presume
> m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
> could be changed into:
> m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE | STRINGIFY_ARG);
> and when handling the PADDING result from update, we could just emit a 
> "" token, but for INCLUDE_FIRST with this we'd need something complex,
> probably a new routine similar to stringify_arg to some extent.
> 
> I've also cross-checked the libcpp implementation with this patch against
> trunk clang which apparently also implements __VA_OPT__ now, on the
> testcases included here the output is the same and on their
> macro_vaopt_expand.cpp testcase, if I remove all tests that test
> #__VA_OPT__ ( contents ) handling which we just reject now, there are still
> some differences:
> $ /usr/src/llvm/obj8/bin/clang++  -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/1
> $ ~/src/gcc/obj20/gcc/cc1plus -quiet -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/2
> diff -up /tmp/1 /tmp/2
> -4: f(0 )
> +4: f(0)
> -6: f(0, a )
> -7: f(0, a )
> +6: f(0, a)
> +7: f(0, a)
> -9: TONG C ( ) B ( ) "A()"
> +9: HT_A() C ( ) B ( ) "A()"
> -16: S foo ;
> +16: S foo;
> -26: B1
> -26_1: B1
> +26: B 1
> +26_1: B 1
> -27: B11
> -27_1: BexpandedA0 11
> -28: B11
> +27: B 11
> +27_1: BA0 11
> +28: B 11
> 
> Perhaps some of the whitespace changes aren't significant, but 9:, and
> 2[678]{,_1}: are significantly different.
> 9: is
> #define LPAREN ( 
> #define A() B LPAREN )
> #define B() C LPAREN )
> #define HT_B() TONG
> #define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
> 9: F(A(),1)
> 
> Thoughts on what is right and why?
> Similarly for expansion on the last token from __VA_OPT__ when followed
> by ##, like:
> #define m1 (
> #define f16() f17 m1 )
> #define f17() f18 m1 )
> #define f18() m2 m1 )
> #define m3f17() g
> #define f19(x, ...) m3 ## __VA_OPT__(x x f16() #x)
> #define f20(x, ...) __VA_OPT__(x x)##m4()
> #define f21() f17
> #define f17m4() h
> t25 f19 (f16 (), 1);
> t26 f20 (f21 (), 2);
> 
> E.g. 26: is:
> #define F(a,...)  __VA_OPT__(B a ## a) ## 1
> 26: F(,1)
> I really wonder why clang emits B1 in that case, there
> is no ## in between B and a, so those are 2 separate tokens
> separated by whitespace, even when a ## a is a placemarker.
> Does that mean __VA_OPT__ should throw away all the placemarkers
> and return the last non-placemarker token for the ## handling?
> 
> Can somebody please take the rest over?
> 
> BTW, Tom, perhaps you should update your MAINTAINERS entry email address...
> 
> 2018-01-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR preprocessor/83063
> 	PR preprocessor/83708
> 	* macro.c (enum macro_arg_token_kind): Fix comment typo.
> 	(vaopt_state): Add m_flags field, reorder m_last_was_paste before
> 	m_state.
> 	(vaopt_state::vaopt_state): Adjust for the above changes.
> 	(vaopt_state::update_flags): Add INCLUDE_FIRST, INCLUDE_LAST and
> 	PADDING.
> 	(vaopt_state::update): Add limit argument, update m_flags member,
> 	return INCLUDE_FIRST instead of INCLUDE on the first and INCLUDE_LAST
> 	instead of INCLUDE on the last token between __VA_OPT__( and ).
> 	Returnn PADDING instead of DROP on the closing ) if no tokens were
> 	INCLUDE*.  Comment spelling fixes.
> 	(vaopt_state::flags): New method.
> 	(replace_args): Adjust vaopt_state::update caller.  Fix handling
> 	of __VA_OPT__ preceded or followed by ## and ensure no paste after
> 	last __VA_OPT__ token if not followed by ##.  Formatting fix.
> 	(create_iso_definition): Adjust vaopt_state::update caller.
> 
> 	* c-c++-common/cpp/va-opt-2.c: New test.
> 	* c-c++-common/cpp/va-opt-3.c: New test.
> 
> --- libcpp/macro.c.jj	2018-01-03 10:42:55.938763447 +0100
> +++ libcpp/macro.c	2018-01-09 17:27:23.603191796 +0100
> @@ -51,7 +51,7 @@ struct macro_arg
>  enum macro_arg_token_kind {
>    MACRO_ARG_TOKEN_NORMAL,
>    /* This is a macro argument token that got transformed into a string
> -     litteral, e.g. #foo.  */
> +     literal, e.g. #foo.  */
>    MACRO_ARG_TOKEN_STRINGIFIED,
>    /* This is a token resulting from the expansion of a macro
>       argument that was itself a macro.  */
> @@ -105,10 +105,11 @@ class vaopt_state {
>      : m_pfile (pfile),
>      m_allowed (any_args),
>      m_variadic (is_variadic),
> -    m_state (0),
>      m_last_was_paste (false),
> +    m_state (0),
>      m_paste_location (0),
> -    m_location (0)
> +    m_location (0),
> +    m_flags (0)
>    {
>    }
>  
> @@ -116,13 +117,16 @@ class vaopt_state {
>    {
>      ERROR,
>      DROP,
> -    INCLUDE
> +    INCLUDE,
> +    INCLUDE_FIRST,
> +    INCLUDE_LAST,
> +    PADDING
>    };
>  
>    /* Given a token, update the state of this tracker and return a
>       boolean indicating whether the token should be be included in the
>       expansion.  */
> -  update_type update (const cpp_token *token)
> +  update_type update (const cpp_token *token, const cpp_token *limit)
>    {
>      /* If the macro isn't variadic, just don't bother.  */
>      if (!m_variadic)
> @@ -139,6 +143,7 @@ class vaopt_state {
>  	  }
>  	++m_state;
>  	m_location = token->src_loc;
> +	m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
>  	return DROP;
>        }
>      else if (m_state == 1)
> @@ -155,6 +160,7 @@ class vaopt_state {
>        }
>      else if (m_state >= 2)
>        {
> +	update_type ret = INCLUDE;
>  	if (m_state == 2 && token->type == CPP_PASTE)
>  	  {
>  	    cpp_error_at (m_pfile, CPP_DL_ERROR, token->src_loc,
> @@ -165,7 +171,10 @@ class vaopt_state {
>  	   case we see a close paren immediately after the open
>  	   paren.  */
>  	if (m_state == 2)
> -	  ++m_state;
> +	  {
> +	    ++m_state;
> +	    ret = INCLUDE_FIRST;
> +	  }
>  
>  	bool was_paste = m_last_was_paste;
>  	m_last_was_paste = false;
> @@ -191,10 +200,27 @@ class vaopt_state {
>  		    return ERROR;
>  		  }
>  
> -		return DROP;
> +		if (ret == INCLUDE_FIRST)
> +		  {
> +		    m_flags |= token->flags & (PASTE_LEFT | SP_DIGRAPH
> +					       | SP_PREV_WHITE);
> +		    return PADDING;
> +		  }
> +		return m_allowed ? DROP : PADDING;
>  	      }
>  	  }
> -	return m_allowed ? INCLUDE : DROP;
> +	if (limit
> +	    && m_state == 3
> +	    && token + 1 < limit
> +	    && token[1].type == CPP_CLOSE_PAREN)
> +	  {
> +	    if (m_allowed && ret != INCLUDE_FIRST)
> +	      m_flags = 0;
> +	    m_flags |= token[1].flags & (PASTE_LEFT | SP_DIGRAPH
> +					 | SP_PREV_WHITE);
> +	    ret = INCLUDE_LAST;
> +	  }
> +	return m_allowed ? ret : DROP;
>        }
>  
>      /* Nothing to do with __VA_OPT__.  */
> @@ -211,6 +237,12 @@ class vaopt_state {
>      return m_state == 0;
>    }
>  
> +  /* Return any flags that should be ored to the current token.  */
> +  unsigned short flags ()
> +  {
> +    return m_flags;
> +  }
> +
>   private:
>  
>    /* The cpp_reader.  */
> @@ -220,6 +252,9 @@ class vaopt_state {
>    bool m_allowed;
>    /* True if the macro is variadic.  */
>    bool m_variadic;
> +  /* If true, the previous token was ##.  This is used to detect when
> +     a paste occurs at the end of the sequence.  */
> +  bool m_last_was_paste;
>  
>    /* The state variable:
>       0 means not parsing
> @@ -228,14 +263,14 @@ class vaopt_state {
>       >= 3 means looking for ")", the number encodes the paren depth.  */
>    int m_state;
>  
> -  /* If true, the previous token was ##.  This is used to detect when
> -     a paste occurs at the end of the sequence.  */
> -  bool m_last_was_paste;
>    /* The location of the paste token.  */
>    source_location m_paste_location;
>  
>    /* Location of the __VA_OPT__ token.  */
>    source_location m_location;
> +
> +  /* The flags from the __VA_OPT__ token and closing ).  */
> +  unsigned short m_flags;
>  };
>  
>  /* Macro expansion.  */
> @@ -1817,9 +1852,9 @@ replace_args (cpp_reader *pfile, cpp_has
>  	   multiple tokens. This is to save memory at the expense of
>  	   accuracy.
>  
> -	   Suppose we have #define SQARE(A) A * A
> +	   Suppose we have #define SQUARE(A) A * A
>  
> -	   And then we do SQARE(2+3)
> +	   And then we do SQUARE(2+3)
>  
>  	   Then the tokens 2, +, 3, will have the same location,
>  	   saying they come from the expansion of the argument A.  */
> @@ -1831,16 +1866,70 @@ replace_args (cpp_reader *pfile, cpp_has
>    i = 0;
>    vaopt_state vaopt_tracker (pfile, macro->variadic,
>  			     args[macro->paramc - 1].count > 0);
> +  unsigned short prev_flags = 0;
>    for (src = macro->exp.tokens; src < limit; src++)
>      {
>        unsigned int arg_tokens_count;
>        macro_arg_token_iter from;
>        const cpp_token **paste_flag = NULL;
>        const cpp_token **tmp_token_ptr;
> +      unsigned short flags = src->flags;
>  
>        /* __VA_OPT__ handling.  */
> -      if (vaopt_tracker.update (src) != vaopt_state::INCLUDE)
> -	continue;
> +      vaopt_state::update_type vaopt_state = vaopt_tracker.update (src, limit);
> +      if (vaopt_state != vaopt_state::INCLUDE)
> +	{
> +	  if (vaopt_state == vaopt_state::PADDING)
> +	    {
> +	      flags = vaopt_tracker.flags ();
> +	      if ((flags & PASTE_LEFT) && (prev_flags & PASTE_LEFT))
> +		continue;
> +	      flags &= ~PASTE_LEFT;
> +	      cpp_token *t = (cpp_token *) padding_token (pfile, src);
> +	      unsigned index = expanded_token_index (pfile, macro, src, i);
> +	      t->flags |= flags;
> +	      /* Allocate a virtual location for the padding token and
> +		 append the token and its location to BUFF and
> +		 VIRT_LOCS.   */
> +	      tokens_buff_add_token (buff, virt_locs, t,
> +				     t->src_loc, t->src_loc,
> +				     map, index);
> +	      prev_flags = flags;
> +	      continue;
> +	    }
> +	  else if (vaopt_state == vaopt_state::INCLUDE_FIRST
> +		   || vaopt_state == vaopt_state::INCLUDE_LAST)
> +	    {
> +	      unsigned short vaopt_flags = vaopt_tracker.flags ();
> +	      if (src->type != CPP_MACRO_ARG)
> +		{
> +		  cpp_token *t = _cpp_temp_token (pfile);
> +		  t->type = src->type;
> +		  t->val = src->val;
> +		  t->flags = flags | vaopt_flags;
> +		  unsigned index = expanded_token_index (pfile, macro, src, i);
> +		  tokens_buff_add_token (buff, virt_locs, t,
> +					 t->src_loc, t->src_loc, map, index);
> +		  i += 1;
> +		  prev_flags = t->flags;
> +		  /* Avoid paste on RHS, __VA_OPT__(c)d or
> +		     __VA_OPT__(c)__VA_OPT__(d).  */
> +		  if (!(vaopt_flags & PASTE_LEFT)
> +		      && vaopt_state == vaopt_state::INCLUDE_LAST)
> +		    {
> +		      const cpp_token *t = &pfile->avoid_paste;
> +		      tokens_buff_add_token (buff, virt_locs,
> +					     t, t->src_loc, t->src_loc,
> +					     NULL, 0);
> +		    }
> +		  continue;
> +		}
> +	      else
> +		flags |= vaopt_flags;
> +	    }
> +	  else
> +	    continue;
> +	}
>  
>        if (src->type != CPP_MACRO_ARG)
>  	{
> @@ -1852,6 +1941,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  				 src->src_loc, src->src_loc,
>  				 map, index);
>  	  i += 1;
> +	  prev_flags = flags;
>  	  continue;
>  	}
>  
> @@ -1866,7 +1956,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  	 below is to initialize the iterator depending on the type of
>  	 tokens the macro argument has.  It also does some adjustment
>  	 related to padding tokens and some pasting corner cases.  */
> -      if (src->flags & STRINGIFY_ARG)
> +      if (flags & STRINGIFY_ARG)
>  	{
>  	  arg_tokens_count = 1;
>  	  macro_arg_token_iter_init (&from,
> @@ -1875,7 +1965,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  				     MACRO_ARG_TOKEN_STRINGIFIED,
>  				     arg, &arg->stringified);
>  	}
> -      else if (src->flags & PASTE_LEFT)
> +      else if (flags & PASTE_LEFT)
>  	{
>  	  arg_tokens_count = arg->count;
>  	  macro_arg_token_iter_init (&from,
> @@ -1884,7 +1974,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  				     MACRO_ARG_TOKEN_NORMAL,
>  				     arg, arg->first);
>  	}
> -      else if (src != macro->exp.tokens && (src[-1].flags & PASTE_LEFT))
> +      else if (prev_flags & PASTE_LEFT)
>  	{
>  	  int num_toks;
>  	  arg_tokens_count = arg->count;
> @@ -1936,7 +2026,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  
>        /* Padding on the left of an argument (unless RHS of ##).  */
>        if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
> -	  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT))
> +	  && src != macro->exp.tokens && !(prev_flags & PASTE_LEFT))
>  	{
>  	  const cpp_token *t = padding_token (pfile, src);
>  	  unsigned index = expanded_token_index (pfile, macro, src, i);
> @@ -1960,9 +2050,9 @@ replace_args (cpp_reader *pfile, cpp_has
>  		 save extra memory while tracking macro expansion
>  		 locations.  So in that case here is what we do:
>  
> -		 Suppose we have #define SQARE(A) A * A
> +		 Suppose we have #define SQUARE(A) A * A
>  
> -		 And then we do SQARE(2+3)
> +		 And then we do SQUARE(2+3)
>  
>  		 Then the tokens 2, +, 3, will have the same location,
>  		 saying they come from the expansion of the argument
> @@ -1970,9 +2060,9 @@ replace_args (cpp_reader *pfile, cpp_has
>  
>  	      So that means we are going to ignore the COUNT tokens
>  	      resulting from the expansion of the current macro
> -	      arugment. In other words all the ARG_TOKENS_COUNT tokens
> +	      argument. In other words all the ARG_TOKENS_COUNT tokens
>  	      resulting from the expansion of the macro argument will
> -	      have the index I. Normally, each of those token should
> +	      have the index I.  Normally, each of those token should
>  	      have index I+J.  */
>  	      unsigned token_index = i;
>  	      unsigned index;
> @@ -1989,9 +2079,9 @@ replace_args (cpp_reader *pfile, cpp_has
>  
>  	  /* With a non-empty argument on the LHS of ##, the last
>  	     token should be flagged PASTE_LEFT.  */
> -	  if (src->flags & PASTE_LEFT)
> -	    paste_flag =
> -	      (const cpp_token **) tokens_buff_last_token_ptr (buff);
> +	  if (flags & PASTE_LEFT)
> +	    paste_flag
> +	      = (const cpp_token **) tokens_buff_last_token_ptr (buff);
>  	}
>        else if (CPP_PEDANTIC (pfile) && ! CPP_OPTION (pfile, c99)
>  	       && ! macro->syshdr && ! cpp_in_system_header (pfile))
> @@ -2021,7 +2111,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  		     NODE_NAME (node), src->val.macro_arg.arg_no);
>  
>        /* Avoid paste on RHS (even case count == 0).  */
> -      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT))
> +      if (!pfile->state.in_directive && !(flags & PASTE_LEFT))
>  	{
>  	  const cpp_token *t = &pfile->avoid_paste;
>  	  tokens_buff_add_token (buff, virt_locs,
> @@ -2035,7 +2125,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  	  cpp_token *token = _cpp_temp_token (pfile);
>  	  token->type = (*paste_flag)->type;
>  	  token->val = (*paste_flag)->val;
> -	  if (src->flags & PASTE_LEFT)
> +	  if (flags & PASTE_LEFT)
>  	    token->flags = (*paste_flag)->flags | PASTE_LEFT;
>  	  else
>  	    token->flags = (*paste_flag)->flags & ~PASTE_LEFT;
> @@ -2043,6 +2133,7 @@ replace_args (cpp_reader *pfile, cpp_has
>  	}
>  
>        i += arg_tokens_count;
> +      prev_flags = flags;
>      }
>  
>    if (track_macro_exp)
> @@ -3304,7 +3395,7 @@ create_iso_definition (cpp_reader *pfile
>  	    }
>  	}
>  
> -      if (vaopt_tracker.update (token) == vaopt_state::ERROR)
> +      if (vaopt_tracker.update (token, NULL) == vaopt_state::ERROR)
>  	return false;
>  
>        following_paste_op = (token->type == CPP_PASTE);
> --- gcc/testsuite/c-c++-common/cpp/va-opt-2.c.jj	2018-01-09 13:06:56.721345854 +0100
> +++ gcc/testsuite/c-c++-common/cpp/va-opt-2.c	2018-01-09 17:13:35.759095064 +0100
> @@ -0,0 +1,41 @@
> +/* PR preprocessor/83063 */
> +/* { dg-do compile } */
> +/* { dg-options "-std=gnu99" { target c } } */
> +/* { dg-options "-std=c++2a" { target c++ } } */
> +
> +#define f1(...) int b##__VA_OPT__(c)
> +#define f2(...) int __VA_OPT__(c)##d
> +#define f3(...) int e##__VA_OPT__()
> +#define f4(...) int __VA_OPT__()##f
> +#define f5(...) int g##__VA_OPT__(h)##i
> +#define f6(...) int j##__VA_OPT__()##k
> +#define f7(...) int l##__VA_OPT__()
> +#define f8(...) int __VA_OPT__()##m
> +#define f9(...) int n##__VA_OPT__()##o
> +#define f10(x, ...) int x##__VA_OPT__(x)
> +#define f11(x, ...) int __VA_OPT__(x)##x
> +#define f12(x, ...) int x##__VA_OPT__(x)##x
> +f1 (1, 2, 3);
> +f1 ();
> +f2 (1, 2);
> +f2 ();
> +f3 (1);
> +f4 (2);
> +f5 (6, 7);
> +f5 ();
> +f6 (8);
> +f7 ();
> +f8 ();
> +f9 ();
> +f10 (p, 5, 6);
> +f10 (p);
> +f11 (q, 7);
> +f11 (q);
> +f12 (r, 1, 2, 3, 4, 5);
> +f12 (r);
> +
> +int
> +main ()
> +{
> +  return bc + b + cd + d + e + f + ghi + gi + jk + l + m + no + pp + p + qq + q + rrr + rr;
> +}
> --- gcc/testsuite/c-c++-common/cpp/va-opt-3.c.jj	2018-01-09 17:22:32.609158440 +0100
> +++ gcc/testsuite/c-c++-common/cpp/va-opt-3.c	2018-01-09 17:20:14.985142201 +0100
> @@ -0,0 +1,69 @@
> +/* PR preprocessor/83063 */
> +/* PR preprocessor/83708 */
> +/* { dg-do preprocess } */
> +/* { dg-options "-std=gnu99" { target c } } */
> +/* { dg-options "-std=c++2a" { target c++ } } */
> +
> +#define f1(...) b##__VA_OPT__(c)
> +#define f2(...) __VA_OPT__(c)##d
> +#define f3(...) e##__VA_OPT__()
> +#define f4(...) __VA_OPT__()##f
> +#define f5(...) g##__VA_OPT__(h)##i
> +#define f6(...) j##__VA_OPT__()##k
> +#define f7(...) l##__VA_OPT__()
> +#define f8(...) __VA_OPT__()##m
> +#define f9(...) n##__VA_OPT__()##o
> +#define f10(x, ...) x##__VA_OPT__(x)
> +#define f11(x, ...) __VA_OPT__(x)##x
> +#define f12(x, ...) x##__VA_OPT__(x)##x
> +#define f13(...) __VA_OPT__(a)__VA_OPT__(b)c
> +#define f14(a, b, c, ...) __VA_OPT__(a)__VA_OPT__(b)c
> +#define f15(a, b, c, ...) __VA_OPT__(a b)__VA_OPT__(b c)a/**/__VA_OPT__(c a)a
> +t1 f1 (1, 2, 3);
> +/* { dg-final { scan-file va-opt-3.i "t1 bc;" } } */
> +t2 f1 ();
> +/* { dg-final { scan-file va-opt-3.i "t2 b;" } } */
> +t3 f2 (1, 2);
> +/* { dg-final { scan-file va-opt-3.i "t3 cd;" } } */
> +t4 f2 ();
> +/* { dg-final { scan-file va-opt-3.i "t4 d;" } } */
> +t5 f3 (1);
> +/* { dg-final { scan-file va-opt-3.i "t5 e;" } } */
> +t6 f4 (2);
> +/* { dg-final { scan-file va-opt-3.i "t6 f;" } } */
> +t7 f5 (6, 7);
> +/* { dg-final { scan-file va-opt-3.i "t7 ghi;" } } */
> +t8 f5 ();
> +/* { dg-final { scan-file va-opt-3.i "t8 gi;" } } */
> +t9 f6 (8);
> +/* { dg-final { scan-file va-opt-3.i "t9 jk;" } } */
> +t10 f7 ();
> +/* { dg-final { scan-file va-opt-3.i "t10 l;" } } */
> +t11 f8 ();
> +/* { dg-final { scan-file va-opt-3.i "t11 m;" } } */
> +t12 f9 ();
> +/* { dg-final { scan-file va-opt-3.i "t12 no;" } } */
> +t13 f10 (p, 5, 6);
> +/* { dg-final { scan-file va-opt-3.i "t13 pp;" } } */
> +t14 f10 (p);
> +/* { dg-final { scan-file va-opt-3.i "t14 p;" } } */
> +t15 f11 (q, 7);
> +/* { dg-final { scan-file va-opt-3.i "t15 qq;" } } */
> +t16 f11 (q);
> +/* { dg-final { scan-file va-opt-3.i "t16 q;" } } */
> +t17 f12 (r, 1, 2, 3, 4, 5);
> +/* { dg-final { scan-file va-opt-3.i "t17 rrr;" } } */
> +t18 f12 (r);
> +/* { dg-final { scan-file va-opt-3.i "t18 rr;" } } */
> +t19 f13 (1, 2);
> +/* { dg-final { scan-file va-opt-3.i "t19 a b c;" } } */
> +t20 f13 ();
> +/* { dg-final { scan-file va-opt-3.i "t20 c;" } } */
> +t21 f14 (3, 4, 5, 2);
> +/* { dg-final { scan-file va-opt-3.i "t21 3 4 5;" } } */
> +t22 f14 (3, 4, 5);
> +/* { dg-final { scan-file va-opt-3.i "t22 5;" } } */
> +t23 f15 (6, 7, 8, 9);
> +/* { dg-final { scan-file va-opt-3.i "t23 6 7 7 8 6 8 6 6;" } } */
> +t24 f15 (6, 7, 8);
> +/* { dg-final { scan-file va-opt-3.i "t24 6 6;" } } */

	Jakub
Jakub Jelinek Jan. 26, 2018, 2:43 p.m. UTC | #2
Ping^2
http://gcc.gnu.org/ml/gcc-patches/2018-01/msg00727.html

On Wed, Jan 17, 2018 at 05:47:12PM +0100, Jakub Jelinek wrote:
> I'd like to ping this patch.
> As I wrote, it isn't a full solution for all the __VA_OPT__ issues,
> but it isn't even clear to me how exactly it should behave, but fixes
> some ICEs and a couple of most important issues and shouldn't make things
> worse, at least on the gcc and clang __VA_OPT__ testcases.

	Jakub
Jason Merrill Feb. 14, 2018, 4:11 p.m. UTC | #3
On 01/10/2018 07:04 AM, Jakub Jelinek wrote:
> I've also cross-checked the libcpp implementation with this patch against
> trunk clang which apparently also implements __VA_OPT__ now, on the
> testcases included here the output is the same and on their
> macro_vaopt_expand.cpp testcase, if I remove all tests that test
> #__VA_OPT__ ( contents ) handling which we just reject now, there are still
> some differences:
> $ /usr/src/llvm/obj8/bin/clang++  -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/1
> $ ~/src/gcc/obj20/gcc/cc1plus -quiet -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/2
> diff -up /tmp/1 /tmp/2
> -4: f(0 )
> +4: f(0)
> -6: f(0, a )
> -7: f(0, a )
> +6: f(0, a)
> +7: f(0, a)
> -9: TONG C ( ) B ( ) "A()"
> +9: HT_A() C ( ) B ( ) "A()"
> -16: S foo ;
> +16: S foo;
> -26: B1
> -26_1: B1
> +26: B 1
> +26_1: B 1
> -27: B11
> -27_1: BexpandedA0 11
> -28: B11
> +27: B 11
> +27_1: BA0 11
> +28: B 11
> 
> Perhaps some of the whitespace changes aren't significant, but 9:, and
> 2[678]{,_1}: are significantly different.
> 9: is
> #define LPAREN (
> #define A() B LPAREN )
> #define B() C LPAREN )
> #define HT_B() TONG
> #define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
> 9: F(A(),1)
> 
> Thoughts on what is right and why?

clang is.  First we substitute into the body of the __VA_OPT__, so

x x A() #x
B ( ) B ( ) A() "A()"

Then we paste, so

HT_B ( ) B ( ) A() "A()"

then rescan, so

TONG C ( ) B ( ) "A()"

> Similarly for expansion on the last token from __VA_OPT__ when followed
> by ##, like:
> #define m1 (
> #define f16() f17 m1 )
> #define f17() f18 m1 )
> #define f18() m2 m1 )
> #define m3f17() g
> #define f19(x, ...) m3 ## __VA_OPT__(x x f16() #x)
> #define f20(x, ...) __VA_OPT__(x x)##m4()
> #define f21() f17
> #define f17m4() h
> t25 f19 (f16 (), 1);
> t26 f20 (f21 (), 2);
> 
> E.g. 26: is:
> #define F(a,...)  __VA_OPT__(B a ## a) ## 1
> 26: F(,1)
> I really wonder why clang emits B1 in that case, there
> is no ## in between B and a, so those are 2 separate tokens
> separated by whitespace, even when a ## a is a placemarker.
> Does that mean __VA_OPT__ should throw away all the placemarkers
> and return the last non-placemarker token for the ## handling?

This is unclear to me.  The standard says that the replacement for the 
__VA_OPT__ is the result of expansion before rescanning and further 
replacement, but it's unclear to me whether the discarding of 
placemarkers should happen or not, since that is mentioned in the 
context of rescanning.  Representing a placemarker as <> below, we first 
expand the __VA_OPT__ body to

B a ## a
B <> ## <>
B <>

then do we have

B <> ## 1
or
B ## 1
?

Looking at the patch now.

Jason
Jason Merrill Feb. 15, 2018, 6:12 a.m. UTC | #4
On 01/10/2018 07:04 AM, Jakub Jelinek wrote:
> The following patch attempts to fix various issues, including some ICEs,
> by introducing 3 new states, two of them are alternatives to INCLUDE used
> for the very first token after __VA_OPT__( , where we want to take into
> account also flags from the __VA_OPT__ token, and before the closing )
> token where we want to use flags from the closing ) token.  Plus
> PADDING which is used for the case where there are no varargs and __VA_OPT__
> is supposed to fold into a placemarker, or for the case of __VA_OPT__(),
> which is similar to that, in both cases we need to take into account in
> those cases both flags from __VA_OPT__ and from the closing ).

I had an idea for a way to simplify this, which I've attached below.

> This is just a partial fix, one thing this patch doesn't change is that
> the standard says that __VA_OPT__ ( contents ) should be treated as
> parameter, which means that #__VA_OPT__ ( contents ) should stringify it,
> which we right now reject.  My preprocessor knowledge is too limited to
> handle this right myself, including all the corner cases, e.g. one can have
> #define f(x, ...) #__VA_OPT__(#x x ## x) etc..  I presume
> m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
> could be changed into:
> m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE | STRINGIFY_ARG);
> and when handling the PADDING result from update, we could just emit a
> "" token, but for INCLUDE_FIRST with this we'd need something complex,
> probably a new routine similar to stringify_arg to some extent.

Yes, I think long term we really need to treat __VA_OPT__ more like an 
argument.

The first patch below makes your testcases work in what seems to me a 
simpler way: pad when we see __VA_OPT__ if we aren't pasting to the 
left, and fix up the end of the body if we're pasting to the right.

The second further patch below makes the rest of the clang testcase work 
the way it does in clang, apart from stringification.  But it feels more 
kludgey.

Thoughts?

Jason
commit f07a7a181489ad2c6287c239d6b034a3933a56ce
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Feb 14 13:59:22 2018 -0500

            PR preprocessor/83063
    
            PR preprocessor/83708
            * macro.c (vaopt_state): Reorder m_last_was_paste before m_state.
            (vaopt_state::vaopt_state): Adjust.
            (vaopt_state::update_flags): Add BEGIN and END.
            (vaopt_state::update): Return them.
            (retcon_paste_flag, padding_ok_after_last_token): New.
            (replace_args): Handle BEGIN and END.
            (tokens_buff_last_token_ptr): Return NULL if no tokens.

diff --git a/gcc/testsuite/c-c++-common/cpp/va-opt-2.c b/gcc/testsuite/c-c++-common/cpp/va-opt-2.c
new file mode 100644
index 00000000000..cff2d6cbe5d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/va-opt-2.c
@@ -0,0 +1,41 @@
+/* PR preprocessor/83063 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f1(...) int b##__VA_OPT__(c)
+#define f2(...) int __VA_OPT__(c)##d
+#define f3(...) int e##__VA_OPT__()
+#define f4(...) int __VA_OPT__()##f
+#define f5(...) int g##__VA_OPT__(h)##i
+#define f6(...) int j##__VA_OPT__()##k
+#define f7(...) int l##__VA_OPT__()
+#define f8(...) int __VA_OPT__()##m
+#define f9(...) int n##__VA_OPT__()##o
+#define f10(x, ...) int x##__VA_OPT__(x)
+#define f11(x, ...) int __VA_OPT__(x)##x
+#define f12(x, ...) int x##__VA_OPT__(x)##x
+f1 (1, 2, 3);
+f1 ();
+f2 (1, 2);
+f2 ();
+f3 (1);
+f4 (2);
+f5 (6, 7);
+f5 ();
+f6 (8);
+f7 ();
+f8 ();
+f9 ();
+f10 (p, 5, 6);
+f10 (p);
+f11 (q, 7);
+f11 (q);
+f12 (r, 1, 2, 3, 4, 5);
+f12 (r);
+
+int
+main ()
+{
+  return bc + b + cd + d + e + f + ghi + gi + jk + l + m + no + pp + p + qq + q + rrr + rr;
+}
diff --git a/gcc/testsuite/c-c++-common/cpp/va-opt-3.c b/gcc/testsuite/c-c++-common/cpp/va-opt-3.c
new file mode 100644
index 00000000000..2e639891070
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/va-opt-3.c
@@ -0,0 +1,69 @@
+/* PR preprocessor/83063 */
+/* PR preprocessor/83708 */
+/* { dg-do preprocess } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f1(...) b##__VA_OPT__(c)
+#define f2(...) __VA_OPT__(c)##d
+#define f3(...) e##__VA_OPT__()
+#define f4(...) __VA_OPT__()##f
+#define f5(...) g##__VA_OPT__(h)##i
+#define f6(...) j##__VA_OPT__()##k
+#define f7(...) l##__VA_OPT__()
+#define f8(...) __VA_OPT__()##m
+#define f9(...) n##__VA_OPT__()##o
+#define f10(x, ...) x##__VA_OPT__(x)
+#define f11(x, ...) __VA_OPT__(x)##x
+#define f12(x, ...) x##__VA_OPT__(x)##x
+#define f13(...) __VA_OPT__(a)__VA_OPT__(b)c
+#define f14(a, b, c, ...) __VA_OPT__(a)__VA_OPT__(b)c
+#define f15(a, b, c, ...) __VA_OPT__(a b)__VA_OPT__(b c)a/**/__VA_OPT__(c a)a
+t1 f1 (1, 2, 3);
+/* { dg-final { scan-file va-opt-3.i "t1 bc;" } } */
+t2 f1 ();
+/* { dg-final { scan-file va-opt-3.i "t2 b;" } } */
+t3 f2 (1, 2);
+/* { dg-final { scan-file va-opt-3.i "t3 cd;" } } */
+t4 f2 ();
+/* { dg-final { scan-file va-opt-3.i "t4 d;" } } */
+t5 f3 (1);
+/* { dg-final { scan-file va-opt-3.i "t5 e;" } } */
+t6 f4 (2);
+/* { dg-final { scan-file va-opt-3.i "t6 f;" } } */
+t7 f5 (6, 7);
+/* { dg-final { scan-file va-opt-3.i "t7 ghi;" } } */
+t8 f5 ();
+/* { dg-final { scan-file va-opt-3.i "t8 gi;" } } */
+t9 f6 (8);
+/* { dg-final { scan-file va-opt-3.i "t9 jk;" } } */
+t10 f7 ();
+/* { dg-final { scan-file va-opt-3.i "t10 l;" } } */
+t11 f8 ();
+/* { dg-final { scan-file va-opt-3.i "t11 m;" } } */
+t12 f9 ();
+/* { dg-final { scan-file va-opt-3.i "t12 no;" } } */
+t13 f10 (p, 5, 6);
+/* { dg-final { scan-file va-opt-3.i "t13 pp;" } } */
+t14 f10 (p);
+/* { dg-final { scan-file va-opt-3.i "t14 p;" } } */
+t15 f11 (q, 7);
+/* { dg-final { scan-file va-opt-3.i "t15 qq;" } } */
+t16 f11 (q);
+/* { dg-final { scan-file va-opt-3.i "t16 q;" } } */
+t17 f12 (r, 1, 2, 3, 4, 5);
+/* { dg-final { scan-file va-opt-3.i "t17 rrr;" } } */
+t18 f12 (r);
+/* { dg-final { scan-file va-opt-3.i "t18 rr;" } } */
+t19 f13 (1, 2);
+/* { dg-final { scan-file va-opt-3.i "t19 a b c;" } } */
+t20 f13 ();
+/* { dg-final { scan-file va-opt-3.i "t20 c;" } } */
+t21 f14 (3, 4, 5, 2);
+/* { dg-final { scan-file va-opt-3.i "t21 3 4 5;" } } */
+t22 f14 (3, 4, 5);
+/* { dg-final { scan-file va-opt-3.i "t22 5;" } } */
+t23 f15 (6, 7, 8, 9);
+/* { dg-final { scan-file va-opt-3.i "t23 6 7 7 8 6 8 6 6;" } } */
+t24 f15 (6, 7, 8);
+/* { dg-final { scan-file va-opt-3.i "t24 6 6;" } } */
diff --git a/libcpp/macro.c b/libcpp/macro.c
index f994ac584cc..00c2ecf5c0e 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -105,8 +105,8 @@ class vaopt_state {
     : m_pfile (pfile),
     m_allowed (any_args),
     m_variadic (is_variadic),
-    m_state (0),
     m_last_was_paste (false),
+    m_state (0),
     m_paste_location (0),
     m_location (0)
   {
@@ -116,7 +116,9 @@ class vaopt_state {
   {
     ERROR,
     DROP,
-    INCLUDE
+    INCLUDE,
+    BEGIN,
+    END
   };
 
   /* Given a token, update the state of this tracker and return a
@@ -139,7 +141,7 @@ class vaopt_state {
 	  }
 	++m_state;
 	m_location = token->src_loc;
-	return DROP;
+	return BEGIN;
       }
     else if (m_state == 1)
       {
@@ -191,7 +193,7 @@ class vaopt_state {
 		    return ERROR;
 		  }
 
-		return DROP;
+		return END;
 	      }
 	  }
 	return m_allowed ? INCLUDE : DROP;
@@ -220,6 +222,9 @@ class vaopt_state {
   bool m_allowed;
   /* True if the macro is variadic.  */
   bool m_variadic;
+  /* If true, the previous token was ##.  This is used to detect when
+     a paste occurs at the end of the sequence.  */
+  bool m_last_was_paste;
 
   /* The state variable:
      0 means not parsing
@@ -228,9 +233,6 @@ class vaopt_state {
      >= 3 means looking for ")", the number encodes the paren depth.  */
   int m_state;
 
-  /* If true, the previous token was ##.  This is used to detect when
-     a paste occurs at the end of the sequence.  */
-  bool m_last_was_paste;
   /* The location of the paste token.  */
   source_location m_paste_location;
 
@@ -1701,6 +1703,37 @@ expanded_token_index (cpp_reader *pfile, cpp_macro *macro,
   return cur_replacement_token - macro->exp.tokens;
 }
 
+/* Copy whether PASTE_LEFT is set from SRC to *PASTE_FLAG.  */
+
+static void
+retcon_paste_flag (cpp_reader *pfile, const cpp_token **paste_flag,
+		   const cpp_token *src)
+{
+  cpp_token *token = _cpp_temp_token (pfile);
+  token->type = (*paste_flag)->type;
+  token->val = (*paste_flag)->val;
+  if (src->flags & PASTE_LEFT)
+    token->flags = (*paste_flag)->flags | PASTE_LEFT;
+  else
+    token->flags = (*paste_flag)->flags & ~PASTE_LEFT;
+  *paste_flag = token;
+}
+
+/* True IFF the last token emitted into BUFF is such that it's OK to do the
+   usual padding before a macro argument.  */
+
+static bool
+padding_ok_after_last_token (_cpp_buff *buff)
+{
+  if (tokens_buff_count (buff) == 0)
+    /* Don't pad at the start.  */
+    return false;
+  if ((*tokens_buff_last_token_ptr (buff))->flags & PASTE_LEFT)
+    /* Don't pad the RHS of ##.  */
+    return false;
+  return true;
+}
+
 /* Replace the parameters in a function-like macro of NODE with the
    actual ARGS, and place the result in a newly pushed token context.
    Expand each argument before replacing, unless it is operated upon
@@ -1841,8 +1874,51 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
       const cpp_token **tmp_token_ptr;
 
       /* __VA_OPT__ handling.  */
-      if (vaopt_tracker.update (src) != vaopt_state::INCLUDE)
-	continue;
+      vaopt_state::update_type vostate = vaopt_tracker.update (src);
+      if (vostate != vaopt_state::INCLUDE)
+	{
+	  if (vostate == vaopt_state::BEGIN)
+	    {
+	      /* Padding on the left of __VA_OPT__ (unless RHS of ##).  */
+	      if (!padding_ok_after_last_token (buff))
+		continue;
+
+	      const cpp_token *t = padding_token (pfile, src);
+	      unsigned index = expanded_token_index (pfile, macro, src, i);
+	      /* Allocate a virtual location for the padding token and
+		 append the token and its location to BUFF and
+		 VIRT_LOCS.   */
+	      tokens_buff_add_token (buff, virt_locs, t,
+				     t->src_loc, t->src_loc,
+				     map, index);
+	    }
+	  else if (vostate == vaopt_state::END)
+	    {
+	      if (src->flags & PASTE_LEFT)
+		{
+		  /* We want to paste the last actual token, if any.  */
+		  paste_flag = tokens_buff_last_token_ptr (buff);
+		  if (paste_flag && *paste_flag == &pfile->avoid_paste)
+		    {
+		      /* Remove an avoid_paste token appended to an argument in
+			 the body.  */
+		      tokens_buff_remove_last_token (buff);
+		      paste_flag = tokens_buff_last_token_ptr (buff);
+		    }
+		  if (paste_flag && (*paste_flag)->type != CPP_PADDING)
+		    retcon_paste_flag (pfile, paste_flag, src);
+		  continue;
+		}
+
+	      /* Otherwise, avoid paste on RHS, __VA_OPT__(c)d or
+		 __VA_OPT__(c)__VA_OPT__(d).  */
+	      const cpp_token *t = &pfile->avoid_paste;
+	      tokens_buff_add_token (buff, virt_locs,
+				     t, t->src_loc, t->src_loc,
+				     NULL, 0);
+	    }
+	  continue;
+	}
 
       if (src->type != CPP_MACRO_ARG)
 	{
@@ -1938,7 +2014,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 
       /* Padding on the left of an argument (unless RHS of ##).  */
       if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
-	  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT))
+	  && padding_ok_after_last_token (buff))
 	{
 	  const cpp_token *t = padding_token (pfile, src);
 	  unsigned index = expanded_token_index (pfile, macro, src, i);
@@ -2033,16 +2109,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 
       /* Add a new paste flag, or remove an unwanted one.  */
       if (paste_flag)
-	{
-	  cpp_token *token = _cpp_temp_token (pfile);
-	  token->type = (*paste_flag)->type;
-	  token->val = (*paste_flag)->val;
-	  if (src->flags & PASTE_LEFT)
-	    token->flags = (*paste_flag)->flags | PASTE_LEFT;
-	  else
-	    token->flags = (*paste_flag)->flags & ~PASTE_LEFT;
-	  *paste_flag = token;
-	}
+	retcon_paste_flag (pfile, paste_flag, src);
 
       i += arg_tokens_count;
     }
@@ -2213,6 +2280,8 @@ tokens_buff_count (_cpp_buff *buff)
 static const cpp_token **
 tokens_buff_last_token_ptr (_cpp_buff *buff)
 {
+  if (BUFF_FRONT (buff) == buff->base)
+    return NULL;
   return &((const cpp_token **) BUFF_FRONT (buff))[-1];
 }
commit 5b3287ad88178162da8ec33969757357483f995b
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Feb 14 21:50:10 2018 -0500

            * macro.c (replace_args): Avoid more padding.

diff --git a/libcpp/macro.c b/libcpp/macro.c
index 00c2ecf5c0e..ab4743c0736 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -1866,6 +1866,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
   i = 0;
   vaopt_state vaopt_tracker (pfile, macro->variadic,
 			     args[macro->paramc - 1].count > 0);
+  const cpp_token *vaopt_padding = NULL;
   for (src = macro->exp.tokens; src < limit; src++)
     {
       unsigned int arg_tokens_count;
@@ -1891,17 +1892,21 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 	      tokens_buff_add_token (buff, virt_locs, t,
 				     t->src_loc, t->src_loc,
 				     map, index);
+	      vaopt_padding = t;
 	    }
 	  else if (vostate == vaopt_state::END)
 	    {
+	      const cpp_token *pad = vaopt_padding;
+	      vaopt_padding = NULL;
+
 	      if (src->flags & PASTE_LEFT)
 		{
 		  /* We want to paste the last actual token, if any.  */
 		  paste_flag = tokens_buff_last_token_ptr (buff);
-		  if (paste_flag && *paste_flag == &pfile->avoid_paste)
+		  /* Remove any padding from inside the __VA_OPT__.  */
+		  while (paste_flag && (*paste_flag)->type == CPP_PADDING
+			 && *paste_flag != pad)
 		    {
-		      /* Remove an avoid_paste token appended to an argument in
-			 the body.  */
 		      tokens_buff_remove_last_token (buff);
 		      paste_flag = tokens_buff_last_token_ptr (buff);
 		    }
@@ -1998,7 +2003,8 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 		    paste_flag = tmp_token_ptr;
 		}
 	      /* Remove the paste flag if the RHS is a placemarker.  */
-	      else if (arg_tokens_count == 0)
+	      else if (arg_tokens_count == 0
+		       && padding_ok_after_last_token (buff))
 		paste_flag = tmp_token_ptr;
 	    }
 	}
@@ -2010,6 +2016,20 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 						 track_macro_expansion),
 				     MACRO_ARG_TOKEN_EXPANDED,
 				     arg, arg->expanded);
+
+	  if (!padding_ok_after_last_token (buff))
+	    {
+	      /* We're expanding an arg at the beginning of __VA_OPT__ which is
+	         pasted with something before the __VA_OPT__.  Skip padding. */
+	      while (arg_tokens_count)
+		{
+		  const cpp_token *t = macro_arg_token_iter_get_token (&from);
+		  if (t->type != CPP_PADDING)
+		    break;
+		  macro_arg_token_iter_forward (&from);
+		  --arg_tokens_count;
+		}
+	    }
 	}
 
       /* Padding on the left of an argument (unless RHS of ##).  */
@@ -2099,7 +2119,8 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 		     NODE_NAME (node), src->val.macro_arg.arg_no);
 
       /* Avoid paste on RHS (even case count == 0).  */
-      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT))
+      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT)
+	  && padding_ok_after_last_token (buff))
 	{
 	  const cpp_token *t = &pfile->avoid_paste;
 	  tokens_buff_add_token (buff, virt_locs,
Jakub Jelinek Feb. 15, 2018, 9:27 a.m. UTC | #5
On Thu, Feb 15, 2018 at 01:12:08AM -0500, Jason Merrill wrote:
> > This is just a partial fix, one thing this patch doesn't change is that
> > the standard says that __VA_OPT__ ( contents ) should be treated as
> > parameter, which means that #__VA_OPT__ ( contents ) should stringify it,
> > which we right now reject.  My preprocessor knowledge is too limited to
> > handle this right myself, including all the corner cases, e.g. one can have
> > #define f(x, ...) #__VA_OPT__(#x x ## x) etc..  I presume
> > m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
> > could be changed into:
> > m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE | STRINGIFY_ARG);
> > and when handling the PADDING result from update, we could just emit a
> > "" token, but for INCLUDE_FIRST with this we'd need something complex,
> > probably a new routine similar to stringify_arg to some extent.
> 
> Yes, I think long term we really need to treat __VA_OPT__ more like an
> argument.
> 
> The first patch below makes your testcases work in what seems to me a
> simpler way: pad when we see __VA_OPT__ if we aren't pasting to the left,
> and fix up the end of the body if we're pasting to the right.
> 
> The second further patch below makes the rest of the clang testcase work the
> way it does in clang, apart from stringification.  But it feels more
> kludgey.
> 
> Thoughts?

Both patches LGTM, thanks for looking at this.  If you apply the second patch,
you might want to apply also following incremental patch with some additional
tests from my (failed) attempt to extend the patch further (this passes with
your second patch).

--- gcc/testsuite/c-c++-common/cpp/va-opt-3.c	2018-01-09 17:20:14.985142201 +0100
+++ gcc/testsuite/c-c++-common/cpp/va-opt-3.c	2018-01-09 17:54:17.564372639 +0100
@@ -19,6 +19,15 @@
 #define f13(...) __VA_OPT__(a)__VA_OPT__(b)c
 #define f14(a, b, c, ...) __VA_OPT__(a)__VA_OPT__(b)c
 #define f15(a, b, c, ...) __VA_OPT__(a b)__VA_OPT__(b c)a/**/__VA_OPT__(c a)a
+#define m1 (
+#define f16() f17 m1 )
+#define f17() f18 m1 )
+#define f18() m2 m1 )
+#define m3f17() g
+#define f19(x, ...) m3 ## __VA_OPT__(x x f16() #x)
+#define f20(x, ...) __VA_OPT__(x x)##m4()
+#define f21() f17
+#define f17m4() h
 t1 f1 (1, 2, 3);
 /* { dg-final { scan-file va-opt-3.i "t1 bc;" } } */
 t2 f1 ();
@@ -69,0 +79,4 @@
+t25 f19 (f16 (), 1);
+/* { dg-final { scan-file va-opt-3.i "t25 g f18 \\( \\) f17 \\( \\) \"f16 \\(\\)\";" } } */
+t26 f20 (f21 (), 2);
+/* { dg-final { scan-file va-opt-3.i "t26 f17 h;" } } */


	Jakub
Jason Merrill Feb. 15, 2018, 5:42 p.m. UTC | #6
On Thu, Feb 15, 2018 at 4:27 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Feb 15, 2018 at 01:12:08AM -0500, Jason Merrill wrote:
>> > This is just a partial fix, one thing this patch doesn't change is that
>> > the standard says that __VA_OPT__ ( contents ) should be treated as
>> > parameter, which means that #__VA_OPT__ ( contents ) should stringify it,
>> > which we right now reject.  My preprocessor knowledge is too limited to
>> > handle this right myself, including all the corner cases, e.g. one can have
>> > #define f(x, ...) #__VA_OPT__(#x x ## x) etc..  I presume
>> > m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
>> > could be changed into:
>> > m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE | STRINGIFY_ARG);
>> > and when handling the PADDING result from update, we could just emit a
>> > "" token, but for INCLUDE_FIRST with this we'd need something complex,
>> > probably a new routine similar to stringify_arg to some extent.
>>
>> Yes, I think long term we really need to treat __VA_OPT__ more like an
>> argument.
>>
>> The first patch below makes your testcases work in what seems to me a
>> simpler way: pad when we see __VA_OPT__ if we aren't pasting to the left,
>> and fix up the end of the body if we're pasting to the right.
>>
>> The second further patch below makes the rest of the clang testcase work the
>> way it does in clang, apart from stringification.  But it feels more
>> kludgey.
>>
>> Thoughts?
>
> Both patches LGTM, thanks for looking at this.  If you apply the second patch,
> you might want to apply also following incremental patch with some additional
> tests from my (failed) attempt to extend the patch further (this passes with
> your second patch).

Great, thanks.  I kept poking at it this morning; instead of checking
the last emitted token for PASTE_LEFT, this version checks whether
it's at the beginning of __VA_OPT__, which seems safer.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 4a0e6c48c397f4cd6bb654c770f4d8d97de015cc
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Feb 14 13:59:22 2018 -0500

            PR preprocessor/83063 - __VA_OPT__ and ##
    
            PR preprocessor/83708
            * macro.c (vaopt_state): Reorder m_last_was_paste before m_state.
            (vaopt_state::vaopt_state): Adjust.
            (vaopt_state::update_flags): Add BEGIN and END.
            (vaopt_state::update): Return them.
            (copy_paste_flag): Factor out of replace_args.
            (last_token_is): New.
            (replace_args): Handle BEGIN and END.  Avoid padding there.
            (tokens_buff_last_token_ptr): Return NULL if no tokens.

diff --git a/libcpp/macro.c b/libcpp/macro.c
index f994ac584cc..776af7bd00e 100644
--- a/libcpp/macro.c
+++ b/libcpp/macro.c
@@ -105,8 +105,8 @@ class vaopt_state {
     : m_pfile (pfile),
     m_allowed (any_args),
     m_variadic (is_variadic),
-    m_state (0),
     m_last_was_paste (false),
+    m_state (0),
     m_paste_location (0),
     m_location (0)
   {
@@ -116,7 +116,9 @@ class vaopt_state {
   {
     ERROR,
     DROP,
-    INCLUDE
+    INCLUDE,
+    BEGIN,
+    END
   };
 
   /* Given a token, update the state of this tracker and return a
@@ -139,7 +141,7 @@ class vaopt_state {
 	  }
 	++m_state;
 	m_location = token->src_loc;
-	return DROP;
+	return BEGIN;
       }
     else if (m_state == 1)
       {
@@ -191,7 +193,7 @@ class vaopt_state {
 		    return ERROR;
 		  }
 
-		return DROP;
+		return END;
 	      }
 	  }
 	return m_allowed ? INCLUDE : DROP;
@@ -220,6 +222,9 @@ class vaopt_state {
   bool m_allowed;
   /* True if the macro is variadic.  */
   bool m_variadic;
+  /* If true, the previous token was ##.  This is used to detect when
+     a paste occurs at the end of the sequence.  */
+  bool m_last_was_paste;
 
   /* The state variable:
      0 means not parsing
@@ -228,9 +233,6 @@ class vaopt_state {
      >= 3 means looking for ")", the number encodes the paren depth.  */
   int m_state;
 
-  /* If true, the previous token was ##.  This is used to detect when
-     a paste occurs at the end of the sequence.  */
-  bool m_last_was_paste;
   /* The location of the paste token.  */
   source_location m_paste_location;
 
@@ -1701,6 +1703,30 @@ expanded_token_index (cpp_reader *pfile, cpp_macro *macro,
   return cur_replacement_token - macro->exp.tokens;
 }
 
+/* Copy whether PASTE_LEFT is set from SRC to *PASTE_FLAG.  */
+
+static void
+copy_paste_flag (cpp_reader *pfile, const cpp_token **paste_flag,
+		 const cpp_token *src)
+{
+  cpp_token *token = _cpp_temp_token (pfile);
+  token->type = (*paste_flag)->type;
+  token->val = (*paste_flag)->val;
+  if (src->flags & PASTE_LEFT)
+    token->flags = (*paste_flag)->flags | PASTE_LEFT;
+  else
+    token->flags = (*paste_flag)->flags & ~PASTE_LEFT;
+  *paste_flag = token;
+}
+
+/* True IFF the last token emitted into BUFF (if any) is PTR.  */
+
+static bool
+last_token_is (_cpp_buff *buff, const cpp_token **ptr)
+{
+  return (ptr && tokens_buff_last_token_ptr (buff) == ptr);
+}
+
 /* Replace the parameters in a function-like macro of NODE with the
    actual ARGS, and place the result in a newly pushed token context.
    Expand each argument before replacing, unless it is operated upon
@@ -1833,6 +1859,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
   i = 0;
   vaopt_state vaopt_tracker (pfile, macro->variadic,
 			     args[macro->paramc - 1].count > 0);
+  const cpp_token **vaopt_start = NULL;
   for (src = macro->exp.tokens; src < limit; src++)
     {
       unsigned int arg_tokens_count;
@@ -1841,8 +1868,58 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
       const cpp_token **tmp_token_ptr;
 
       /* __VA_OPT__ handling.  */
-      if (vaopt_tracker.update (src) != vaopt_state::INCLUDE)
-	continue;
+      vaopt_state::update_type vostate = vaopt_tracker.update (src);
+      if (vostate != vaopt_state::INCLUDE)
+	{
+	  if (vostate == vaopt_state::BEGIN)
+	    {
+	      /* Padding on the left of __VA_OPT__ (unless RHS of ##).  */
+	      if (src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT))
+		{
+		  const cpp_token *t = padding_token (pfile, src);
+		  unsigned index = expanded_token_index (pfile, macro, src, i);
+		  /* Allocate a virtual location for the padding token and
+		     append the token and its location to BUFF and
+		     VIRT_LOCS.   */
+		  tokens_buff_add_token (buff, virt_locs, t,
+					 t->src_loc, t->src_loc,
+					 map, index);
+		}
+	      vaopt_start = tokens_buff_last_token_ptr (buff);
+	    }
+	  else if (vostate == vaopt_state::END)
+	    {
+	      const cpp_token **start = vaopt_start;
+	      vaopt_start = NULL;
+
+	      /* Remove any tail padding from inside the __VA_OPT__.  */
+	      paste_flag = tokens_buff_last_token_ptr (buff);
+	      while (paste_flag && paste_flag != start
+		     && (*paste_flag)->type == CPP_PADDING)
+		{
+		  tokens_buff_remove_last_token (buff);
+		  paste_flag = tokens_buff_last_token_ptr (buff);
+		}
+
+	      if (src->flags & PASTE_LEFT)
+		{
+		  /* With a non-empty __VA_OPT__ on the LHS of ##, the last
+		     token should be flagged PASTE_LEFT.  */
+		  if (paste_flag && (*paste_flag)->type != CPP_PADDING)
+		    copy_paste_flag (pfile, paste_flag, src);
+		}
+	      else
+		{
+		  /* Otherwise, avoid paste on RHS, __VA_OPT__(c)d or
+		     __VA_OPT__(c)__VA_OPT__(d).  */
+		  const cpp_token *t = &pfile->avoid_paste;
+		  tokens_buff_add_token (buff, virt_locs,
+					 t, t->src_loc, t->src_loc,
+					 NULL, 0);
+		}
+	    }
+	  continue;
+	}
 
       if (src->type != CPP_MACRO_ARG)
 	{
@@ -1921,8 +1998,11 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 		  else
 		    paste_flag = tmp_token_ptr;
 		}
-	      /* Remove the paste flag if the RHS is a placemarker.  */
-	      else if (arg_tokens_count == 0)
+	      /* Remove the paste flag if the RHS is a placemarker, unless the
+		 previous emitted token is at the beginning of __VA_OPT__;
+		 placemarkers within __VA_OPT__ are ignored in that case.  */
+	      else if (arg_tokens_count == 0
+		       && tmp_token_ptr != vaopt_start)
 		paste_flag = tmp_token_ptr;
 	    }
 	}
@@ -1934,11 +2014,26 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 						 track_macro_expansion),
 				     MACRO_ARG_TOKEN_EXPANDED,
 				     arg, arg->expanded);
+
+	  if (last_token_is (buff, vaopt_start))
+	    {
+	      /* We're expanding an arg at the beginning of __VA_OPT__.
+		 Skip padding. */
+	      while (arg_tokens_count)
+		{
+		  const cpp_token *t = macro_arg_token_iter_get_token (&from);
+		  if (t->type != CPP_PADDING)
+		    break;
+		  macro_arg_token_iter_forward (&from);
+		  --arg_tokens_count;
+		}
+	    }
 	}
 
       /* Padding on the left of an argument (unless RHS of ##).  */
       if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
-	  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT))
+	  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT)
+	  && !last_token_is (buff, vaopt_start))
 	{
 	  const cpp_token *t = padding_token (pfile, src);
 	  unsigned index = expanded_token_index (pfile, macro, src, i);
@@ -2023,7 +2118,8 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 		     NODE_NAME (node), src->val.macro_arg.arg_no);
 
       /* Avoid paste on RHS (even case count == 0).  */
-      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT))
+      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT)
+	  && !last_token_is (buff, vaopt_start))
 	{
 	  const cpp_token *t = &pfile->avoid_paste;
 	  tokens_buff_add_token (buff, virt_locs,
@@ -2033,16 +2129,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
 
       /* Add a new paste flag, or remove an unwanted one.  */
       if (paste_flag)
-	{
-	  cpp_token *token = _cpp_temp_token (pfile);
-	  token->type = (*paste_flag)->type;
-	  token->val = (*paste_flag)->val;
-	  if (src->flags & PASTE_LEFT)
-	    token->flags = (*paste_flag)->flags | PASTE_LEFT;
-	  else
-	    token->flags = (*paste_flag)->flags & ~PASTE_LEFT;
-	  *paste_flag = token;
-	}
+	copy_paste_flag (pfile, paste_flag, src);
 
       i += arg_tokens_count;
     }
@@ -2213,6 +2300,8 @@ tokens_buff_count (_cpp_buff *buff)
 static const cpp_token **
 tokens_buff_last_token_ptr (_cpp_buff *buff)
 {
+  if (BUFF_FRONT (buff) == buff->base)
+    return NULL;
   return &((const cpp_token **) BUFF_FRONT (buff))[-1];
 }
 
diff --git a/gcc/testsuite/c-c++-common/cpp/va-opt-2.c b/gcc/testsuite/c-c++-common/cpp/va-opt-2.c
new file mode 100644
index 00000000000..cff2d6cbe5d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/va-opt-2.c
@@ -0,0 +1,41 @@
+/* PR preprocessor/83063 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f1(...) int b##__VA_OPT__(c)
+#define f2(...) int __VA_OPT__(c)##d
+#define f3(...) int e##__VA_OPT__()
+#define f4(...) int __VA_OPT__()##f
+#define f5(...) int g##__VA_OPT__(h)##i
+#define f6(...) int j##__VA_OPT__()##k
+#define f7(...) int l##__VA_OPT__()
+#define f8(...) int __VA_OPT__()##m
+#define f9(...) int n##__VA_OPT__()##o
+#define f10(x, ...) int x##__VA_OPT__(x)
+#define f11(x, ...) int __VA_OPT__(x)##x
+#define f12(x, ...) int x##__VA_OPT__(x)##x
+f1 (1, 2, 3);
+f1 ();
+f2 (1, 2);
+f2 ();
+f3 (1);
+f4 (2);
+f5 (6, 7);
+f5 ();
+f6 (8);
+f7 ();
+f8 ();
+f9 ();
+f10 (p, 5, 6);
+f10 (p);
+f11 (q, 7);
+f11 (q);
+f12 (r, 1, 2, 3, 4, 5);
+f12 (r);
+
+int
+main ()
+{
+  return bc + b + cd + d + e + f + ghi + gi + jk + l + m + no + pp + p + qq + q + rrr + rr;
+}
diff --git a/gcc/testsuite/c-c++-common/cpp/va-opt-3.c b/gcc/testsuite/c-c++-common/cpp/va-opt-3.c
new file mode 100644
index 00000000000..1a5a7b2383e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/cpp/va-opt-3.c
@@ -0,0 +1,94 @@
+/* PR preprocessor/83063 */
+/* PR preprocessor/83708 */
+/* { dg-do preprocess } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f1(...) b##__VA_OPT__(c)
+#define f2(...) __VA_OPT__(c)##d
+#define f3(...) e##__VA_OPT__()
+#define f4(...) __VA_OPT__()##f
+#define f5(...) g##__VA_OPT__(h)##i
+#define f6(...) j##__VA_OPT__()##k
+#define f7(...) l##__VA_OPT__()
+#define f8(...) __VA_OPT__()##m
+#define f9(...) n##__VA_OPT__()##o
+#define f10(x, ...) x##__VA_OPT__(x)
+#define f11(x, ...) __VA_OPT__(x)##x
+#define f12(x, ...) x##__VA_OPT__(x)##x
+#define f13(...) __VA_OPT__(a)__VA_OPT__(b)c
+#define f14(a, b, c, ...) __VA_OPT__(a)__VA_OPT__(b)c
+#define f15(a, b, c, ...) __VA_OPT__(a b)__VA_OPT__(b c)a/**/__VA_OPT__(c a)a
+#define m1 (
+#define f16() f17 m1 )
+#define f17() f18 m1 )
+#define f18() m2 m1 )
+#define m3f17() g
+#define f19(x, ...) m3 ## __VA_OPT__(x x f16() #x)
+#define f20(x, ...) __VA_OPT__(x x)##m4()
+#define f21() f17
+#define f17m4() h
+#define f22(x,...)  1 ## __VA_OPT__(x ## x 2) ## 3
+#define f23(x,...)  1 ## __VA_OPT__(x 2) ## 3
+#define f24(x,...)  1 ## __VA_OPT__(2 x) ## 3
+#define f25(x,...)  1 ## __VA_OPT__(2 x ## x) ## 3
+t1 f1 (1, 2, 3);
+/* { dg-final { scan-file va-opt-3.i "t1 bc;" } } */
+t2 f1 ();
+/* { dg-final { scan-file va-opt-3.i "t2 b;" } } */
+t3 f2 (1, 2);
+/* { dg-final { scan-file va-opt-3.i "t3 cd;" } } */
+t4 f2 ();
+/* { dg-final { scan-file va-opt-3.i "t4 d;" } } */
+t5 f3 (1);
+/* { dg-final { scan-file va-opt-3.i "t5 e;" } } */
+t6 f4 (2);
+/* { dg-final { scan-file va-opt-3.i "t6 f;" } } */
+t7 f5 (6, 7);
+/* { dg-final { scan-file va-opt-3.i "t7 ghi;" } } */
+t8 f5 ();
+/* { dg-final { scan-file va-opt-3.i "t8 gi;" } } */
+t9 f6 (8);
+/* { dg-final { scan-file va-opt-3.i "t9 jk;" } } */
+t10 f7 ();
+/* { dg-final { scan-file va-opt-3.i "t10 l;" } } */
+t11 f8 ();
+/* { dg-final { scan-file va-opt-3.i "t11 m;" } } */
+t12 f9 ();
+/* { dg-final { scan-file va-opt-3.i "t12 no;" } } */
+t13 f10 (p, 5, 6);
+/* { dg-final { scan-file va-opt-3.i "t13 pp;" } } */
+t14 f10 (p);
+/* { dg-final { scan-file va-opt-3.i "t14 p;" } } */
+t15 f11 (q, 7);
+/* { dg-final { scan-file va-opt-3.i "t15 qq;" } } */
+t16 f11 (q);
+/* { dg-final { scan-file va-opt-3.i "t16 q;" } } */
+t17 f12 (r, 1, 2, 3, 4, 5);
+/* { dg-final { scan-file va-opt-3.i "t17 rrr;" } } */
+t18 f12 (r);
+/* { dg-final { scan-file va-opt-3.i "t18 rr;" } } */
+t19 f13 (1, 2);
+/* { dg-final { scan-file va-opt-3.i "t19 a b c;" } } */
+t20 f13 ();
+/* { dg-final { scan-file va-opt-3.i "t20 c;" } } */
+t21 f14 (3, 4, 5, 2);
+/* { dg-final { scan-file va-opt-3.i "t21 3 4 5;" } } */
+t22 f14 (3, 4, 5);
+/* { dg-final { scan-file va-opt-3.i "t22 5;" } } */
+t23 f15 (6, 7, 8, 9);
+/* { dg-final { scan-file va-opt-3.i "t23 6 7 7 8 6 8 6 6;" } } */
+t24 f15 (6, 7, 8);
+/* { dg-final { scan-file va-opt-3.i "t24 6 6;" } } */
+t25 f19 (f16 (), 1);
+/* { dg-final { scan-file va-opt-3.i {t25 g f18 \( \) f17 \( \) "f16 \(\)";} } } */
+t26 f20 (f21 (), 2);
+/* { dg-final { scan-file va-opt-3.i "t26 f17 h;" } } */
+t27 f22 (, x);
+/* { dg-final { scan-file va-opt-3.i "t27 123;" } } */
+t28 f23 (, x);
+/* { dg-final { scan-file va-opt-3.i "t28 123;" } } */
+t29 f24 (, x);
+/* { dg-final { scan-file va-opt-3.i "t29 123;" } } */
+t30 f25 (, x);
+/* { dg-final { scan-file va-opt-3.i "t30 123;" } } */
diff mbox series

Patch

diff -up /tmp/1 /tmp/2
-4: f(0 )
+4: f(0)
-6: f(0, a )
-7: f(0, a )
+6: f(0, a)
+7: f(0, a)
-9: TONG C ( ) B ( ) "A()"
+9: HT_A() C ( ) B ( ) "A()"
-16: S foo ;
+16: S foo;
-26: B1
-26_1: B1
+26: B 1
+26_1: B 1
-27: B11
-27_1: BexpandedA0 11
-28: B11
+27: B 11
+27_1: BA0 11
+28: B 11

Perhaps some of the whitespace changes aren't significant, but 9:, and
2[678]{,_1}: are significantly different.
9: is
#define LPAREN ( 
#define A() B LPAREN )
#define B() C LPAREN )
#define HT_B() TONG
#define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
9: F(A(),1)

Thoughts on what is right and why?
Similarly for expansion on the last token from __VA_OPT__ when followed
by ##, like:
#define m1 (
#define f16() f17 m1 )
#define f17() f18 m1 )
#define f18() m2 m1 )
#define m3f17() g
#define f19(x, ...) m3 ## __VA_OPT__(x x f16() #x)
#define f20(x, ...) __VA_OPT__(x x)##m4()
#define f21() f17
#define f17m4() h
t25 f19 (f16 (), 1);
t26 f20 (f21 (), 2);

E.g. 26: is:
#define F(a,...)  __VA_OPT__(B a ## a) ## 1
26: F(,1)
I really wonder why clang emits B1 in that case, there
is no ## in between B and a, so those are 2 separate tokens
separated by whitespace, even when a ## a is a placemarker.
Does that mean __VA_OPT__ should throw away all the placemarkers
and return the last non-placemarker token for the ## handling?

Can somebody please take the rest over?

BTW, Tom, perhaps you should update your MAINTAINERS entry email address...

2018-01-10  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/83063
	PR preprocessor/83708
	* macro.c (enum macro_arg_token_kind): Fix comment typo.
	(vaopt_state): Add m_flags field, reorder m_last_was_paste before
	m_state.
	(vaopt_state::vaopt_state): Adjust for the above changes.
	(vaopt_state::update_flags): Add INCLUDE_FIRST, INCLUDE_LAST and
	PADDING.
	(vaopt_state::update): Add limit argument, update m_flags member,
	return INCLUDE_FIRST instead of INCLUDE on the first and INCLUDE_LAST
	instead of INCLUDE on the last token between __VA_OPT__( and ).
	Returnn PADDING instead of DROP on the closing ) if no tokens were
	INCLUDE*.  Comment spelling fixes.
	(vaopt_state::flags): New method.
	(replace_args): Adjust vaopt_state::update caller.  Fix handling
	of __VA_OPT__ preceded or followed by ## and ensure no paste after
	last __VA_OPT__ token if not followed by ##.  Formatting fix.
	(create_iso_definition): Adjust vaopt_state::update caller.

	* c-c++-common/cpp/va-opt-2.c: New test.
	* c-c++-common/cpp/va-opt-3.c: New test.

--- libcpp/macro.c.jj	2018-01-03 10:42:55.938763447 +0100
+++ libcpp/macro.c	2018-01-09 17:27:23.603191796 +0100
@@ -51,7 +51,7 @@  struct macro_arg
 enum macro_arg_token_kind {
   MACRO_ARG_TOKEN_NORMAL,
   /* This is a macro argument token that got transformed into a string
-     litteral, e.g. #foo.  */
+     literal, e.g. #foo.  */
   MACRO_ARG_TOKEN_STRINGIFIED,
   /* This is a token resulting from the expansion of a macro
      argument that was itself a macro.  */
@@ -105,10 +105,11 @@  class vaopt_state {
     : m_pfile (pfile),
     m_allowed (any_args),
     m_variadic (is_variadic),
-    m_state (0),
     m_last_was_paste (false),
+    m_state (0),
     m_paste_location (0),
-    m_location (0)
+    m_location (0),
+    m_flags (0)
   {
   }
 
@@ -116,13 +117,16 @@  class vaopt_state {
   {
     ERROR,
     DROP,
-    INCLUDE
+    INCLUDE,
+    INCLUDE_FIRST,
+    INCLUDE_LAST,
+    PADDING
   };
 
   /* Given a token, update the state of this tracker and return a
      boolean indicating whether the token should be be included in the
      expansion.  */
-  update_type update (const cpp_token *token)
+  update_type update (const cpp_token *token, const cpp_token *limit)
   {
     /* If the macro isn't variadic, just don't bother.  */
     if (!m_variadic)
@@ -139,6 +143,7 @@  class vaopt_state {
 	  }
 	++m_state;
 	m_location = token->src_loc;
+	m_flags = token->flags & (PREV_FALLTHROUGH | PREV_WHITE);
 	return DROP;
       }
     else if (m_state == 1)
@@ -155,6 +160,7 @@  class vaopt_state {
       }
     else if (m_state >= 2)
       {
+	update_type ret = INCLUDE;
 	if (m_state == 2 && token->type == CPP_PASTE)
 	  {
 	    cpp_error_at (m_pfile, CPP_DL_ERROR, token->src_loc,
@@ -165,7 +171,10 @@  class vaopt_state {
 	   case we see a close paren immediately after the open
 	   paren.  */
 	if (m_state == 2)
-	  ++m_state;
+	  {
+	    ++m_state;
+	    ret = INCLUDE_FIRST;
+	  }
 
 	bool was_paste = m_last_was_paste;
 	m_last_was_paste = false;
@@ -191,10 +200,27 @@  class vaopt_state {
 		    return ERROR;
 		  }
 
-		return DROP;
+		if (ret == INCLUDE_FIRST)
+		  {
+		    m_flags |= token->flags & (PASTE_LEFT | SP_DIGRAPH
+					       | SP_PREV_WHITE);
+		    return PADDING;
+		  }
+		return m_allowed ? DROP : PADDING;
 	      }
 	  }
-	return m_allowed ? INCLUDE : DROP;
+	if (limit
+	    && m_state == 3
+	    && token + 1 < limit
+	    && token[1].type == CPP_CLOSE_PAREN)
+	  {
+	    if (m_allowed && ret != INCLUDE_FIRST)
+	      m_flags = 0;
+	    m_flags |= token[1].flags & (PASTE_LEFT | SP_DIGRAPH
+					 | SP_PREV_WHITE);
+	    ret = INCLUDE_LAST;
+	  }
+	return m_allowed ? ret : DROP;
       }
 
     /* Nothing to do with __VA_OPT__.  */
@@ -211,6 +237,12 @@  class vaopt_state {
     return m_state == 0;
   }
 
+  /* Return any flags that should be ored to the current token.  */
+  unsigned short flags ()
+  {
+    return m_flags;
+  }
+
  private:
 
   /* The cpp_reader.  */
@@ -220,6 +252,9 @@  class vaopt_state {
   bool m_allowed;
   /* True if the macro is variadic.  */
   bool m_variadic;
+  /* If true, the previous token was ##.  This is used to detect when
+     a paste occurs at the end of the sequence.  */
+  bool m_last_was_paste;
 
   /* The state variable:
      0 means not parsing
@@ -228,14 +263,14 @@  class vaopt_state {
      >= 3 means looking for ")", the number encodes the paren depth.  */
   int m_state;
 
-  /* If true, the previous token was ##.  This is used to detect when
-     a paste occurs at the end of the sequence.  */
-  bool m_last_was_paste;
   /* The location of the paste token.  */
   source_location m_paste_location;
 
   /* Location of the __VA_OPT__ token.  */
   source_location m_location;
+
+  /* The flags from the __VA_OPT__ token and closing ).  */
+  unsigned short m_flags;
 };
 
 /* Macro expansion.  */
@@ -1817,9 +1852,9 @@  replace_args (cpp_reader *pfile, cpp_has
 	   multiple tokens. This is to save memory at the expense of
 	   accuracy.
 
-	   Suppose we have #define SQARE(A) A * A
+	   Suppose we have #define SQUARE(A) A * A
 
-	   And then we do SQARE(2+3)
+	   And then we do SQUARE(2+3)
 
 	   Then the tokens 2, +, 3, will have the same location,
 	   saying they come from the expansion of the argument A.  */
@@ -1831,16 +1866,70 @@  replace_args (cpp_reader *pfile, cpp_has
   i = 0;
   vaopt_state vaopt_tracker (pfile, macro->variadic,
 			     args[macro->paramc - 1].count > 0);
+  unsigned short prev_flags = 0;
   for (src = macro->exp.tokens; src < limit; src++)
     {
       unsigned int arg_tokens_count;
       macro_arg_token_iter from;
       const cpp_token **paste_flag = NULL;
       const cpp_token **tmp_token_ptr;
+      unsigned short flags = src->flags;
 
       /* __VA_OPT__ handling.  */
-      if (vaopt_tracker.update (src) != vaopt_state::INCLUDE)
-	continue;
+      vaopt_state::update_type vaopt_state = vaopt_tracker.update (src, limit);
+      if (vaopt_state != vaopt_state::INCLUDE)
+	{
+	  if (vaopt_state == vaopt_state::PADDING)
+	    {
+	      flags = vaopt_tracker.flags ();
+	      if ((flags & PASTE_LEFT) && (prev_flags & PASTE_LEFT))
+		continue;
+	      flags &= ~PASTE_LEFT;
+	      cpp_token *t = (cpp_token *) padding_token (pfile, src);
+	      unsigned index = expanded_token_index (pfile, macro, src, i);
+	      t->flags |= flags;
+	      /* Allocate a virtual location for the padding token and
+		 append the token and its location to BUFF and
+		 VIRT_LOCS.   */
+	      tokens_buff_add_token (buff, virt_locs, t,
+				     t->src_loc, t->src_loc,
+				     map, index);
+	      prev_flags = flags;
+	      continue;
+	    }
+	  else if (vaopt_state == vaopt_state::INCLUDE_FIRST
+		   || vaopt_state == vaopt_state::INCLUDE_LAST)
+	    {
+	      unsigned short vaopt_flags = vaopt_tracker.flags ();
+	      if (src->type != CPP_MACRO_ARG)
+		{
+		  cpp_token *t = _cpp_temp_token (pfile);
+		  t->type = src->type;
+		  t->val = src->val;
+		  t->flags = flags | vaopt_flags;
+		  unsigned index = expanded_token_index (pfile, macro, src, i);
+		  tokens_buff_add_token (buff, virt_locs, t,
+					 t->src_loc, t->src_loc, map, index);
+		  i += 1;
+		  prev_flags = t->flags;
+		  /* Avoid paste on RHS, __VA_OPT__(c)d or
+		     __VA_OPT__(c)__VA_OPT__(d).  */
+		  if (!(vaopt_flags & PASTE_LEFT)
+		      && vaopt_state == vaopt_state::INCLUDE_LAST)
+		    {
+		      const cpp_token *t = &pfile->avoid_paste;
+		      tokens_buff_add_token (buff, virt_locs,
+					     t, t->src_loc, t->src_loc,
+					     NULL, 0);
+		    }
+		  continue;
+		}
+	      else
+		flags |= vaopt_flags;
+	    }
+	  else
+	    continue;
+	}
 
       if (src->type != CPP_MACRO_ARG)
 	{
@@ -1852,6 +1941,7 @@  replace_args (cpp_reader *pfile, cpp_has
 				 src->src_loc, src->src_loc,
 				 map, index);
 	  i += 1;
+	  prev_flags = flags;
 	  continue;
 	}
 
@@ -1866,7 +1956,7 @@  replace_args (cpp_reader *pfile, cpp_has
 	 below is to initialize the iterator depending on the type of
 	 tokens the macro argument has.  It also does some adjustment
 	 related to padding tokens and some pasting corner cases.  */
-      if (src->flags & STRINGIFY_ARG)
+      if (flags & STRINGIFY_ARG)
 	{
 	  arg_tokens_count = 1;
 	  macro_arg_token_iter_init (&from,
@@ -1875,7 +1965,7 @@  replace_args (cpp_reader *pfile, cpp_has
 				     MACRO_ARG_TOKEN_STRINGIFIED,
 				     arg, &arg->stringified);
 	}
-      else if (src->flags & PASTE_LEFT)
+      else if (flags & PASTE_LEFT)
 	{
 	  arg_tokens_count = arg->count;
 	  macro_arg_token_iter_init (&from,
@@ -1884,7 +1974,7 @@  replace_args (cpp_reader *pfile, cpp_has
 				     MACRO_ARG_TOKEN_NORMAL,
 				     arg, arg->first);
 	}
-      else if (src != macro->exp.tokens && (src[-1].flags & PASTE_LEFT))
+      else if (prev_flags & PASTE_LEFT)
 	{
 	  int num_toks;
 	  arg_tokens_count = arg->count;
@@ -1936,7 +2026,7 @@  replace_args (cpp_reader *pfile, cpp_has
 
       /* Padding on the left of an argument (unless RHS of ##).  */
       if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
-	  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT))
+	  && src != macro->exp.tokens && !(prev_flags & PASTE_LEFT))
 	{
 	  const cpp_token *t = padding_token (pfile, src);
 	  unsigned index = expanded_token_index (pfile, macro, src, i);
@@ -1960,9 +2050,9 @@  replace_args (cpp_reader *pfile, cpp_has
 		 save extra memory while tracking macro expansion
 		 locations.  So in that case here is what we do:
 
-		 Suppose we have #define SQARE(A) A * A
+		 Suppose we have #define SQUARE(A) A * A
 
-		 And then we do SQARE(2+3)
+		 And then we do SQUARE(2+3)
 
 		 Then the tokens 2, +, 3, will have the same location,
 		 saying they come from the expansion of the argument
@@ -1970,9 +2060,9 @@  replace_args (cpp_reader *pfile, cpp_has
 
 	      So that means we are going to ignore the COUNT tokens
 	      resulting from the expansion of the current macro
-	      arugment. In other words all the ARG_TOKENS_COUNT tokens
+	      argument. In other words all the ARG_TOKENS_COUNT tokens
 	      resulting from the expansion of the macro argument will
-	      have the index I. Normally, each of those token should
+	      have the index I.  Normally, each of those token should
 	      have index I+J.  */
 	      unsigned token_index = i;
 	      unsigned index;
@@ -1989,9 +2079,9 @@  replace_args (cpp_reader *pfile, cpp_has
 
 	  /* With a non-empty argument on the LHS of ##, the last
 	     token should be flagged PASTE_LEFT.  */
-	  if (src->flags & PASTE_LEFT)
-	    paste_flag =
-	      (const cpp_token **) tokens_buff_last_token_ptr (buff);
+	  if (flags & PASTE_LEFT)
+	    paste_flag
+	      = (const cpp_token **) tokens_buff_last_token_ptr (buff);
 	}
       else if (CPP_PEDANTIC (pfile) && ! CPP_OPTION (pfile, c99)
 	       && ! macro->syshdr && ! cpp_in_system_header (pfile))
@@ -2021,7 +2111,7 @@  replace_args (cpp_reader *pfile, cpp_has
 		     NODE_NAME (node), src->val.macro_arg.arg_no);
 
       /* Avoid paste on RHS (even case count == 0).  */
-      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT))
+      if (!pfile->state.in_directive && !(flags & PASTE_LEFT))
 	{
 	  const cpp_token *t = &pfile->avoid_paste;
 	  tokens_buff_add_token (buff, virt_locs,
@@ -2035,7 +2125,7 @@  replace_args (cpp_reader *pfile, cpp_has
 	  cpp_token *token = _cpp_temp_token (pfile);
 	  token->type = (*paste_flag)->type;
 	  token->val = (*paste_flag)->val;
-	  if (src->flags & PASTE_LEFT)
+	  if (flags & PASTE_LEFT)
 	    token->flags = (*paste_flag)->flags | PASTE_LEFT;
 	  else
 	    token->flags = (*paste_flag)->flags & ~PASTE_LEFT;
@@ -2043,6 +2133,7 @@  replace_args (cpp_reader *pfile, cpp_has
 	}
 
       i += arg_tokens_count;
+      prev_flags = flags;
     }
 
   if (track_macro_exp)
@@ -3304,7 +3395,7 @@  create_iso_definition (cpp_reader *pfile
 	    }
 	}
 
-      if (vaopt_tracker.update (token) == vaopt_state::ERROR)
+      if (vaopt_tracker.update (token, NULL) == vaopt_state::ERROR)
 	return false;
 
       following_paste_op = (token->type == CPP_PASTE);
--- gcc/testsuite/c-c++-common/cpp/va-opt-2.c.jj	2018-01-09 13:06:56.721345854 +0100
+++ gcc/testsuite/c-c++-common/cpp/va-opt-2.c	2018-01-09 17:13:35.759095064 +0100
@@ -0,0 +1,41 @@ 
+/* PR preprocessor/83063 */
+/* { dg-do compile } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f1(...) int b##__VA_OPT__(c)
+#define f2(...) int __VA_OPT__(c)##d
+#define f3(...) int e##__VA_OPT__()
+#define f4(...) int __VA_OPT__()##f
+#define f5(...) int g##__VA_OPT__(h)##i
+#define f6(...) int j##__VA_OPT__()##k
+#define f7(...) int l##__VA_OPT__()
+#define f8(...) int __VA_OPT__()##m
+#define f9(...) int n##__VA_OPT__()##o
+#define f10(x, ...) int x##__VA_OPT__(x)
+#define f11(x, ...) int __VA_OPT__(x)##x
+#define f12(x, ...) int x##__VA_OPT__(x)##x
+f1 (1, 2, 3);
+f1 ();
+f2 (1, 2);
+f2 ();
+f3 (1);
+f4 (2);
+f5 (6, 7);
+f5 ();
+f6 (8);
+f7 ();
+f8 ();
+f9 ();
+f10 (p, 5, 6);
+f10 (p);
+f11 (q, 7);
+f11 (q);
+f12 (r, 1, 2, 3, 4, 5);
+f12 (r);
+
+int
+main ()
+{
+  return bc + b + cd + d + e + f + ghi + gi + jk + l + m + no + pp + p + qq + q + rrr + rr;
+}
--- gcc/testsuite/c-c++-common/cpp/va-opt-3.c.jj	2018-01-09 17:22:32.609158440 +0100
+++ gcc/testsuite/c-c++-common/cpp/va-opt-3.c	2018-01-09 17:20:14.985142201 +0100
@@ -0,0 +1,69 @@ 
+/* PR preprocessor/83063 */
+/* PR preprocessor/83708 */
+/* { dg-do preprocess } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f1(...) b##__VA_OPT__(c)
+#define f2(...) __VA_OPT__(c)##d
+#define f3(...) e##__VA_OPT__()
+#define f4(...) __VA_OPT__()##f
+#define f5(...) g##__VA_OPT__(h)##i
+#define f6(...) j##__VA_OPT__()##k
+#define f7(...) l##__VA_OPT__()
+#define f8(...) __VA_OPT__()##m
+#define f9(...) n##__VA_OPT__()##o
+#define f10(x, ...) x##__VA_OPT__(x)
+#define f11(x, ...) __VA_OPT__(x)##x
+#define f12(x, ...) x##__VA_OPT__(x)##x
+#define f13(...) __VA_OPT__(a)__VA_OPT__(b)c
+#define f14(a, b, c, ...) __VA_OPT__(a)__VA_OPT__(b)c
+#define f15(a, b, c, ...) __VA_OPT__(a b)__VA_OPT__(b c)a/**/__VA_OPT__(c a)a
+t1 f1 (1, 2, 3);
+/* { dg-final { scan-file va-opt-3.i "t1 bc;" } } */
+t2 f1 ();
+/* { dg-final { scan-file va-opt-3.i "t2 b;" } } */
+t3 f2 (1, 2);
+/* { dg-final { scan-file va-opt-3.i "t3 cd;" } } */
+t4 f2 ();
+/* { dg-final { scan-file va-opt-3.i "t4 d;" } } */
+t5 f3 (1);
+/* { dg-final { scan-file va-opt-3.i "t5 e;" } } */
+t6 f4 (2);
+/* { dg-final { scan-file va-opt-3.i "t6 f;" } } */
+t7 f5 (6, 7);
+/* { dg-final { scan-file va-opt-3.i "t7 ghi;" } } */
+t8 f5 ();
+/* { dg-final { scan-file va-opt-3.i "t8 gi;" } } */
+t9 f6 (8);
+/* { dg-final { scan-file va-opt-3.i "t9 jk;" } } */
+t10 f7 ();
+/* { dg-final { scan-file va-opt-3.i "t10 l;" } } */
+t11 f8 ();
+/* { dg-final { scan-file va-opt-3.i "t11 m;" } } */
+t12 f9 ();
+/* { dg-final { scan-file va-opt-3.i "t12 no;" } } */
+t13 f10 (p, 5, 6);
+/* { dg-final { scan-file va-opt-3.i "t13 pp;" } } */
+t14 f10 (p);
+/* { dg-final { scan-file va-opt-3.i "t14 p;" } } */
+t15 f11 (q, 7);
+/* { dg-final { scan-file va-opt-3.i "t15 qq;" } } */
+t16 f11 (q);
+/* { dg-final { scan-file va-opt-3.i "t16 q;" } } */
+t17 f12 (r, 1, 2, 3, 4, 5);
+/* { dg-final { scan-file va-opt-3.i "t17 rrr;" } } */
+t18 f12 (r);
+/* { dg-final { scan-file va-opt-3.i "t18 rr;" } } */
+t19 f13 (1, 2);
+/* { dg-final { scan-file va-opt-3.i "t19 a b c;" } } */
+t20 f13 ();
+/* { dg-final { scan-file va-opt-3.i "t20 c;" } } */
+t21 f14 (3, 4, 5, 2);
+/* { dg-final { scan-file va-opt-3.i "t21 3 4 5;" } } */
+t22 f14 (3, 4, 5);
+/* { dg-final { scan-file va-opt-3.i "t22 5;" } } */
+t23 f15 (6, 7, 8, 9);
+/* { dg-final { scan-file va-opt-3.i "t23 6 7 7 8 6 8 6 6;" } } */
+t24 f15 (6, 7, 8);
+/* { dg-final { scan-file va-opt-3.i "t24 6 6;" } } */