llvm / llvm-project

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

[clang] Different behavior between SFINAE and requires #76415

Open 16bit-ykiko opened 11 months ago

16bit-ykiko commented 11 months ago

See code below:

struct Any
{
    template<typename T> requires std::is_copy_constructible_v<T>
    operator T();
};
static_assert(std::is_copy_constructible_v<Any>);

Obviously, the type Any can be copy constructible. But clang17 reject the code. If I modify it to the equivalent form of SFINAE:

struct Any
{
    template<typename T, typename = std::enable_if_t<std::is_copy_constructible_v<T>>>
    operator T();
};
static_assert(std::is_copy_constructible_v<Any>);

then it can work. The test code is at godbolt,

llvmbot commented 11 months ago

@llvm/issue-subscribers-clang-frontend

Author: ykiko (16bit-ykiko)

See code below: ```c++ struct Any { template<typename T> requires std::is_copy_constructible_v<T> operator T(); }; static_assert(std::is_copy_constructible_v<Any>); ``` Obviously, the type Any can be copy constructible. But clang17 reject the code. If I modify it to the equivalent form of SFINAE: ```c++ struct Any { template<typename T, typename = std::enable_if_t<std::is_copy_constructible_v<T>>> operator T(); }; static_assert(std::is_copy_constructible_v<Any>); ``` then it can work. The test code is at [godbolt](https://godbolt.org/z/zajsjna7b),
zyn0217 commented 11 months ago

Reduced by cvise:

template <int v> struct integral_constant {
    static constexpr int value = v;
};

template <bool v> using bool_constant = integral_constant<v>;

struct Any;

template <typename, typename... Args>
struct is_constructible_impl : bool_constant<__is_constructible(Any, Args...)> {
};

template <typename Tp>
struct is_copy_constructible_impl : is_constructible_impl<Tp, Tp> {};

template <typename Tp>
constexpr bool is_copy_constructible_v = is_copy_constructible_impl<Tp>::value;

struct Any {
    template <typename T>
    requires is_copy_constructible_v<T>
    operator T();
};

static_assert(is_copy_constructible_v<Any>);
philnik777 commented 11 months ago

Reduced further:

template <typename Tp>
constexpr bool is_copy_constructible_v = __is_constructible(Tp, const Tp&);

struct Any {
    template <typename T>
    requires is_copy_constructible_v<T>
    operator T();
};

static_assert(is_copy_constructible_v<Any>);

When you reduce it further to

struct Any {
    template <typename T>
    requires __is_constructible(T, const T&)
    operator T();
};

static_assert(__is_constructible(Any, const Any&));

you get a good error message. This makes me think that it might not actually a bug, but simply a difference in when the constraint is checked. If you make sure the check isn't recursive by changing it to (!__is_same(T, Any) && is_copy_constructible_v<T>) everything works as expected.

16bit-ykiko commented 11 months ago

But in fact, only Clang rejects the code. Maybe it's a little hard to find something that can prove which compiler is right in C++ standard. It is no doubt that the type Any can be copy constructible. It has a default copy constructor, and the C++ standard has no statements showing that an overloaded conversion operator will suppress the automatic generation of the default copy constructor. As you say, just add ! __is_same(T, Any) to make it right, but I think this should be done by the compiler rather than users. I guess GCC and MSVC have maked this judgment.

zygoloid commented 11 months ago

During the instantiation of std::is_copy_constructible_v<Any>, the same value is recursively needed when instantiating operator Any. But it's currently being instantiated, which means the value is not available, so the constraint is non-constant, which is an error. So Clang's behavior here is correct.

The other implementations are taking a shortcut in overload resolution, and not actually properly forming the overload set when determining whether Any is copy-constructible. They never consider the option of operator T with T = Any. This is generally justified by appeal to [temp.inst]/9:

If the function selected by overload resolution can be determined without instantiating a class template definition, it is unspecified whether that instantiation actually takes place.

This rule doesn't actually permit that implementation technique used by the other compilers, because what these implementations are skipping isn't the instantiation of a class template definition, but it's intended to permit those kinds of optimizations of overload resolution. Clang somewhat intentionally doesn't take advantage of this permission, because that gives a more consistent behavior, albeit a less-permissive one, and the optimization doesn't seem to actually help much in practice beyond accepting non-conforming code such as this.

It might be interesting for someone to take this to the C++ committee and see if we can get some of these overload resolution shortcuts standardized, but unless that happens, the original example here is ill-formed per the C++ language rules.

zyn0217 commented 11 months ago

Thank you Richard for the explanation. Fwiw, class.conv.fct/p4 also says,

A conversion function is never invoked for implicit or explicit conversions of an object to the same object type (or a reference to it), to a base class of that type (or a reference to it), or to cv void.

This is likely why gcc and msvc are taking the shortcut approach, although this provision doesn't relate to the overload candidate determination.