Incorrect macro expansion in clang on Windows with VC++ target

[Quuxplusone]

Bugzilla Link	PR20277
Status	NEW
Importance	P normal
Reported by	Edward Diener (eldlistmailingz@tropicsoft.com)
Reported on	2014-07-09 19:06:59 -0700
Last modified on	2014-07-10 17:31:27 -0700
Version	trunk
Hardware	PC Windows XP
CC	jonathan.sauer@gmx.de, llvm-bugs@lists.llvm.org, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Clang is not doing macro expansion correctly in an instance which I will show.
The build is clang on Windows targeting VC++ built from the latest trunk. The
source is:

# define REM(...) __VA_ARGS__
# define EAT(...)
# define CAT(a, b) CAT_I(a, b)
# define CAT_I(a, b) a ## b
# define LPAREN() (
# define RPAREN() )
# define COMMA() ,
# define STL(seq) STL_I(SBT(seq))
# define STL_I(bseq) STL_A bseq NIL STL_B bseq
# define STL_A(m, e) m(LPAREN() e COMMA() STL_A_ID)
# define STL_A_ID() STL_A
# define STL_B(m, e) m(RPAREN() STL_B_ID)
# define STL_B_ID() STL_B
# define SBT(seq) CAT(SBT_A seq, 0)
# define SBT_A(...) (SBT_GET_REM(__VA_ARGS__), __VA_ARGS__)() SBT_B
# define SBT_B(...) (SBT_GET_REM(__VA_ARGS__), __VA_ARGS__)() SBT_A
# define SBT_GET_REM(...) REM
# define SBT_A0 (EAT, ?)
# define SBT_B0 (EAT, ?)

int main()
{
STL(())
return 0;
}

The expansion of STL should be:

(,BOOST_PP_NIL)

Admittedly placed in a CPP main function this should lead to a compiler error
etc. since '(, BOOST_PP_NIL )' by itself as C++ is invalid, but I have included
it in this way to keep things simple and to show that the macro expansion
itself is valid. The point is that clang.exe gives these errors trying to
expand the macro:

-------------------------------------------------------------------------------

seq_clang_failure.cpp(23,1) :  error: too few arguments provided to function-
like macro invocation
STL(())
^
seq_clang_failure.cpp(8,25) :  note: expanded from macro 'STL'
# define STL(seq) STL_I(SBT(seq))
                        ^
seq_clang_failure.cpp(14,23) :  note: expanded from macro 'SBT'
# define SBT(seq) CAT(SBT_A seq, 0)
                      ^
seq_clang_failure.cpp(15,59) :  note: expanded from macro 'SBT_A'
# define SBT_A(...) (SBT_GET_REM(__VA_ARGS__), __VA_ARGS__)() SBT_B
                                                          ^
seq_clang_failure.cpp(3,26) :  note: expanded from macro 'CAT'
# define CAT(a, b) CAT_I(a, b)
                         ^
seq_clang_failure.cpp(4,22) :  note: expanded from macro 'CAT_I'
# define CAT_I(a, b) a ## b
                     ^
seq_clang_failure.cpp(9,28) :  note: expanded from macro 'STL_I'
# define STL_I(bseq) STL_A bseq NIL STL_B bseq
                           ^
seq_clang_failure.cpp(10,10) :  note: macro 'STL_A' defined here
# define STL_A(m, e) m(LPAREN() e COMMA() STL_A_ID)
         ^
seq_clang_failure.cpp(23,1) :  error: use of undeclared identifier 'STL_A'
STL(())
^
seq_clang_failure.cpp(8,19) :  note: expanded from macro 'STL'
# define STL(seq) STL_I(SBT(seq))
                  ^
seq_clang_failure.cpp(9,22) :  note: expanded from macro 'STL_I'
# define STL_I(bseq) STL_A bseq NIL STL_B bseq
                     ^
seq_clang_failure.cpp(23,1) :  error: use of undeclared identifier 'EAT'
seq_clang_failure.cpp(8,25) :  note: expanded from macro 'STL'
# define STL(seq) STL_I(SBT(seq))
                        ^
