LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 43282 - Preprocessor bug/incompatibility regarding __VA_ARGS__ and comma when compiling with -fms-compatibility on Windows
Summary: Preprocessor bug/incompatibility regarding __VA_ARGS__ and comma when compili...
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC All
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
: 43584 45005 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-09-11 10:23 PDT by Tom Hender
Modified: 2020-02-25 14:28 PST (History)
6 users (show)

See Also:
Fixed By Commit(s): D69626


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Hender 2019-09-11 10:23:01 PDT
We are currently exploring to compile our software on Windows with Clang, but hit a couple Clang/LLVM bugs along the way. This one is about incorrect emulation of preprocessor.

Minimal code to reproduction the problem:
#define BUILD_LIB_XYZ ,
#define DLL(...) COMBINE(DLL_HELP_GET_THIRD_ARGUMENT(__VA_ARGS__, "dllexport", "dllimport"))
#define DLL_HELP_GET_THIRD_ARGUMENT(A, B, C, ...) C
#define COMBINE(...) __VA_ARGS__
static_assert(0, DLL(BUILD_LIB_XYZ));

The code should output "dllexport" because DLL() gets a comma inside the __VA_ARGS__ which shifts the "dllexport" from the second to the third argument.
However on Clang with -fms-compatibility this is not the case, and "dllimport" is printed instead. Note that only on Microsoft's compiler the COMBINE is necessary to get this correct behavour.

Background: In our software project we use this preprocessor trick to choose between __declspec(dllimport) and __declspec(dllexport) (or actually __attribute__((dllexport)) and __attribute__((dllimport)) on Clang). It works like this: In the build system we define a preprocessor symbol like BUILD_LIB_XYZ=, only when building library XYZ. This automatically evaluates to DLLEXPORT for all the API of library XYZ and DLLIMPORT for everything else without having to setup new macros for each library. It worked fine on Microsoft's Compiler since at least 2015, GCC and even Clang, normally at least. However not when provided with -fms-compatibility. We cannot disable that option either though because our source files sometimes use <Windows.h> for platformspecific API and as far as we know, there is no way to only selectively apply -fms-compatibility for certain #includes.

Live code on Godbolt: https://gcc.godbolt.org/z/KODv16
Comment 1 Reid Kleckner 2019-09-12 11:30:26 PDT
I think this has been reported a few times. Issue 42112 looks like the closest, but there may be others.

We're in a bad situation where clang -fms-compatibility is neither standards conformant nor MSVC compatible, and users (Boost) in the past have been frustrated with that, since they don't want to add ifdefs to cope with our buggy compatibility hacks.

My hope is that MSVC will implement conforming rules, as described here, and then we will tell users to turn that on and everything will be standard:
https://devblogs.microsoft.com/cppblog/msvc-preprocessor-progress-towards-conformance/
Comment 2 Reid Kleckner 2019-10-07 11:25:23 PDT
*** Bug 43584 has been marked as a duplicate of this bug. ***
Comment 3 Eric Astor 2019-10-29 15:17:44 PDT
Working with an instrumented version of pp-trace, I've managed to minimize the repro a bit further:

#define TEST(...) COMBINE(THIRD_ARGUMENT(__VA_ARGS__, "a", "b"))
#define THIRD_ARGUMENT(A, B, C, ...) C
#define COMBINE(...) __VA_ARGS__
static_assert(0, TEST(,));

This should output "a" because TEST() gets a comma inside the __VA_ARGS__... but with -fms-compatibility, single commas are flagged with Token::IgnoredComma to keep from being considered as an argument separator, so we instead output "b".

If we instead use

#define TEST(...) THIRD_ARGUMENT(__VA_ARGS__, "a", "b")

without the COMBINE(), we still output "b" successfully replicate the non-compliance of the MSVC preprocessor. However, MSVC behaves correctly when the intervening COMBINE() is in place - and we don't!

To fix this, we need to clear the IgnoredComma flag at the right point when expanding with an intervening macro. I don't really understand PPMacroExpansion.cpp yet, so I haven't found the right point to do this. Anyone else have an idea?
Comment 4 Eric Astor 2019-10-30 10:00:59 PDT
I believe https://reviews.llvm.org/D69626 will fix this.
Comment 5 Reid Kleckner 2020-02-25 14:28:25 PST
*** Bug 45005 has been marked as a duplicate of this bug. ***