* [Bug c++/101488] Implement p1042r1 __VA_OPT__ placemarker changes
2021-07-17 14:21 [Bug c++/101488] New: Implement p1042r1 __VA_OPT__ placemarker changes jakub at gcc dot gnu.org
@ 2021-07-17 14:29 ` jakub at gcc dot gnu.org
2021-07-19 12:51 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-07-17 14:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101488
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I've tried a WIP (with just 0 && or commenting out parts of current code
instead of removing/cleaning up) that attempts to treat __VA_OPT__ ( ... ) with
## before __VA_OPT__ and/or after ) more similarly to the case where the
__VA_OPT__, ( and ) tokens are missing:
--- libcpp/macro.c.jj 2021-07-16 11:10:08.512925510 +0200
+++ libcpp/macro.c 2021-07-17 15:55:03.178568895 +0200
@@ -2059,7 +2059,7 @@ 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 (0 && paste_flag && paste_flag != start
&& (*paste_flag)->type == CPP_PADDING)
{
tokens_buff_remove_last_token (buff);
@@ -2184,7 +2184,7 @@ replace_args (cpp_reader *pfile, cpp_has
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)
+/* && tmp_token_ptr != vaopt_start */)
paste_flag = tmp_token_ptr;
}
}
@@ -2197,7 +2197,7 @@ replace_args (cpp_reader *pfile, cpp_has
MACRO_ARG_TOKEN_EXPANDED,
arg, arg->expanded);
- if (last_token_is (buff, vaopt_start))
+ if (0 && last_token_is (buff, vaopt_start))
{
/* We're expanding an arg at the beginning of __VA_OPT__.
Skip padding. */
@@ -2215,7 +2215,7 @@ replace_args (cpp_reader *pfile, cpp_has
/* 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)
- && !last_token_is (buff, vaopt_start))
+ /* && !last_token_is (buff, vaopt_start) */)
{
const cpp_token *t = padding_token (pfile, src);
unsigned index = expanded_token_index (pfile, macro, src, i);
@@ -2301,7 +2301,7 @@ replace_args (cpp_reader *pfile, cpp_has
/* Avoid paste on RHS (even case count == 0). */
if (!pfile->state.in_directive && !(src->flags & PASTE_LEFT)
- && !last_token_is (buff, vaopt_start))
+ /*&& !last_token_is (buff, vaopt_start)*/)
{
const cpp_token *t = &pfile->avoid_paste;
tokens_buff_add_token (buff, virt_locs,
@@ -3544,6 +3544,8 @@ create_iso_definition (cpp_reader *pfile
bool varadic = false;
bool ok = false;
cpp_macro *macro = NULL;
+ bool vaopt_after_paste = false;
+ unsigned int vaopt_end_idx = 0;
/* Look at the first token, to see if this is a function-like
macro. */
@@ -3700,14 +3702,33 @@ create_iso_definition (cpp_reader *pfile
token[-1].flags |= SP_DIGRAPH;
if (token->flags & PREV_WHITE)
token[-1].flags |= SP_PREV_WHITE;
+ if (macro->count == vaopt_end_idx)
+ token[-2].flags |= PASTE_LEFT;
}
following_paste_op = true;
}
else
following_paste_op = false;
- if (vaopt_tracker.update (token) == vaopt_state::ERROR)
+ vaopt_state::update_type vostate = vaopt_tracker.update (token);
+ if (vostate == vaopt_state::INCLUDE)
+ continue;
+ if (vostate == vaopt_state::ERROR)
goto out;
+ if (vostate == vaopt_state::BEGIN
+ && macro->count > 1
+ && (token[-1].flags & PASTE_LEFT))
+ vaopt_after_paste = true;
+ if (vostate == vaopt_state::DROP && vaopt_after_paste)
+ {
+ vaopt_after_paste = false;
+ /* Duplicate the PASTE_LEFT flag on ( after __VA_OPT__
+ if ## is before __VA_OPT__. */
+ if (!vaopt_tracker.stringify ())
+ token->flags |= PASTE_LEFT;
+ }
+ if (vostate == vaopt_state::END)
+ vaopt_end_idx = macro->count;
}
/* We're committed to winning now. */
and that results into identical output on the #c0 testcase above as clang++,
and ditto macro_vaopt_p1042r1.cpp testcase, but macro_vaopt_expand.cpp differs
slightly:
-9: HT_A() C ( ) B ( ) "A()"
+9: TONG C ( ) B ( ) "A()"
-27_1: BA0 11
+27_1: BexpandedA0 11
(- is gcc with #__VA_OPT__ and the above patch, + is clang++). That actually
matches how the F0 case in
#define LPAREN (
#define A0 expandedA0
#define A1 expandedA1 A0
#define A2 expandedA2 A1
#define A3 expandedA3 A2
#define A() B LPAREN )
#define B() C LPAREN )
#define C() D LPAREN )
#define HT_B() TONG
#define F0(x, ...) HT_ ## x x A() #x
#define F(x, ...) HT_ ## __VA_OPT__(x x A() #x)
8: F0(A(),1)
9: F(A(),1)
is handled, so I'm getting lost on what is supposed to be special about
__VA_OPT__ here. In any case, the unpatched libcpp handled that F0 and F cases
the same as clang++. And we probably need more extensive testsuite coverage...
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/101488] Implement p1042r1 __VA_OPT__ placemarker changes
2021-07-17 14:21 [Bug c++/101488] New: Implement p1042r1 __VA_OPT__ placemarker changes jakub at gcc dot gnu.org
2021-07-17 14:29 ` [Bug c++/101488] " jakub at gcc dot gnu.org
@ 2021-07-19 12:51 ` jakub at gcc dot gnu.org
2021-07-19 14:38 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-07-19 12:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101488
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Different patch:
--- libcpp/macro.c.jj 2021-07-16 11:10:08.512925510 +0200
+++ libcpp/macro.c 2021-07-19 14:19:48.217122675 +0200
@@ -236,10 +236,20 @@ class vaopt_state {
return m_state == 0;
}
+ /* Return true for # __VA_OPT__. */
bool stringify () const
{
return m_stringify;
- }
+ }
+
+ /* Return true if inside of __VA_OPT__ args but not inside of extra ()s
+ nested in it, i.e. when if next token is ), it will be END.
+ Next token is guaranteed to appear, as otherwise it would be ERROR
+ earlier. */
+ bool maybe_before_end () const
+ {
+ return m_state == 3;
+ }
private:
@@ -2025,6 +2035,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;
+ bool vaopt_paste_left = false;
for (src = macro->exp.tokens; src < limit; src++)
{
unsigned int arg_tokens_count;
@@ -2050,6 +2061,8 @@ replace_args (cpp_reader *pfile, cpp_has
t->src_loc, t->src_loc,
map, index);
}
+ vaopt_paste_left = (src != macro->exp.tokens
+ && (src[-1].flags & PASTE_LEFT));
vaopt_start = tokens_buff_last_token_ptr (buff);
}
else if (vostate == vaopt_state::END)
@@ -2059,7 +2072,7 @@ 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 (0 && paste_flag && paste_flag != start
&& (*paste_flag)->type == CPP_PADDING)
{
tokens_buff_remove_last_token (buff);
@@ -2184,7 +2197,7 @@ replace_args (cpp_reader *pfile, cpp_has
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)
+/* && tmp_token_ptr != vaopt_start */)
paste_flag = tmp_token_ptr;
}
}
@@ -2214,8 +2227,9 @@ replace_args (cpp_reader *pfile, cpp_has
/* 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)
- && !last_token_is (buff, vaopt_start))
+ && src != macro->exp.tokens
+ && !(src[-1].flags & PASTE_LEFT)
+ && (!last_token_is (buff, vaopt_start) || !vaopt_paste_left))
{
const cpp_token *t = padding_token (pfile, src);
unsigned index = expanded_token_index (pfile, macro, src, i);
@@ -2300,8 +2314,12 @@ 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)
+ && (!vaopt_tracker.maybe_before_end ()
+ || src[1].type != CPP_CLOSE_PAREN
+ || (src[1].flags & PASTE_LEFT) == 0)
+ /*&& !last_token_is (buff, vaopt_start)*/)
{
const cpp_token *t = &pfile->avoid_paste;
tokens_buff_add_token (buff, virt_locs,
With this one, clang's macro_vaopt_expand.cpp and macro_vaopt_p1042r1.cpp and
the #c0 testcase preprocess the same between g++ and clang++, but
c-c++-common/cpp/va-opt-3.c has one difference:
t26 f20 (f21 (), 2);
/* { dg-final { scan-file va-opt-3.i "t26 f17 h;" } } */
which yields with the patched cc1plus into
t26 f17 f17 m4();
I guess I understand why for the cases of x ## __VA_OPT__ (y z) and __VA_OPT__
(a b) ## c we want
for y and b macros use MACRO_ARG_TOKEN_EXPANDED instead of
MACRO_ARG_TOKEN_NORMAL when it would be
x ## y z or a b ## c, but already in the above patch I had to uncomment again
the
if (last_token_is (buff, vaopt_start))
{
/* We're expanding an arg at the beginning of __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 token removal because otherwise e.g. the #c1 testcase (and
macro_vaopt_expand.cpp) misbehaves - MACRO_ARG_TOKEN_EXPANDED
tokens for the x macro start with CPP_PADDING and at least if clang++ is
correct we don't want it there.
The question is if we should optimize away a single CPP_PADDING tokens or
unlimited number of them, whether it is possible
that somehow the macro expanded definition would start with multiple
CPP_PADDING tokens and whether some of them shouldn't be kept.
For the t26 case, we need to remove some trailing CPP_PADDING tokens too, but
as various testcases show, not all of them.
So, perhaps remember in the loop in some integer variable how many CPP_PADDING
tokens should be removed if next token would be vaopt_state::END
with PASTE_LEFT on it?
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/101488] Implement p1042r1 __VA_OPT__ placemarker changes
2021-07-17 14:21 [Bug c++/101488] New: Implement p1042r1 __VA_OPT__ placemarker changes jakub at gcc dot gnu.org
2021-07-17 14:29 ` [Bug c++/101488] " jakub at gcc dot gnu.org
2021-07-19 12:51 ` jakub at gcc dot gnu.org
@ 2021-07-19 14:38 ` jakub at gcc dot gnu.org
2021-09-01 19:34 ` cvs-commit at gcc dot gnu.org
2021-09-01 19:35 ` jakub at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-07-19 14:38 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101488
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 51172
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51172&action=edit
gcc12-pr101488.patch
Another version of the patch, this one passes the clang macro_vaopt_* tests and
preprocesses the #c0 testcase (now extended into va-opt-7.c) and other tests in
gcc testsuite the same.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Bug c++/101488] Implement p1042r1 __VA_OPT__ placemarker changes
2021-07-17 14:21 [Bug c++/101488] New: Implement p1042r1 __VA_OPT__ placemarker changes jakub at gcc dot gnu.org
` (2 preceding siblings ...)
2021-07-19 14:38 ` jakub at gcc dot gnu.org
@ 2021-09-01 19:34 ` cvs-commit at gcc dot gnu.org
2021-09-01 19:35 ` jakub at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-09-01 19:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101488
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:e928cf47f350e46eacb48ed954112e603ef3800a
commit r12-3298-ge928cf47f350e46eacb48ed954112e603ef3800a
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Sep 1 21:31:25 2021 +0200
libcpp: __VA_OPT__ p1042r1 placemarker changes [PR101488]
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.
2021-09-01 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* [Bug c++/101488] Implement p1042r1 __VA_OPT__ placemarker changes
2021-07-17 14:21 [Bug c++/101488] New: Implement p1042r1 __VA_OPT__ placemarker changes jakub at gcc dot gnu.org
` (3 preceding siblings ...)
2021-09-01 19:34 ` cvs-commit at gcc dot gnu.org
@ 2021-09-01 19:35 ` jakub at gcc dot gnu.org
4 siblings, 0 replies; 6+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-09-01 19:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101488
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |RESOLVED
Resolution|--- |FIXED
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 6+ messages in thread