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

require min of VS2015 Update 3 for constexpr #1686

Merged
merged 1 commit into from
Oct 4, 2016

Conversation

MikeHolman
Copy link
Contributor

No description provided.

@dilijev
Copy link
Contributor

dilijev commented Oct 4, 2016

Good thing I just updated to VS 2015 Update 3 :P

This won't affect support for VS 2013, right? See #1484

@MikeHolman
Copy link
Contributor Author

This will not impact VS2013 support, as all constexpr code is behind flag/not used for impacting runtime behavior. I had originally set min version where we support constexpr to VS2015 RTM (used for some static asserts, behind the HAS_CONSTEXPR flag). Unfortunately, VS2015 RTM had some bugs with constexpr, in our case causing the assert condition to not be considered static.

I think this was probably fixed in Update 1 since VC team put out a whole blog post talking about fixing/improving constexpr. But I have Update 3, which I know works, so figured we may as well be conservative.

@jianchun
Copy link

jianchun commented Oct 4, 2016

How about clang? Try turn it on for non MSC?

@MikeHolman
Copy link
Contributor Author

@jianchun I thought about it but didn't know what exactly the condition should be and this seemed sufficiently inconsequential that whoever has some particular want can add it in. But if you know the appropriate macro condition I'll add it.

@dilijev
Copy link
Contributor

dilijev commented Oct 4, 2016

@MikeHolman is there a significant runtime performance gain (or other benefits) for using constexpr IFF the compiler version is good? If so, I'll make sure all of our build infra that we use to produce builds we use for testing has Dev14 U3.

Also, in order to get those same benefits we should definitely turn it on for clang if it is supported (AFAIK it is).

As a data point, my Dev14 U3 cl.exe version is:

Prod version:   14.00.24215.1
File version:   19.00.24215.1

I'd assume that translates to 190024215 which seems slightly newer than the version you indicated.

@MikeHolman
Copy link
Contributor Author

Ok, by popular demand, I've loosened the restriction so it will be defined for clang build as well.

@dilijev Right now the only thing it is being used for is as a condition in CompileAsserts, so it's useful to have in CI, but not going to see any perf benefit (and I think in general C++ backends are good enough at this type of code that it wouldn't be significant win).

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.

5 participants