Skip to content
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

Audit MSVC references in cmake files to consider clang++ #1669

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Audit MSVC references in cmake files to consider clang++ #1669

merged 1 commit into from
Sep 26, 2023

Conversation

Maetveis
Copy link
Contributor

@Maetveis Maetveis commented Sep 26, 2023

There are three major compilers on Windows targeting the MSVC ABI (i.e. linking with microsofts STL etc.):

  • MSVC
  • clang-cl aka clang with the MSVC compatible CLI
  • clang++ aka clang with gcc compatible CLI

The cmake variable MSVC is only set for the first two as it defined in terms of the CLI interface provided:

Set to true when the compiler is some version of Microsoft Visual
C++ or another compiler simulating the Visual C++ cl command-line syntax.

(from cmake docs)

For many of the checks in the library its the ABI that matters not the cmdline, so check CMAKE_CXX_SIMULATE_ID too, if it is MSVC the current compiler is targeting the MSVC ABI. This handles clang++

Fixes: #1597

@google-cla
Copy link

google-cla bot commented Sep 26, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

There are three major compilers on Windows targeting the MSVC ABI (i.e.
linking with microsofts STL etc.):
  - `MSVC`
  - `clang-cl` aka clang with the MSVC compatible CLI
  - `clang++` aka clang with gcc compatible CLI

The cmake variable `MSVC` is only set for the first two as it defined in
terms of the CLI interface provided:

> Set to true when the compiler is some version of Microsoft Visual
> C++ or another compiler simulating the Visual C++ cl command-line syntax.

(from cmake docs)

For many of the tests in the library its the ABI that matters not the
cmdline, so check `CMAKE_CXX_SIMULATE_ID` too, if it is `MSVC` the
current compiler is targeting the MSVC ABI. This handles `clang++`
@Maetveis
Copy link
Contributor Author

I don't think the sanitizer builds failing are relevant, this shouldn't change anything on Linux and all windows jobs pass.

@dmah42
Copy link
Member

dmah42 commented Sep 26, 2023

I don't think the sanitizer builds failing are relevant, this shouldn't change anything on Linux and all windows jobs pass.

agreed. filed #1670

@dmah42 dmah42 merged commit c9106a7 into google:main Sep 26, 2023
54 of 60 checks passed
@Maetveis Maetveis deleted the msvc_consider_clang_plus_plus branch September 26, 2023 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] cxx03_test Fails Due to C++ Version
2 participants