Open joto opened 8 years ago
edit 2: amended for forwarding, is it correct now?
If has_type</*remove_reference?*/ T, Types...>
then:
noexcept(std::is_nothrow_constructible<
typename std::remove_reference<T>::type,
T&&
>::value)
If it needs conversion, then perhaps be something like:
noexcept(std::is_nothrow_constructible<
typename detail::convertible_type_noref<T>::type,
T&&
>::value)
To reduce clutter I just made up detail::convertible_type_noref<T>
, which is:
template <typename T>
using convertible_type_noref<T> = convertible_type<typename std::remove_reference<T>::type>;
and added member ::type
which should be obvious what it is and that it could be there ;)
What gets constructed is the target_type
, not T
. Usually this will be the same, but it doesn't have to be if there is implicit conversion going on.
That's what I said, if has_type<T, Types...>
then target_type==T
else target_type==convertible_type_noref<T>::type
@lightmare not sure I understand what you mean here. Maybe you can create a real pull request so we can better see what you mean.
Ok, but I'll probably need some help understanding where std::remove_reference
should be applied and where not.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0086r0.pdf
variant<int&>
References are supported as alternative types. Assignment to such a value is ill-formed.
In anthonyw and tomilov implementations, which use an actual union
for storage, it's clear what that translates to. But in mapbox::variant using raw byte array and reinterpret_cast
, it's difficult to grasp. I think currently it just drops the reference and stores an int
.
@lightmare: We very likely will not support references at all. They have not worked in the past and it is unclear to me, why there are a few places in the code using std::remove_reference
. I have added #70 to fix this.
Looking further, I do think we want proper noexcept
s at some point, but this needs a lot more tests to make sure every combination of constructors, assignments, swaps, etc. is handled correctly. Extra problem here is that a wrong noexcept
declaration will not be detected by the compiler.
I think the way forward is to first remove noexcept
everywhere. We should only keep it on (or add it to) functions where it is a) obviously correct and b) any implementation change we might do in the future will preserve it. This way we get to a correct interface fast. Adding more noexcept
s later is then backwards compatible and can be done piece by piece. The drawback is that this is an interface change so it might break something in users of variant. On the other hand the wrong noexcept
were always wrong anyway and nobody noticed.
We should only keep it on (or add it to) functions where it is a) obviously correct and b) any implementation change we might do in the future will preserve it.
I'll add notes to specific lines in my previous pull req. indicating which are a), b) or neither.
The constructor for
variant
is declarednoexcept
which is only true if the constructor for the underlying type is noexcept. Because we don't know which type that will be we can only mark the constructor noexcept if all underlying types have a noexcept constructor.https://github.com/mapbox/variant/blob/861faa8125ae7c2d9e41e33b3d5d97a17213c415/variant.hpp#L584-L590
There is a comment
// nothrow
in there. It is unclear what it means. Maybe it was meant to say that we expect the constructor to be "nothrow", but that really must be encoded in code.