-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clanc-cl's -fms-compatibility breaks aliased operators (or
, and
, etc.) for user provided system headers
#41772
Comments
After further digging I found out that it can be reproduced with direct clang -cc1 test.cpp -isystem external/include -fms-compatibility Above command fails also on Linux (WSL with clang-7 in my case). Reproduced with clang-7, clang-8 and trunk version. |
The last update for today: I've pinned down the difference to the lexer with -dump-tokens Lexer with MS compatibility reports
identifier 'x' [LeadingSpace] Loc=<external/include\lib.h:4:12>
identifier 'x' [LeadingSpace] Loc=<external/include\lib.h:4:12> The question remains: why only for headers originated from -isystem directories...? -I just works. |
Finally I found the logic behind this, clang\lib\Lex\Preprocessor.cpp, lines 715-720: Identifier.setIdentifierInfo(II); I suppose my issue is not an issue and is "working as intended", even though the reasoning behind is unknown to me. Does MSVC use internally reserved names as variables? Also, can anything be done here? |
This is being amplified in that as of cmake 3.19, it is passing user system headers via -imsvc path/to/header/ |
I think clang is doing this so that the iso646.h header compiles with clang: You can see here that MSVC accepts this source, but normally clang rejects it: The "is system header" check is a proxy for "are we in iso646.h or something like it". IIRC Boost likes to use these operators, and they have some macros for them as well. You should be able to work around the problem either by including iso646.h, or by replicating the relevant #defines. I am aware that C++ says these should always be available, but clang was essentially trying to copy MSVC's behavior here. I did some more testing, and it seems that if you are using MSVC, you have to include iso646.h to use the alternative operator names: So, while I understand that clang is failing to accept conforming C++ source code with -fms-compatibility set, it seems to me that MSVC would also reject this source code. In other words, clang-cl is matching MSVC. Are there any examples of something that MSVC would accept but clang rejects? If not, this seems like a low-severity bug. |
https://reviews.llvm.org/D33782 are relevant here. |
It seems to me then that we can power down this pre-processor hack when foperator-names is enabled. The net result will be that people using |
The ciso646 is not at play here with clang-cl. It is filtered by __cplusplus being defined inside iso646.h. |
You're correct, iso646.h isn't relevant, but I do recall that, in the past, we had to make accommodations for users like Boost using Anyway, I think we can revert this change. I sent https://reviews.llvm.org/D108720 to propose it. If users need to use old Windows SDK versions that use |
I believe this one can be closed since it was fixed in release 14.0.0 thanks to @rnk https://godbolt.org/z/svEEe77G9 |
Extended Description
Suppose you have two files,
main.cpp
#include <lib.h>
int main(){}
and external library
external/include/foo.h
:#pragma once
bool foo(int x) {
int y; // generate some warning to check whenever -isystem works
return x or x;
}
You can compile it with
clang-cl /Iexternal\include /W4 test.cpp
and it will issue an warning abouty
.external\include\lib.h(3,9): warning: unused variable 'y' [-Wunused-variable]
You can try to suppress it by adding
-Xclang -isystemexternal\include
, but it will lead to strange error:It turns out that alternative tokens are breaking compilation (only) for headers which are under
-isystem
flagged directories.or
in main.cpp always compiles.I've also tested
and
andbitand
out of https://en.cppreference.com/w/cpp/language/operator_alternative, same result.You can work it around with preprocessor "-Dor=||":
PS. I hope I've tagged this issue correctly to frontend category.
The text was updated successfully, but these errors were encountered: