-
Notifications
You must be signed in to change notification settings - Fork 51
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 Optional based on variantSrc::variant #806
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Minor points added as comments.
Can you please add test coverage for openPMD::auxiliary::Option
in test/AuxiliaryTest.cpp
?
test/AuxiliaryTest.cpp
Outdated
|
||
Option<int> opt; | ||
|
||
REQUIRE_THROWS(opt.get()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line calls std::terminate()
while throwing mpark::throw_bad_variant_access
in GCC 4.8 and 4.9.
For some reason, MPARK_EXCEPTIONS
is not defined for this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR: mpark/variant#76
Will push a work-around here and unify the variant include, to avoid potential ODR issues.
Add unit tests for `auxiliary::Option`
Enable exceptions with older GCC. See upstream PR for release 1.4.0.
c6d486f
to
1e51f7d
Compare
Adds a class template
openPMD::auxiliary::Option
to provide for somewhat the same functionality as C++17'sstd::optional
and uses it in several places in the ADIOS2 backend. Close #563.