[PATCH] libcpp: __VA_OPT__ p1042r1 placemarker changes [PR101488]

Jason Merrill jason@redhat.com
Mon Aug 16 22:07:57 GMT 2021


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
> 



More information about the Gcc-patches mailing list