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

Used macros to allow compilation with C++11 and constexpr #25

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Conversation

oxt3479
Copy link
Contributor

@oxt3479 oxt3479 commented Jul 16, 2020

Changes were alluded to in a previous pull request. Getting this through early is beneficial as changes needed for CUDA integration will have a lot of conflicts with this PR if not done on top of them.

Previously, compilation with C++11 was not possible because of limited utility of constexpr. IMATH_CONSTEXPR14 is now used primarily on methods which are not return statements. It is defined under the condition that the compiler exceeds C++14, and is specified in config/ImathConfig.h.in

@cary-ilm
Copy link
Member

It looks like something got botched here, the files contain the merge markers:
<<<<<

Did something not get merged properly?

@oxt3479
Copy link
Contributor Author

oxt3479 commented Jul 16, 2020

Yep, Working or resolving that now. Should be quick.

@oxt3479
Copy link
Contributor Author

oxt3479 commented Jul 16, 2020

Merging issues were fixed.

@lgritz
Copy link
Contributor

lgritz commented Jul 16, 2020

I assume that the way you found these is just to build under C++11 and fix everyplace that you got an error about constexpr not being allowed?

//
// Constexpr C++14 conditional definition
//
#if(__cplusplus >= 201402L)
Copy link
Contributor

Choose a reason for hiding this comment

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

Older versions of MSVS were not good about setting this, and so you will end up with C++14 compliant MSVS versions that will not constexpr these even though they can. I would recommend the following test instead:

#if (__cplusplus >= 201402L) || (defined(_MSC_VER) && _MSC_VER >= 1914)

Copy link
Contributor

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM!

(modulo the comment I made about ensuring that we use the constexpr on MSVS)

@oxt3479
Copy link
Contributor Author

oxt3479 commented Jul 16, 2020

I assume that the way you found these is just to build under C++11 and fix everyplace that you got an error about constexpr not being allowed?

Yeah that was the primary means of finding these.

@oxt3479 oxt3479 merged commit ac39a81 into AcademySoftwareFoundation:master Jul 16, 2020
@lgritz
Copy link
Contributor

lgritz commented Jul 16, 2020

This is really great, @oxt3479, I've wanted this for years.

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.

3 participants