diff mbox series

libcpp: __VA_OPT__ p1042r1 placemarker changes [PR101488]

Message ID 20210720095044.GY2380545@tucnak
State New
Headers show
Series libcpp: __VA_OPT__ p1042r1 placemarker changes [PR101488] | expand

Commit Message

Jakub Jelinek July 20, 2021, 9:50 a.m. UTC
Hi!

So, besides missing #__VA_OPT__ patch for which I've posted patch last week,
P1042R1 introduced some placemarker changes for __VA_OPT__, most notably
the addition of before "removal of placemarker tokens," rescanning ...
and the
#define H4(X, ...) __VA_OPT__(a X ## X) ## b
H4(, 1)  // replaced by a b
example mentioned there where we replace it currently with ab

The following patch are the minimum changes (except for the
__builtin_expect) that achieve the same preprocessing between current
clang++ and patched gcc on all the testcases I've tried (i.e. gcc __VA_OPT__
testsuite in c-c++-common/cpp/va-opt* including the new test and the clang
clang/test/Preprocessor/macro_va_opt* testcases).

At one point I was trying to implement the __VA_OPT__(args) case as if
for non-empty __VA_ARGS__ it expanded as if __VA_OPT__( and ) were missing,
but from the tests it seems that is not how it should work, in particular
if after (or before) we have some macro argument and it is not followed
(or preceded) by ##, then it should be macro expanded even when __VA_OPT__
is after ## or ) is followed by ##.  And it seems that not removing any
padding tokens isn't possible either, because the expansion of the arguments
typically has a padding token at the start and end and those at least
according to the testsuite need to go.  It is unclear if it would be enough
to remove just one or if all padding tokens should be removed.
Anyway, e.g. the previous removal of all padding tokens at the end of
__VA_OPT__ is undesirable, as it e.g. eats also the padding tokens needed
for the H4 example from the paper.

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

2021-07-20  Jakub Jelinek  <jakub@redhat.com>

	PR preprocessor/101488
	* macro.c (replace_args): Fix up handling of CPP_PADDING tokens at the
	start or end of __VA_OPT__ arguments when preceeded or followed by ##.

	* c-c++-common/cpp/va-opt-3.c: Adjust expected output.
	* c-c++-common/cpp/va-opt-7.c: New test.


	Jakub

Comments

Jason Merrill Aug. 16, 2021, 10:07 p.m. UTC | #1
On 7/20/21 5:50 AM, Jakub Jelinek wrote:
> Hi!
> 
> So, besides missing #__VA_OPT__ patch for which I've posted patch last week,
> P1042R1 introduced some placemarker changes for __VA_OPT__, most notably
> the addition of before "removal of placemarker tokens," rescanning ...
> and the
> #define H4(X, ...) __VA_OPT__(a X ## X) ## b
> H4(, 1)  // replaced by a b
> example mentioned there where we replace it currently with ab
> 
> The following patch are the minimum changes (except for the
> __builtin_expect) that achieve the same preprocessing between current
> clang++ and patched gcc on all the testcases I've tried (i.e. gcc __VA_OPT__
> testsuite in c-c++-common/cpp/va-opt* including the new test and the clang
> clang/test/Preprocessor/macro_va_opt* testcases).
> 
> At one point I was trying to implement the __VA_OPT__(args) case as if
> for non-empty __VA_ARGS__ it expanded as if __VA_OPT__( and ) were missing,
> but from the tests it seems that is not how it should work, in particular
> if after (or before) we have some macro argument and it is not followed
> (or preceded) by ##, then it should be macro expanded even when __VA_OPT__
> is after ## or ) is followed by ##.

> And it seems that not removing any
> padding tokens isn't possible either, because the expansion of the arguments
> typically has a padding token at the start and end and those at least
> according to the testsuite need to go.

Makes sense, just like we discard padding around macro arguments.

> It is unclear if it would be enough
> to remove just one or if all padding tokens should be removed.
> Anyway, e.g. the previous removal of all padding tokens at the end of
> __VA_OPT__ is undesirable, as it e.g. eats also the padding tokens needed
> for the H4 example from the paper.

Hmm, I don't see why.  Looking at the H4 example, it seems that the 
expansion of __VA_OPT__ should be

  a <placemarker>

so when we paste to b, b is pasted to the placemarker, leaving a as a 
separate token.

> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2021-07-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR preprocessor/101488
> 	* macro.c (replace_args): Fix up handling of CPP_PADDING tokens at the
> 	start or end of __VA_OPT__ arguments when preceeded or followed by ##.
> 
> 	* c-c++-common/cpp/va-opt-3.c: Adjust expected output.
> 	* c-c++-common/cpp/va-opt-7.c: New test.
> 
> --- libcpp/macro.c.jj	2021-07-16 11:10:08.512925510 +0200
> +++ libcpp/macro.c	2021-07-19 15:58:59.819101659 +0200
> @@ -2025,6 +2026,7 @@ replace_args (cpp_reader *pfile, cpp_has
>     i = 0;
>     vaopt_state vaopt_tracker (pfile, macro->variadic, &args[macro->paramc - 1]);
>     const cpp_token **vaopt_start = NULL;
> +  unsigned vaopt_padding_tokens = 0;
>     for (src = macro->exp.tokens; src < limit; src++)
>       {
>         unsigned int arg_tokens_count;
> @@ -2034,7 +2036,7 @@ replace_args (cpp_reader *pfile, cpp_has
>   
>         /* __VA_OPT__ handling.  */
>         vaopt_state::update_type vostate = vaopt_tracker.update (src);
> -      if (vostate != vaopt_state::INCLUDE)
> +      if (__builtin_expect (vostate != vaopt_state::INCLUDE, false))
>   	{
>   	  if (vostate == vaopt_state::BEGIN)
>   	    {
> @@ -2059,7 +2061,9 @@ replace_args (cpp_reader *pfile, cpp_has
>   
>   	      /* Remove any tail padding from inside the __VA_OPT__.  */
>   	      paste_flag = tokens_buff_last_token_ptr (buff);
> -	      while (paste_flag && paste_flag != start
> +	      while (vaopt_padding_tokens--
> +		     && paste_flag
> +		     && paste_flag != start
>   		     && (*paste_flag)->type == CPP_PADDING)
>   		{
>   		  tokens_buff_remove_last_token (buff);
> @@ -2103,6 +2107,7 @@ replace_args (cpp_reader *pfile, cpp_has
>   	  continue;
>   	}
>   
> +      vaopt_padding_tokens = 0;
>         if (src->type != CPP_MACRO_ARG)
>   	{
>   	  /* Allocate a virtual location for token SRC, and add that
> @@ -2180,11 +2185,8 @@ replace_args (cpp_reader *pfile, cpp_has
>   		  else
>   		    paste_flag = tmp_token_ptr;
>   		}
> -	      /* 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)
> +	      /* Remove the paste flag if the RHS is a placemarker.  */
> +	      else if (arg_tokens_count == 0)
>   		paste_flag = tmp_token_ptr;
>   	    }
>   	}
> @@ -2259,8 +2262,12 @@ replace_args (cpp_reader *pfile, cpp_has
>   		token_index += j;
>   
>   	      index = expanded_token_index (pfile, macro, src, token_index);
> -	      tokens_buff_add_token (buff, virt_locs,
> -				     macro_arg_token_iter_get_token (&from),
> +	      const cpp_token *tok = macro_arg_token_iter_get_token (&from);
> +	      if (tok->type == CPP_PADDING)
> +		vaopt_padding_tokens++;
> +	      else
> +		vaopt_padding_tokens = 0;
> +	      tokens_buff_add_token (buff, virt_locs, tok,
>   				     macro_arg_token_iter_get_location (&from),
>   				     src->src_loc, map, index);
>   	      macro_arg_token_iter_forward (&from);
> @@ -2300,13 +2307,13 @@ 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)
> -	  && !last_token_is (buff, vaopt_start))
> +      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT))
>   	{
>   	  const cpp_token *t = &pfile->avoid_paste;
>   	  tokens_buff_add_token (buff, virt_locs,
>   				 t, t->src_loc, t->src_loc,
>   				 NULL, 0);
> +	  vaopt_padding_tokens++;
>   	}
>   
>         /* Add a new paste flag, or remove an unwanted one.  */
> --- gcc/testsuite/c-c++-common/cpp/va-opt-3.c.jj	2020-01-12 11:54:37.003404507 +0100
> +++ gcc/testsuite/c-c++-common/cpp/va-opt-3.c	2021-07-19 15:24:52.684959978 +0200
> @@ -85,10 +85,10 @@ t25 f19 (f16 (), 1);
>   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;" } } */
> +/* { dg-final { scan-file va-opt-3.i "t27 1 23;" } } */
>   t28 f23 (, x);
> -/* { dg-final { scan-file va-opt-3.i "t28 123;" } } */
> +/* { dg-final { scan-file va-opt-3.i "t28 1 23;" } } */
>   t29 f24 (, x);
> -/* { dg-final { scan-file va-opt-3.i "t29 123;" } } */
> +/* { dg-final { scan-file va-opt-3.i "t29 12 3;" } } */
>   t30 f25 (, x);
> -/* { dg-final { scan-file va-opt-3.i "t30 123;" } } */
> +/* { dg-final { scan-file va-opt-3.i "t30 12 3;" } } */
> --- gcc/testsuite/c-c++-common/cpp/va-opt-7.c.jj	2021-07-19 15:45:21.464253740 +0200
> +++ gcc/testsuite/c-c++-common/cpp/va-opt-7.c	2021-07-19 16:52:30.567330487 +0200
> @@ -0,0 +1,101 @@
> +/* PR preprocessor/101488 */
> +/* { dg-do preprocess } */
> +/* { dg-options "-std=gnu99" { target c } } */
> +/* { dg-options "-std=c++2a" { target c++ } } */
> +
> +#define f0() n
> +#define f1(x,...) a ## __VA_OPT__ (a) ## a
> +#define f2(x,...) a ## __VA_OPT__ () ## a
> +#define f3(x,...) a ## __VA_OPT__ (x) ## a
> +#define f4(x,...) a ## __VA_OPT__ (x##x) ## a
> +#define f5(x,...) a ## __VA_OPT__ (x##x 1) ## a
> +#define f6(x,...) a ## __VA_OPT__ (1 x##x) ## a
> +#define f7(x,...) __VA_OPT__ (f0 x ## x ) ## 1
> +#define f8(x,...) __VA_OPT__ (f0 x) ## 1
> +#define f9(x,...) f0 ## __VA_OPT__ (x 1) ## 1
> +#define f10(x,...) f0 ## __VA_OPT__ (x ## x 1) ## 1
> +#define f11(x, ...) __VA_OPT__(a x ## x) ## b
> +#define f12(x, ...) a ## __VA_OPT__(x ## x b)
> +#define f13(x) x ## x b
> +#define ab def
> +#define bc ghi
> +#define abc jkl
> +#define f14(x, ...) a ## __VA_OPT__(x b x) ## c
> +t1 f1(,);
> +/* { dg-final { scan-file va-opt-7.i "t1 aa;" } } */
> +t2 f1(,1);
> +/* { dg-final { scan-file va-opt-7.i "t2 aaa;" } } */
> +t3 f1(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t3 aaa;" } } */
> +t4 f2(,);
> +/* { dg-final { scan-file va-opt-7.i "t4 aa;" } } */
> +t5 f2(,1);
> +/* { dg-final { scan-file va-opt-7.i "t5 aa;" } } */
> +t6 f2(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t6 aa;" } } */
> +t7 f3(,);
> +/* { dg-final { scan-file va-opt-7.i "t7 aa;" } } */
> +t8 f3(,1);
> +/* { dg-final { scan-file va-opt-7.i "t8 aa;" } } */
> +t9 f3(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t9 a2a;" } } */
> +t10 f4(,);
> +/* { dg-final { scan-file va-opt-7.i "t10 aa;" } } */
> +t11 f4(,1);
> +/* { dg-final { scan-file va-opt-7.i "t11 aa;" } } */
> +t12 f4(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t12 a22a;" } } */
> +t13 f5(,);
> +/* { dg-final { scan-file va-opt-7.i "t13 aa;" } } */
> +t14 f5(,1);
> +/* { dg-final { scan-file va-opt-7.i "t14 a 1a;" } } */
> +t15 f5(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t15 a22 1a;" } } */
> +t16 f6(,);
> +/* { dg-final { scan-file va-opt-7.i "t16 aa;" } } */
> +t17 f6(,1);
> +/* { dg-final { scan-file va-opt-7.i "t17 a1 a;" } } */
> +t18 f6(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t18 a1 22a;" } } */
> +t19 f7(,);
> +/* { dg-final { scan-file va-opt-7.i "t19 1;" } } */
> +t20 f7(,1);
> +/* { dg-final { scan-file va-opt-7.i "t20 f0 1;" } } */
> +t21 f7(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t21 f0 221;" } } */
> +t22 f8(,);
> +/* { dg-final { scan-file va-opt-7.i "t22 1;" } } */
> +t23 f8(,1);
> +/* { dg-final { scan-file va-opt-7.i "t23 f0 1;" } } */
> +t24 f8(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t24 f0 21;" } } */
> +t25 f9(,);
> +/* { dg-final { scan-file va-opt-7.i "t25 f01;" } } */
> +t26 f9(,1);
> +/* { dg-final { scan-file va-opt-7.i "t26 f0 11;" } } */
> +t27 f9(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t27 f02 11;" } } */
> +t28 f10(,);
> +/* { dg-final { scan-file va-opt-7.i "t28 f01;" } } */
> +t29 f10(,1);
> +/* { dg-final { scan-file va-opt-7.i "t29 f0 11;" } } */
> +t30 f10(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t30 f022 11;" } } */
> +t31 f11(,);
> +/* { dg-final { scan-file va-opt-7.i "t31 b;" } } */
> +t32 f11(,1);
> +/* { dg-final { scan-file va-opt-7.i "t32 a b;" } } */
> +t33 f11(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t33 a 22b;" } } */
> +t34 f12(,);
> +/* { dg-final { scan-file va-opt-7.i "t34 a;" } } */
> +t35 f12(,1);
> +/* { dg-final { scan-file va-opt-7.i "t35 a b;" } } */
> +t36 f12(2,1);
> +/* { dg-final { scan-file va-opt-7.i "t36 a22 b;" } } */
> +t37 f14(,);
> +/* { dg-final { scan-file va-opt-7.i "t37 ac;" } } */
> +t38 f14(,1);
> +/* { dg-final { scan-file va-opt-7.i "t38 a b c;" } } */
> +t39 f14(f13(),1);
> +/* { dg-final { scan-file va-opt-7.i "t39 def b ghi;" } } */
> 
> 	Jakub
>
Jakub Jelinek Aug. 17, 2021, 8:25 a.m. UTC | #2
On Mon, Aug 16, 2021 at 06:07:57PM -0400, Jason Merrill wrote:
> > It is unclear if it would be enough
> > to remove just one or if all padding tokens should be removed.
> > Anyway, e.g. the previous removal of all padding tokens at the end of
> > __VA_OPT__ is undesirable, as it e.g. eats also the padding tokens needed
> > for the H4 example from the paper.
> 
> Hmm, I don't see why.  Looking at the H4 example, it seems that the
> expansion of __VA_OPT__ should be
> 
>  a <placemarker>
> 
> so when we paste to b, b is pasted to the placemarker, leaving a as a
> separate token.

#define H4(X, ...) __VA_OPT__(a X ## X) ## b
H4(, 1)  // replaced by a b

We actually get with vanilla trunk
  a <placemarker> <placemarker>
where the former comes from:
2216	      /* Padding on the left of an argument (unless RHS of ##).  */
2217	      if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
2218		  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT)
2219		  && !last_token_is (buff, vaopt_start))
2220		{
2221		  const cpp_token *t = padding_token (pfile, src);
2222		  unsigned index = expanded_token_index (pfile, macro, src, i);
2223		  /* Allocate a virtual location for the padding token and
2224		     append the token and its location to BUFF and
2225		     VIRT_LOCS.   */
2226		  tokens_buff_add_token (buff, virt_locs, t,
2227					 t->src_loc, t->src_loc,
2228					 map, index);
2229		}
and the latter one is added at
2303	      /* Avoid paste on RHS (even case count == 0).  */
2304	      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT)
2305		  && !last_token_is (buff, vaopt_start))
2306		{
2307		  const cpp_token *t = &pfile->avoid_paste;
2308		  tokens_buff_add_token (buff, virt_locs,
2309					 t, t->src_loc, t->src_loc,
2310					 NULL, 0);
2311		}
and trunk eats both <placemarker>s in:
              /* 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);
                }
and thus H4(, 1) is replaced by ab instead of the right a b.

We want to remove the latter <placemarker> but not the former one, and
the patch adds the vaopt_padding_tokens counter for it to control
how many placemarkers are removed on vaopt_state::END.
As can be seen in #c1 and #c2 of the PR, I've tried various approaches,
but neither worked out for all the cases except the posted one.

	Jakub
Jason Merrill Aug. 17, 2021, 3:32 p.m. UTC | #3
On 8/17/21 4:25 AM, Jakub Jelinek wrote:
> On Mon, Aug 16, 2021 at 06:07:57PM -0400, Jason Merrill wrote:
>>> It is unclear if it would be enough
>>> to remove just one or if all padding tokens should be removed.
>>> Anyway, e.g. the previous removal of all padding tokens at the end of
>>> __VA_OPT__ is undesirable, as it e.g. eats also the padding tokens needed
>>> for the H4 example from the paper.
>>
>> Hmm, I don't see why.  Looking at the H4 example, it seems that the
>> expansion of __VA_OPT__ should be
>>
>>   a <placemarker>
>>
>> so when we paste to b, b is pasted to the placemarker, leaving a as a
>> separate token.
> 
> #define H4(X, ...) __VA_OPT__(a X ## X) ## b
> H4(, 1)  // replaced by a b
> 
> We actually get with vanilla trunk
>    a <placemarker> <placemarker>
> where the former comes from:
> 2216	      /* Padding on the left of an argument (unless RHS of ##).  */
> 2217	      if ((!pfile->state.in_directive || pfile->state.directive_wants_padding)
> 2218		  && src != macro->exp.tokens && !(src[-1].flags & PASTE_LEFT)
> 2219		  && !last_token_is (buff, vaopt_start))
> 2220		{
> 2221		  const cpp_token *t = padding_token (pfile, src);
> 2222		  unsigned index = expanded_token_index (pfile, macro, src, i);
> 2223		  /* Allocate a virtual location for the padding token and
> 2224		     append the token and its location to BUFF and
> 2225		     VIRT_LOCS.   */
> 2226		  tokens_buff_add_token (buff, virt_locs, t,
> 2227					 t->src_loc, t->src_loc,
> 2228					 map, index);
> 2229		}
> and the latter one is added at
> 2303	      /* Avoid paste on RHS (even case count == 0).  */
> 2304	      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT)
> 2305		  && !last_token_is (buff, vaopt_start))
> 2306		{
> 2307		  const cpp_token *t = &pfile->avoid_paste;
> 2308		  tokens_buff_add_token (buff, virt_locs,
> 2309					 t, t->src_loc, t->src_loc,
> 2310					 NULL, 0);
> 2311		}
> and trunk eats both <placemarker>s in:
>                /* 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);
>                  }
> and thus H4(, 1) is replaced by ab instead of the right a b.
> 
> We want to remove the latter <placemarker> but not the former one, and
> the patch adds the vaopt_padding_tokens counter for it to control
> how many placemarkers are removed on vaopt_state::END.
> As can be seen in #c1 and #c2 of the PR, I've tried various approaches,
> but neither worked out for all the cases except the posted one.

I notice that the second placemarker you mention is avoid_paste, which 
seems relevant.  This seems to also work, at least it doesn't seem to 
break any of the va_opt tests.  Thoughts?

Jason
Jakub Jelinek Aug. 17, 2021, 4:25 p.m. UTC | #4
On Tue, Aug 17, 2021 at 08:32:50AM -0700, Jason Merrill wrote:
> > We want to remove the latter <placemarker> but not the former one, and
> > the patch adds the vaopt_padding_tokens counter for it to control
> > how many placemarkers are removed on vaopt_state::END.
> > As can be seen in #c1 and #c2 of the PR, I've tried various approaches,
> > but neither worked out for all the cases except the posted one.
> 
> I notice that the second placemarker you mention is avoid_paste, which seems
> relevant.  This seems to also work, at least it doesn't seem to break any of
> the va_opt tests.  Thoughts?

I've verified my patch + your incremental patch works not just on the
va-opt* tests in gcc testsuite, but also behaves the same as without the
incremental patch on the clang testcases (I think it is all covered now in
our testsuite, checked just to make sure).

So, looks just fine to me.  I can include your patch in my bootstrap/regtest
tonight.

> >From d6cc54280e1c4dba91e883721e05ab0037f4a896 Mon Sep 17 00:00:00 2001
> From: Jason Merrill <jason@redhat.com>
> Date: Tue, 17 Aug 2021 08:12:02 -0700
> Subject: [PATCH] libcpp: __VA_OPT__ tweak
> To: gcc-patches@gcc.gnu.org
> 
> libcpp/ChangeLog:
> 
> 	* macro.c (replace_args): When __VA_OPT__ is on the LHS of ##,
> 	remove trailing avoid_paste tokens.
> ---
>  libcpp/macro.c | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/libcpp/macro.c b/libcpp/macro.c
> index 35eaae383a7..acdbe6ab14f 100644
> --- a/libcpp/macro.c
> +++ b/libcpp/macro.c
> @@ -2025,7 +2025,6 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
>    i = 0;
>    vaopt_state vaopt_tracker (pfile, macro->variadic, &args[macro->paramc - 1]);
>    const cpp_token **vaopt_start = NULL;
> -  unsigned vaopt_padding_tokens = 0;
>    for (src = macro->exp.tokens; src < limit; src++)
>      {
>        unsigned int arg_tokens_count;
> @@ -2058,16 +2057,7 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
>  	      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 (vaopt_padding_tokens--
> -		     && 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 (vaopt_tracker.stringify ())
>  		{
> @@ -2088,6 +2078,14 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
>  		}
>  	      else if (src->flags & PASTE_LEFT)
>  		{
> +		  /* Don't avoid paste after all.  */
> +		  while (paste_flag && paste_flag != start
> +			 && *paste_flag == &pfile->avoid_paste)
> +		    {
> +		      tokens_buff_remove_last_token (buff);
> +		      paste_flag = tokens_buff_last_token_ptr (buff);
> +		    }
> +
>  		  /* 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)
> @@ -2106,7 +2104,6 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
>  	  continue;
>  	}
>  
> -      vaopt_padding_tokens = 0;
>        if (src->type != CPP_MACRO_ARG)
>  	{
>  	  /* Allocate a virtual location for token SRC, and add that
> @@ -2261,10 +2258,6 @@ replace_args (cpp_reader *pfile, cpp_hashnode *node, cpp_macro *macro,
>  
>  	      index = expanded_token_index (pfile, macro, src, token_index);
>  	      const cpp_token *tok = macro_arg_token_iter_get_token (&from);
> -	      if (tok->type == CPP_PADDING)
> -		vaopt_padding_tokens++;
> -	      else
> -		vaopt_padding_tokens = 0;
>  	      tokens_buff_add_token (buff, virt_locs, tok,
>  				     macro_arg_token_iter_get_location (&from),
>  				     src->src_loc, map, index);
> @@ -2311,7 +2304,6 @@ 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,
>  				 NULL, 0);
> -	  vaopt_padding_tokens++;
>  	}
>  
>        /* Add a new paste flag, or remove an unwanted one.  */
> -- 
> 2.27.0
> 


	Jakub
diff mbox series

Patch

--- libcpp/macro.c.jj	2021-07-16 11:10:08.512925510 +0200
+++ libcpp/macro.c	2021-07-19 15:58:59.819101659 +0200
@@ -2025,6 +2026,7 @@  replace_args (cpp_reader *pfile, cpp_has
   i = 0;
   vaopt_state vaopt_tracker (pfile, macro->variadic, &args[macro->paramc - 1]);
   const cpp_token **vaopt_start = NULL;
+  unsigned vaopt_padding_tokens = 0;
   for (src = macro->exp.tokens; src < limit; src++)
     {
       unsigned int arg_tokens_count;
@@ -2034,7 +2036,7 @@  replace_args (cpp_reader *pfile, cpp_has
 
       /* __VA_OPT__ handling.  */
       vaopt_state::update_type vostate = vaopt_tracker.update (src);
-      if (vostate != vaopt_state::INCLUDE)
+      if (__builtin_expect (vostate != vaopt_state::INCLUDE, false))
 	{
 	  if (vostate == vaopt_state::BEGIN)
 	    {
@@ -2059,7 +2061,9 @@  replace_args (cpp_reader *pfile, cpp_has
 
 	      /* Remove any tail padding from inside the __VA_OPT__.  */
 	      paste_flag = tokens_buff_last_token_ptr (buff);
-	      while (paste_flag && paste_flag != start
+	      while (vaopt_padding_tokens--
+		     && paste_flag
+		     && paste_flag != start
 		     && (*paste_flag)->type == CPP_PADDING)
 		{
 		  tokens_buff_remove_last_token (buff);
@@ -2103,6 +2107,7 @@  replace_args (cpp_reader *pfile, cpp_has
 	  continue;
 	}
 
+      vaopt_padding_tokens = 0;
       if (src->type != CPP_MACRO_ARG)
 	{
 	  /* Allocate a virtual location for token SRC, and add that
@@ -2180,11 +2185,8 @@  replace_args (cpp_reader *pfile, cpp_has
 		  else
 		    paste_flag = tmp_token_ptr;
 		}
-	      /* 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)
+	      /* Remove the paste flag if the RHS is a placemarker.  */
+	      else if (arg_tokens_count == 0)
 		paste_flag = tmp_token_ptr;
 	    }
 	}
@@ -2259,8 +2262,12 @@  replace_args (cpp_reader *pfile, cpp_has
 		token_index += j;
 
 	      index = expanded_token_index (pfile, macro, src, token_index);
-	      tokens_buff_add_token (buff, virt_locs,
-				     macro_arg_token_iter_get_token (&from),
+	      const cpp_token *tok = macro_arg_token_iter_get_token (&from);
+	      if (tok->type == CPP_PADDING)
+		vaopt_padding_tokens++;
+	      else
+		vaopt_padding_tokens = 0;
+	      tokens_buff_add_token (buff, virt_locs, tok,
 				     macro_arg_token_iter_get_location (&from),
 				     src->src_loc, map, index);
 	      macro_arg_token_iter_forward (&from);
@@ -2300,13 +2307,13 @@  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)
-	  && !last_token_is (buff, vaopt_start))
+      if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT))
 	{
 	  const cpp_token *t = &pfile->avoid_paste;
 	  tokens_buff_add_token (buff, virt_locs,
 				 t, t->src_loc, t->src_loc,
 				 NULL, 0);
+	  vaopt_padding_tokens++;
 	}
 
       /* Add a new paste flag, or remove an unwanted one.  */
--- gcc/testsuite/c-c++-common/cpp/va-opt-3.c.jj	2020-01-12 11:54:37.003404507 +0100
+++ gcc/testsuite/c-c++-common/cpp/va-opt-3.c	2021-07-19 15:24:52.684959978 +0200
@@ -85,10 +85,10 @@  t25 f19 (f16 (), 1);
 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;" } } */
+/* { dg-final { scan-file va-opt-3.i "t27 1 23;" } } */
 t28 f23 (, x);
-/* { dg-final { scan-file va-opt-3.i "t28 123;" } } */
+/* { dg-final { scan-file va-opt-3.i "t28 1 23;" } } */
 t29 f24 (, x);
-/* { dg-final { scan-file va-opt-3.i "t29 123;" } } */
+/* { dg-final { scan-file va-opt-3.i "t29 12 3;" } } */
 t30 f25 (, x);
-/* { dg-final { scan-file va-opt-3.i "t30 123;" } } */
+/* { dg-final { scan-file va-opt-3.i "t30 12 3;" } } */
--- gcc/testsuite/c-c++-common/cpp/va-opt-7.c.jj	2021-07-19 15:45:21.464253740 +0200
+++ gcc/testsuite/c-c++-common/cpp/va-opt-7.c	2021-07-19 16:52:30.567330487 +0200
@@ -0,0 +1,101 @@ 
+/* PR preprocessor/101488 */
+/* { dg-do preprocess } */
+/* { dg-options "-std=gnu99" { target c } } */
+/* { dg-options "-std=c++2a" { target c++ } } */
+
+#define f0() n
+#define f1(x,...) a ## __VA_OPT__ (a) ## a
+#define f2(x,...) a ## __VA_OPT__ () ## a
+#define f3(x,...) a ## __VA_OPT__ (x) ## a
+#define f4(x,...) a ## __VA_OPT__ (x##x) ## a
+#define f5(x,...) a ## __VA_OPT__ (x##x 1) ## a
+#define f6(x,...) a ## __VA_OPT__ (1 x##x) ## a
+#define f7(x,...) __VA_OPT__ (f0 x ## x ) ## 1
+#define f8(x,...) __VA_OPT__ (f0 x) ## 1
+#define f9(x,...) f0 ## __VA_OPT__ (x 1) ## 1
+#define f10(x,...) f0 ## __VA_OPT__ (x ## x 1) ## 1
+#define f11(x, ...) __VA_OPT__(a x ## x) ## b
+#define f12(x, ...) a ## __VA_OPT__(x ## x b)
+#define f13(x) x ## x b
+#define ab def
+#define bc ghi
+#define abc jkl
+#define f14(x, ...) a ## __VA_OPT__(x b x) ## c
+t1 f1(,);
+/* { dg-final { scan-file va-opt-7.i "t1 aa;" } } */
+t2 f1(,1);
+/* { dg-final { scan-file va-opt-7.i "t2 aaa;" } } */
+t3 f1(2,1);
+/* { dg-final { scan-file va-opt-7.i "t3 aaa;" } } */
+t4 f2(,);
+/* { dg-final { scan-file va-opt-7.i "t4 aa;" } } */
+t5 f2(,1);
+/* { dg-final { scan-file va-opt-7.i "t5 aa;" } } */
+t6 f2(2,1);
+/* { dg-final { scan-file va-opt-7.i "t6 aa;" } } */
+t7 f3(,);
+/* { dg-final { scan-file va-opt-7.i "t7 aa;" } } */
+t8 f3(,1);
+/* { dg-final { scan-file va-opt-7.i "t8 aa;" } } */
+t9 f3(2,1);
+/* { dg-final { scan-file va-opt-7.i "t9 a2a;" } } */
+t10 f4(,);
+/* { dg-final { scan-file va-opt-7.i "t10 aa;" } } */
+t11 f4(,1);
+/* { dg-final { scan-file va-opt-7.i "t11 aa;" } } */
+t12 f4(2,1);
+/* { dg-final { scan-file va-opt-7.i "t12 a22a;" } } */
+t13 f5(,);
+/* { dg-final { scan-file va-opt-7.i "t13 aa;" } } */
+t14 f5(,1);
+/* { dg-final { scan-file va-opt-7.i "t14 a 1a;" } } */
+t15 f5(2,1);
+/* { dg-final { scan-file va-opt-7.i "t15 a22 1a;" } } */
+t16 f6(,);
+/* { dg-final { scan-file va-opt-7.i "t16 aa;" } } */
+t17 f6(,1);
+/* { dg-final { scan-file va-opt-7.i "t17 a1 a;" } } */
+t18 f6(2,1);
+/* { dg-final { scan-file va-opt-7.i "t18 a1 22a;" } } */
+t19 f7(,);
+/* { dg-final { scan-file va-opt-7.i "t19 1;" } } */
+t20 f7(,1);
+/* { dg-final { scan-file va-opt-7.i "t20 f0 1;" } } */
+t21 f7(2,1);
+/* { dg-final { scan-file va-opt-7.i "t21 f0 221;" } } */
+t22 f8(,);
+/* { dg-final { scan-file va-opt-7.i "t22 1;" } } */
+t23 f8(,1);
+/* { dg-final { scan-file va-opt-7.i "t23 f0 1;" } } */
+t24 f8(2,1);
+/* { dg-final { scan-file va-opt-7.i "t24 f0 21;" } } */
+t25 f9(,);
+/* { dg-final { scan-file va-opt-7.i "t25 f01;" } } */
+t26 f9(,1);
+/* { dg-final { scan-file va-opt-7.i "t26 f0 11;" } } */
+t27 f9(2,1);
+/* { dg-final { scan-file va-opt-7.i "t27 f02 11;" } } */
+t28 f10(,);
+/* { dg-final { scan-file va-opt-7.i "t28 f01;" } } */
+t29 f10(,1);
+/* { dg-final { scan-file va-opt-7.i "t29 f0 11;" } } */
+t30 f10(2,1);
+/* { dg-final { scan-file va-opt-7.i "t30 f022 11;" } } */
+t31 f11(,);
+/* { dg-final { scan-file va-opt-7.i "t31 b;" } } */
+t32 f11(,1);
+/* { dg-final { scan-file va-opt-7.i "t32 a b;" } } */
+t33 f11(2,1);
+/* { dg-final { scan-file va-opt-7.i "t33 a 22b;" } } */
+t34 f12(,);
+/* { dg-final { scan-file va-opt-7.i "t34 a;" } } */
+t35 f12(,1);
+/* { dg-final { scan-file va-opt-7.i "t35 a b;" } } */
+t36 f12(2,1);
+/* { dg-final { scan-file va-opt-7.i "t36 a22 b;" } } */
+t37 f14(,);
+/* { dg-final { scan-file va-opt-7.i "t37 ac;" } } */
+t38 f14(,1);
+/* { dg-final { scan-file va-opt-7.i "t38 a b c;" } } */
+t39 f14(f13(),1);
+/* { dg-final { scan-file va-opt-7.i "t39 def b ghi;" } } */