Re: [PATCH] __VA_OPT__ fixes (PR preprocessor/83063, PR preprocessor/83708)

[Jason Merrill <jason at redhat dot com>]

On 01/10/2018 07:04 AM, Jakub Jelinek wrote:
> I've also cross-checked the libcpp implementation with this patch against
> trunk clang which apparently also implements __VA_OPT__ now, on the
> testcases included here the output is the same and on their
> macro_vaopt_expand.cpp testcase, if I remove all tests that test
> #__VA_OPT__ ( contents ) handling which we just reject now, there are still
> some differences:
> $ /usr/src/llvm/obj8/bin/clang++  -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/1
> $ ~/src/gcc/obj20/gcc/cc1plus -quiet -E /tmp/macro_vaopt_expand.cpp -std=c++2a > /tmp/2
> diff -up /tmp/1 /tmp/2
> -4: f(0 )
> +4: f(0)
> -6: f(0, a )
> -7: f(0, a )
> +6: f(0, a)
> +7: f(0, a)
> -9: TONG C ( ) B ( ) "A()"
> +9: HT_A() C ( ) B ( ) "A()"
> -16: S foo ;
> +16: S foo;
> -26: B1
> -26_1: B1
> +26: B 1
> +26_1: B 1
> -27: B11
> -27_1: BexpandedA0 11
> -28: B11
> +27: B 11
> +27_1: BA0 11
> +28: B 11
>
> Perhaps some of the whitespace changes aren't significant, but 9:, and
> 2[678]{,_1}: are significantly different.
> 9: is
> #define LPAREN (
> #define A() B LPAREN )
> #define B() C LPAREN )
> #define HT_B() TONG
> #define F(x, ...) HT_ ## __VA_OPT__(x x A()  #x)
> 9: F(A(),1)
>
> Thoughts on what is right and why?

clang is.  First we substitute into the body of the __VA_OPT__, so

x x A() #x
B ( ) B ( ) A() "A()"

Then we paste, so

HT_B ( ) B ( ) A() "A()"

then rescan, so

TONG C ( ) B ( ) "A()"

> Similarly for expansion on the last token from __VA_OPT__ when followed
> by ##, like:
> #define m1 (
> #define f16() f17 m1 )
> #define f17() f18 m1 )
> #define f18() m2 m1 )
> #define m3f17() g
> #define f19(x, ...) m3 ## __VA_OPT__(x x f16() #x)
> #define f20(x, ...) __VA_OPT__(x x)##m4()
> #define f21() f17
> #define f17m4() h
> t25 f19 (f16 (), 1);
> t26 f20 (f21 (), 2);
>
> E.g. 26: is:
> #define F(a,...)  __VA_OPT__(B a ## a) ## 1
> 26: F(,1)
> I really wonder why clang emits B1 in that case, there
> is no ## in between B and a, so those are 2 separate tokens
> separated by whitespace, even when a ## a is a placemarker.
> Does that mean __VA_OPT__ should throw away all the placemarkers
> and return the last non-placemarker token for the ## handling?

This is unclear to me.  The standard says that the replacement for the 
__VA_OPT__ is the result of expansion before rescanning and further 
replacement, but it's unclear to me whether the discarding of 
placemarkers should happen or not, since that is mentioned in the 
context of rescanning.  Representing a placemarker as <> below, we first 
expand the __VA_OPT__ body to

B a ## a
B <> ## <>
B <>

then do we have

B <> ## 1
or
B ## 1
?

Looking at the patch now.

Jason