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 option for Visual Studio C++ compiler #2836

Closed
wants to merge 2 commits into from

Conversation

polarina
Copy link
Contributor

No description provided.

@@ -188,6 +188,9 @@ def get_options(self):
'C++ exception handling type.',
['none', 'a', 's', 'sc'],
'sc'),
'cpp_std': coredata.UserComboOption('cpp_std', 'C++ language standard to use',
['none', 'c++11', 'c++14', 'c++17', 'c++latest'],
Copy link
Member

Choose a reason for hiding this comment

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

[Flake8]

[E128] continuation line under-indented for visual indent

@@ -188,6 +188,9 @@ def get_options(self):
'C++ exception handling type.',
['none', 'a', 's', 'sc'],
'sc'),
'cpp_std': coredata.UserComboOption('cpp_std', 'C++ language standard to use',
['none', 'c++11', 'c++14', 'c++17', 'c++latest'],
'none'),
Copy link
Member

Choose a reason for hiding this comment

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

[Flake8]

[E128] continuation line under-indented for visual indent

@jibsen
Copy link
Contributor

jibsen commented Dec 27, 2017

Wouldn't you have to check the MSVC version? in VS2015 there is only c++14 and c++latest, and in versions before that I don't think the switch is there.

@polarina
Copy link
Contributor Author

@jibsen right, I missed that. From what I gathered, MSVC tends to ignore unknown compiler flags, which is presumably why the VS2015 AppVeyor build surprisingly passed (VS2015 does not support C++17).

Speaking of which, is there a way to disable the new 13 vc c++17 test for VS2010 and VS2015 builds?

@nirbheek
Copy link
Member

You can skip a test with error('MESON_SKIP_TEST <msg>')

@polarina polarina force-pushed the vscppstd branch 3 times, most recently from 20005fc to faffe03 Compare December 28, 2017 15:29
@polarina
Copy link
Contributor Author

The patches should be ready. I am unsure as to why the VS2015 build on AppVeyor is failing. If someone more knowledgeful about AppVeyour could take a look, that'd be appreciated.

Thanks!

@nirbheek
Copy link
Member

@jpakkane could you please restart that appveyor job? It error out due to an unrelated problem.

@jpakkane
Copy link
Member

jpakkane commented Jan 6, 2018

The conceptual problem that this brings is that there are many projects that have default values for cpp_std set to, say, c++14. The question is what should we do if compiling these with VS? Currently there is no option so they are ignored. But what should happen in the future? Erroring out due to an invalid value is the simple thing but it's not a particularly good UX. Should they be remapped to "at least as new or newer version" so that for example c++11 gets remapped to c++17? Something else?

@polarina
Copy link
Contributor Author

polarina commented Jan 6, 2018

Good question. I am unconvinced that promoting a language version request for, say, C++11 to C++17, is a good idea. C++17 brings in a number of incompatibilities with the C++11 standard -- removal of the register keyword, for example, which VS will enforce if given /std:c++17. Future standards may do the same.

@nirbheek
Copy link
Member

nirbheek commented Feb 9, 2018

I think we should emit a warning when an invalid cpp_std option is set while building with a version of MSVC that supports the /std option. According to the docs the option is supported starting with Visual C++ 2013 Update 3.

@nirbheek nirbheek added compilers OS:windows Winodows OS specific issues labels Feb 9, 2018
@mikkelfj
Copy link

mikkelfj commented Mar 3, 2018

@polarina

The patches should be ready. I am unsure as to why the VS2015 build on AppVeyor is failing. If someone more knowledgeful about AppVeyour could take a look, that'd be appreciated.

I'm not sure at all this is related, but I was having problems with VS2015 x86 for C:

#2134

@polarina polarina force-pushed the vscppstd branch 2 times, most recently from 62fcd96 to 0374def Compare April 19, 2018 16:26
@polarina
Copy link
Contributor Author

I've rebased the changes on top of current master. Can we get another round of reviews, please?

Copy link
Member

@nirbheek nirbheek left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in getting this review done. It slipped through the cracks.

## Added `cpp_std` option for the Visual Studio C++ compiler

Allows the use of C++17 features and experimental not-yet-standardized
features. Valid options are `c++11`, `c++14`, `c++17`, and `c++latest`.
Copy link
Member

Choose a reason for hiding this comment

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

This should be added into the docs/markdown/snippets directory.

@@ -221,6 +231,9 @@ def get_option_compile_args(self, options):
std = options['cpp_eh']
if std.value != 'none':
args.append('/EH' + std.value)
std = options['cpp_std']
if std.value not in ['none', 'c++11']:
args.append('/std:' + std.value)
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this value added for c++11? Please add a comment about that.

Also, why aren't we warning when cpp_std: on an MSVC compiler that does not support the /std: argument?

project('wincpp', 'cpp')

if meson.get_compiler('cpp').version().version_compare('<19.11')
error('MESON_SKIP_TEST Visual Studio 2017 (version 15.3) or later is required for C++17 support')
Copy link
Member

Choose a reason for hiding this comment

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

Why are you skipping the test here? Also where's the cpp_std: option set for this test?

@jpakkane
Copy link
Member

jpakkane commented Oct 7, 2018

The code around language version has changed so much that this probably requires a full reworking. Thus closing to reduce clutter on the MR page. Please open a new MR with updated code if you get this working against latest trunk. Thanks and sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compilers OS:windows Winodows OS specific issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants