llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.4k stars 12.15k forks source link

cosntexpr constructor breaks out expression SFINAE #41675

Closed lichray closed 5 years ago

lichray commented 5 years ago
Bugzilla Link 42330
Resolution INVALID
Resolved on Jun 19, 2019 15:42
Version trunk
OS Windows NT
Attachments reproduce
CC @DougGregor,@zygoloid

Extended Description

Clang does not compile the following program:

include

struct A {}; struct B {};

template struct MyPtr { MyPtr() = default; template constexpr MyPtr(MyPtr v) : ptr(v.ptr) {}

T* ptr_; };

template struct __identity { using type = T; };

template struct __overload;

template <> struct __overload<> { void operator()() const; };

template <class _Tp, class... _Types> struct overload<_Tp, _Types...> : overload<_Types...> { using overload<_Types...>::operator(); static auto __test(_Tp (&&)[1]) -> identity<_Tp>;

template auto operator()(_Tp, _Up&& t) const -> decltype(test({ std::forward<_Up>(__t) })); };

template <class _Tp, class... _Types> using best_match_t = typename std::invoke_result_t<overload<_Types...>, _Tp, _Tp>::type;

void test() { using t = __best_match_t<MyPtr, MyPtr, MyPtr>; }

:11:23: error: cannot initialize a member subobject of type 'B ' with an lvalue of type 'A ' MyPtr(MyPtr v) : ptr(v.ptr) {} ^ ~~ :32:26: note: in instantiation of function template specialization 'MyPtr::MyPtr' requested here -> decltype(__test({ std::forward<_Up>(__t) })); ^ [...]/include/c++/v1/type_traits:4425:23: note: while substituting deduced template arguments into function template 'operator()' [with _Up = MyPtr]

Essentially it is saying that a constexpr constructor is called and failed during substitution, but that shouldn't happen. What should happen though is MyPtr being considered convertible to MyPtr, but it's a worse match. MSVC gives the right answer.

Commenting out the constexpr on line 10 fixes this issue.

lichray commented 5 years ago

I think what's going on here is:

  • When initializing the array, Clang and GCC think the conversion from the list element to the array element type happens "inside" the braces
  • When initializing the class with the constructor, Clang (but not GCC) thinks the conversion from the list element to the parameter type happens "outside" the braces

Unfortunately, I think that the second point is a bug. Consider a case such as this:

template struct X { constexpr operator int() { return N; }; };

using V = __best_match_t<X<123>, short>;

Here, in order to tell whether the list-initialization is valid, we may need to evaluate the call to X<123>::operator int(), so we should instantiate it.

Agreed.

So I think your workaround just "happens to work". (And it doesn't work for GCC, which only accepts the above case [...]

Yes, the intention is to reject V.

So the conclusion in my mind is that this constructor

template constexpr MyPtr(MyPtr v) : ptr(v.ptr) {}

is just not sfinae-friendly.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 5 years ago

That approach disables user-defined conversions from _Up to _Tp. Is that what you intended?

No, but I'm not sure whether I get it -- none of the compilers think so:

Sorry, that's a mistake on my part.

I think what's going on here is:

  • When initializing the array, Clang and GCC think the conversion from the list element to the array element type happens "inside" the braces
  • When initializing the class with the constructor, Clang (but not GCC) thinks the conversion from the list element to the parameter type happens "outside" the braces

Unfortunately, I think that the second point is a bug. Consider a case such as this:

template struct X { constexpr operator int() { return N; }; };

using V = __best_match_t<X<123>, short>;

Here, in order to tell whether the list-initialization is valid, we may need to evaluate the call to X<123>::operator int(), so we should instantiate it. So I think your workaround just "happens to work". (And it doesn't work for GCC, which only accepts the above case because it triggers instantiation here -- and also because it has a bug with how it handles references with no preceding initialization in constant expressions, but that's not really relevant here.)

lichray commented 5 years ago

I assume that should be

template struct __undo_braces { __undo_braces(_Tp); };

Yes

That approach disables user-defined conversions from _Up to _Tp. Is that what you intended?

No, but I'm not sure whether I get it -- none of the compilers think so:

https://godbolt.org/z/CARCEx

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 5 years ago

I assume that should be

template struct __undo_braces { __undo_braces(_Tp); };

? That approach disables user-defined conversions from _Up to _Tp. Is that what you intended?

lichray commented 5 years ago

Clang is right; checking the validity of braced initialization requires evaluation in order to do a narrowing check, so it triggers instantiation of constexpr functions

Do you need the array and braced initialization here?

I do need a braced initialization to do narrowing check; my current workaround is

template struct __undo_braces { __undo_braces(T); }

...

static auto test(undo_braces<_Tp>) -> __identity<_Tp>;

Is that just "happens to work?"

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 5 years ago

Clang is right; checking the validity of braced initialization requires evaluation in order to do a narrowing check, so it triggers instantiation of constexpr functions

Do you need the array and braced initialization here?