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 <ja...@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
