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

add cpp_std support for MSVC #4390

Merged
merged 1 commit into from
Oct 22, 2018
Merged

add cpp_std support for MSVC #4390

merged 1 commit into from
Oct 22, 2018

Conversation

strega-nil
Copy link
Contributor

As of right now, we have to use this silly hack https://github.com/ubsan/ublib/blob/master/meson.build#L17, so I decided to add C++17 support.

Mostly based on #2836

@@ -1,8 +1,8 @@
project('msvc_cpp17', 'cpp', default_options: ['cpp_std=c++17'])

if meson.get_compiler('cpp').version().version_compare('<19.11')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to check that the compiler is msvc before checking the version, this could be gcc, clang, icc, or a host of other compilers.

Also, please squash this back into the previous patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do, I planned to make sure it worked first.

@jpakkane
Copy link
Member

The problem we are going to face with this is that there are existing projects that default to, say, gnu++17. We would really want those to work when compiling with VS. In the simd module what we do is map the values to "closest equivalent" compiler switch for the given compiler. Maybe we could do the same here? It would require having a mapping table (presumably per compiler) from possible values of cpp_std (including compiler specific modes) to the corresponding compiler switch.

In this case gnu++1z would map to c++latest when using MSVC.

We could make it error out if using a value not supported by the current compiler. This is a bit inconvenient but it is easily fixed by giving an explicit -Dcpp_std=c++something to Meson when configuring the build dir for the first time.

@strega-nil
Copy link
Contributor Author

Hmm.

Alternatively, you could deprecate cpp_std=gnu++XY, and add a cpp_compliant (or something along those lines) switch, which implies:

  • -std=c++XY for GCC/clang
  • -permissive- for MSVC

and alternatively, cpp_compliant=false means

  • -std=gnu++XY for GCC/clang
  • -permissive for MSVC

and cpp_std=gnu++XY would be equivalent to cpp_std=c++XY, cpp_compliant=false.

@jpakkane
Copy link
Member

Having more than one toggle switch for this is not really a nice UI. The dialect names have very specific meanings and people are used to them and want to use them directly. The important design decisions here seem to include:

  • is gnu++17 in the list of options presented when using VS?
  • if yes, what will be used if the user selects it? (presumably c++17 without permissive)
  • if no, what happens when people try to use it? (silently map it to vs++17, exit with an error, something else)

@strega-nil
Copy link
Contributor Author

@jpakkane fair. I think it's reasonable to not support gnu++17 when running VS, since even if it does work, it's probably unintentional (and same for the other way). If a user then wants to build a GNU++-style project with Visual C++, then they specifically should pass cpp_std on the command line, to say "Yes, I acknowledge that this might not work".

@dcbaker
Copy link
Member

dcbaker commented Oct 21, 2018

I agree that we shouldn't try to map compiler extensions unless they're actually compatible. Mapping gnu++14 to c++14 or vs++14 is just going to lead to compilation failures without warning that will confuse people. It would be better to make it a hard error. Especially since anyone writing C or C++ should know that gnu standards are not portable.

@jpakkane
Copy link
Member

jpakkane commented Oct 21, 2018

Interestingly the CI failure is exactly because of this issue (the Travis one is spurious):

meson.build:1:0: ERROR:  Value "c++14" for combo option "cpp_std" is not
one of the choices. Possible choices are: "none", "c++11".

One of the configurations in the test matrix is VS 2015. According to the code it should add C++14 but apparently does not.

But at least we know that trying to use an incorrect value causes a hard error.

@strega-nil
Copy link
Contributor Author

strega-nil commented Oct 22, 2018

I apparently used the wrong version numbers. I've changed it to a test for >=19 (i.e., VS2015) for C++14; and the first VS2015U3 version number, for -std: flags.

Also, I do have the code for /permissive- - should it be a part of this PR, or a separate one?

@jpakkane
Copy link
Member

Let's do this first. Smaller changes are better.

@strega-nil
Copy link
Contributor Author

cool!

@jpakkane jpakkane merged commit f1546e2 into mesonbuild:master Oct 22, 2018
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.

4 participants