seq_clang_failure.cpp(14,19) :  note: expanded from macro 'SBT'
# define SBT(seq) CAT(SBT_A seq, 0)
                  ^
seq_clang_failure.cpp(3,20) :  note: expanded from macro 'CAT'
# define CAT(a, b) CAT_I(a, b)
                   ^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see
all)
<scratch space>(2,1) :  note: expanded from here
SBT_B0
^
seq_clang_failure.cpp(19,18) :  note: expanded from macro 'SBT_B0'
# define SBT_B0 (EAT, ?)
                 ^
seq_clang_failure.cpp(9,28) :  note: expanded from macro 'STL_I'
# define STL_I(bseq) STL_A bseq NIL STL_B bseq
                           ^
seq_clang_failure.cpp(23,1) :  error: too few arguments provided to function-
like macro invocation
seq_clang_failure.cpp(8,25) :  note: expanded from macro 'STL'
# define STL(seq) STL_I(SBT(seq))
                        ^
seq_clang_failure.cpp(14,23) :  note: expanded from macro 'SBT'
# define SBT(seq) CAT(SBT_A seq, 0)
                      ^
seq_clang_failure.cpp(15,59) :  note: expanded from macro 'SBT_A'
# define SBT_A(...) (SBT_GET_REM(__VA_ARGS__), __VA_ARGS__)() SBT_B
                                                          ^
seq_clang_failure.cpp(3,26) :  note: expanded from macro 'CAT'
# define CAT(a, b) CAT_I(a, b)
                         ^
seq_clang_failure.cpp(4,22) :  note: expanded from macro 'CAT_I'
# define CAT_I(a, b) a ## b
                     ^
seq_clang_failure.cpp(9,43) :  note: expanded from macro 'STL_I'
# define STL_I(bseq) STL_A bseq NIL STL_B bseq
                                          ^
seq_clang_failure.cpp(12,10) :  note: macro 'STL_B' defined here
# define STL_B(m, e) m(RPAREN() STL_B_ID)
         ^

------------------------------------------------------------------------------

The code is actually taken from Boost PP from the BOOST_PP_SEQ_TO_LIST
functionality when using clang. I have simplified its presentation as a bug
for convenience. A '()' is a valid Boost PP seq and '(,BOOST_PP_NIL)' is a
valid Boost PP list. The STL macro is just my shorthand for the actual
BOOST_PP_SEQ_TO_LIST.

This is a serious bug. Doing straightforward macro expansion should not fail in
clang. The macro expansion exceeds in all versions of gcc, as well as in the
Boost Wave driver. So unless clang know something that I do not know about
macro expansion in the latest C++, this needs to be fixed.

The command line parameters are:

-TP /Od /Ob0 /W3 /GR /MDd  /Zc:forScope /Zc:wchar_t -fmsc-version=1800 /wd4675
/EHs -c

This is being compiled by the Boost Build system on Windows 7 Ultimate SP1,
with the latest clang.exe built successfully in Visual Studio 2012 using the
release build.

[Quuxplusone]

Preprocessing this with clang r212447 (trunk from Monday) on Mac OS X results
in:

% ~/LLVM/build/Release+Asserts/bin/clang -v -E clang.cpp
clang version 3.5.0 (trunk 212447)
[...]
int main()
{
( , NIL )
return 0;
}

So this seems to be Windows-specific.

[Quuxplusone]

It may be specific to the Windows build of clang using Visual Studio. It cannot
have anything to do with running under Windows per se AFAICS.

It would be nice if someone on Windows would:

1) Use CMake to generate the "Visual Studio 12 2013" solution
2) Build the release version of clang using the solution file generated by
CMake.
3) Test the release version of cmake passing the parameters as specified in my
original report.

This would offer verification that ny testing is correct and this is a bug
somewhere in the generation of clang-cl.exe. Of course finding and fixing the
bug is the most important thing.

[Quuxplusone]

(In reply to comment #2)
> It may be specific to the Windows build of clang using Visual Studio. It
> cannot have anything to do with running under Windows per se AFAICS.

I'm not so sure. Visual C++ has a non-standard preprocessor, which clang
emulates in Microsoft-compatibility mode. Looking into clang's source I found
the following:

lib/Lex/TokenLexer.cpp#131

  // In Microsoft-compatibility mode, a comma is removed in the expansion
  // of " ... , __VA_ARGS__ " if __VA_ARGS__ is empty.  This extension is
  // not supported by gcc.
  if (!HasPasteOperator && !PP.getLangOpts().MSVCCompat)
    return false;

If I'm not mistaken this means that this macro:

  # define SBT_A(...) (SBT_GET_REM(__VA_ARGS__), __VA_ARGS__)() SBT_B

when called with no parameters, will expand to this:

  # define SBT_A(...) (SBT_GET_REM())() SBT_B

instead of this (note the comma):

  # define SBT_A(...) (SBT_GET_REM(), )() SBT_B

[Quuxplusone]

(In reply to comment #3)
GCC does something similar when using ##__VA_ARGS__ (clang supports this as an
extension). Modifying your test case like this:

  # define SBT_A(...) (SBT_GET_REM(__VA_ARGS__), ##__VA_ARGS__)() SBT_B
  # define SBT_B(...) (SBT_GET_REM(__VA_ARGS__), ##__VA_ARGS__)() SBT_A

results in almost the same error messages.

[Quuxplusone]

(In reply to comment #3)
> (In reply to comment #2)
> > It may be specific to the Windows build of clang using Visual Studio. It
> > cannot have anything to do with running under Windows per se AFAICS.
>
> I'm not so sure. Visual C++ has a non-standard preprocessor, which clang
> emulates in Microsoft-compatibility mode. Looking into clang's source I
> found the following:
>
> lib/Lex/TokenLexer.cpp#131
>
>   // In Microsoft-compatibility mode, a comma is removed in the expansion
>   // of " ... , __VA_ARGS__ " if __VA_ARGS__ is empty.  This extension is
>   // not supported by gcc.
>   if (!HasPasteOperator && !PP.getLangOpts().MSVCCompat)
>     return false;
>
>
> If I'm not mistaken this means that this macro:
>
>   # define SBT_A(...) (SBT_GET_REM(__VA_ARGS__), __VA_ARGS__)() SBT_B
>
> when called with no parameters, will expand to this:
>
>   # define SBT_A(...) (SBT_GET_REM())() SBT_B
>
> instead of this (note the comma):
>
>   # define SBT_A(...) (SBT_GET_REM(), )() SBT_B

If clang-cl.exe is actually going to be emulating Microsoft's broken
preprocessor, then it is a useless product and I will never personally use it.
This is completely the wrong thing to do. I will voice this on the clang
developer mailing list in as strong a way that I can, but I cannot believe that
this decision was made by any persons or people among the clang developers who
know how broken the VC++ preprocessor is and why trying to emulate that
brokeness is a foolish thing to do.

[Quuxplusone]

(In reply to comment #5)
> If clang-cl.exe is actually going to be emulating Microsoft's broken
> preprocessor, then it is a useless product and I will never personally use
> it. This is completely the wrong thing to do. I will voice this on the clang
> developer mailing list in as strong a way that I can, but I cannot believe
> that this decision was made by any persons or people among the clang
> developers who know how broken the VC++ preprocessor is and why trying to
> emulate that brokeness is a foolish thing to do.

There is a certain amount of preprocessor emulation that we cannot avoid if we
wish to use the Microsoft STL headers.  Consider r184968 from June 2013, which
may be relevant to Boost PP.  That commit was necessary to parse VS2012
<type_traits>, which I am certain Boost is using elsewhere.  In general, we try
as hard as possible to avoid compatibility hacks like this, but sometimes there
are system headers that force our hand.

See also the commits that Alp made in May to support things that Boost itself
does like "#ifdef and".  Boost is actually sufficiently popular that just
fixing the latest Boost isn't actually enough, we end up having to implement
hacks in the compiler just to deal with the large body of existing code.