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

refine noexcept specifiers on constructors #62

Merged
merged 3 commits into from
Jan 28, 2016
Merged

Conversation

lightmare
Copy link
Contributor

Refs #61

A couple additions to detail::value_traits:

bool is_direct = direct_index != invalid_value;
bool is_valid = index != invalid_value;
typedef value_type = remove_reference<T>; // do it here so there's less clutter on use
typedef target_type = tuple_element<index, tuple<Types...>>

Because of the target_type, instantiation of value_traits<T, U> fails if !is_convertible<T, U>. I use this for enabling conversion constructor overloads.

I had to comment two tests because of that, I don't know how to test for "substitution failure must be an error". I tried a few things, but always ended up with invalid use of incomplete type in std::tuple_element<0ul, std::tuple<>>.

@lightmare
Copy link
Contributor Author

Hm, so gcc 4.7 doesn't have ./variant.hpp:831:25: error: ‘is_nothrow_destructible’ is not a member of ‘std’. That can be wrapped in #ifdef.

But MSVC has more trouble. Question is: is it the compiler's failure to understand SFINAE, or am I using it wrong? Here's the line where Traits substitution fails (on purpose) if T is not convertible to any stored type: https://github.com/mapbox/variant/pull/62/files#diff-dab208458c807c54b530c13b19c80187R589 . Does anyone know whether deferring the failure until Traits::target_type is needed in noexcept would make MSVC happy?

Previously I used value_traits substitution failure for enabling
the conversion constructor, but MSVC couldn't get over tuple_element
with out-of-bounds index, which I used inside value_traits to make
it fail for non-convertible types.

So now value_traits substitution always succeeds, with target_type
being void for non-convertible types, and the constructor is enabled
via is_valid flag.
@artemp
Copy link
Contributor

artemp commented Jan 28, 2016

Hm, so gcc 4.7 doesn't have ./variant.hpp:831:25: error: ‘is_nothrow_destructible’ is not a member of ‘std’. That can be wrapped in #ifdef.

std::is_nothrow_destructible is part of C++11 and this only means gcc 4.7 doesn't confirm to the standard

But MSVC has more trouble.

Which version are you using ? It seems to work on AppVeyor
/cc @lightmare @joto

@artemp
Copy link
Contributor

artemp commented Jan 28, 2016

Overall looks like move in the right direction thanks again @lightmare.

I'm inclined to merge this PR as it's definitely an improvement from just having noexcept :)

@lightmare
Copy link
Contributor Author

Which version are you using ? It seems to work on AppVeyor

I'm not using MSVC. The original commit failed on AppVeyor, I fixed that in cfa1f95

Ah I missed it, great !

@artemp artemp merged commit cfa1f95 into mapbox:master Jan 28, 2016
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.

2 participants