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
>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
