mpark / variant

C++17 `std::variant` for C++11/14/17
https://mpark.github.io/variant
Boost Software License 1.0
659 stars 88 forks source link

`static_assert` vs SFINAE #3

Closed mpark closed 7 years ago

mpark commented 8 years ago

For type-based operations, there must be exactly one instance of T in the list of alternatives. Determine whether this violation should be a static_assert within the function or a SFINAE + = delete;.

mpark commented 8 years ago

Should be SFINAE + = delete as per http://cplusplus.github.io/LWG/lwg-defects.html#2367

viboes commented 8 years ago

I don't see how this issue is related. To which operations are you referring to? For get, it should be assert as it is done for pair and tuple.

mpark commented 8 years ago

@viboes: I think you're right. In that comment, I was referring to constructors. That is, std::is_constructible<variant<int, std::string>, in_place_type_t<foo>>::value should be false, rather than true or a hard error. What do you think?

viboes commented 8 years ago

Ah I see how the issue is related to these constructors. The overloaded constructor should not participate in overload resolution until Foo is one of the types.

I don't see what do you mean by + = delete. There is nothing to delete. SFINAE do everything; isn't it?

mpark commented 8 years ago

I think semantically they would behave the same. It came up because the current proposal is worded as follows:

Remarks: The function shall not participate in overload resolution unless T is one of Types.... The function shall be = delete if there are multiple occurrences of T in Types.... If T’s selected constructor is a constexpr constructor, this constructor shall be a constexpr constructor.

Perhaps we need to revisit the wording and consider the SFINAE wording of "does not participate in overload resolution".

In terms of the actual implementation, = delete would report different error messages. Whether it's better or not is perhaps subjective.

#include <iostream>
#include <type_traits>

struct Bar {
  Bar() = delete;
};

template <typename T>
struct Foo {
  template <typename U = T,
            std::enable_if_t<std::is_default_constructible<U>::value, int> = 0>
  Foo() {}

  template <typename U = T,
            std::enable_if_t<!std::is_default_constructible<U>::value, int> = 0>
  Foo() = delete;
};

int main() {
  static_assert(!std::is_default_constructible<Bar>::value, "");
  static_assert(!std::is_default_constructible<Foo<Bar>>::value, "");
}

This compiles successfully with or without the presence of the deleted constructor. If I were to attempt to construction however, the error messages are different:

With = delete, I get:

a.cc:22:3: error: call to deleted constructor of 'Foo<Bar>'
  Foo<Bar>{};
  ^       ~~
a.cc:16:3: note: 'Foo<Bar, 0>' has been explicitly marked deleted here
  Foo() = delete;
  ^
1 error generated.

whereas without the = delete, I get:

a.cc:18:3: error: no matching constructor for initialization of 'Foo<Bar>'
  Foo<Bar>{};
  ^       ~~
/usr/local/Cellar/llvm36/3.6.2/lib/llvm-3.6/bin/../include/c++/v1/type_traits:235:78: note:
      candidate template ignored: disabled by 'enable_if' [with U = Bar]
  ...<bool _Bp, class _Tp = void> using enable_if_t = typename enable_if<_Bp, _Tp>::t...
                                                                         ^
a.cc:9:8: note: candidate constructor (the implicit copy constructor) not viable:
      requires 1 argument, but 0 were provided
struct Foo {
       ^
a.cc:9:8: note: candidate constructor (the implicit move constructor) not viable:
      requires 1 argument, but 0 were provided
1 error generated.
viboes commented 8 years ago

I see. I would let this as an implementation detail.

mpark commented 8 years ago

Yep. The updated proposal will use the SFINAE wording for these constructors.