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