llvm / llvm-project

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

Clang does not use user defined conversion while gcc does #65137

Open Horrih opened 1 year ago

Horrih commented 1 year ago

Hello all,

Disclaimer : I am not well versed in the standard, so the issue might come from gcc being more permissive.

While migrating an app from gcc to clang, we encountered the following issue with user conversion.

struct A
{
    A(const A& other);
    A(A&& other);
};

struct B
{
    operator const A();
};

void f()
{
    B b;
    A a(b);
}

The issue can be reproduced on godbolt.org with clang 16 and std=c++14. Either of the following fix the compilation error

Here is the compiler output

src/zob.cpp:15:9: error: no viable conversion from 'B' to 'A'
    A a(b);
        ^
src/zob.cpp:3:5: note: candidate constructor not viable: no known conversion from 'B' to 'const A &' for 1st argument
    A(const A& other);
    ^
src/zob.cpp:4:5: note: candidate constructor not viable: no known conversion from 'B' to 'A &&' for 1st argument
    A(A&& other);
    ^
src/zob.cpp:9:5: note: candidate function
    operator const A();
    ^
src/zob.cpp:4:11: note: passing argument to parameter 'other' here
    A(A&& other);
llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

shafik commented 1 year ago

This looks like a clang bug, it looks like we are treating the move constructor as viable in C++14 but I am not sure why. The diagnostic is not super helpful either.

CC @zygoloid

zygoloid commented 1 year ago

This looks like it's falling through an odd crack in the language wording.

As @shafik notes, the problem here is that overload resolution is picking the move constructor, but then when we come to actually call the move constructor, we can't initialize its reference parameter. The issue is that we are able to form an implicit conversion sequence from b to A&&, despite the corresponding initialization not being valid.

Implicit conversion sequence formation defers to the initialization rules, but it's not really clear about under what circumstances an implicit conversion sequence can't be formed. A reasonable-seeming interpretation would be that if reference initialization says that the reference binding is ill-formed, then an implicit conversion sequence can't be formed. But in C++14, we end up here:

https://timsong-cpp.github.io/cppwp/n4140/dcl.init.ref#5.2.2.1

... and [over.match.copy] says to ignore cv-qualifiers in the conversion function's type, so we will successfully pick the operator const A there. The subsequent direct binding would be ill-formed, but the initialization wording doesn't check that.

So it's not incredibly clear whether this condition should result in there being no ability to form an implicit conversion sequence. Arguably "the direct-initialization does not result in a direct binding" if the direct-initialization is not even valid, so perhaps for that reason we should fail to form an implicit conversion sequence here. It seems pretty clear to me that the move constructor should not be viable in this example. (And I've not looked through the Clang code to see if the above is what's happening.)

I'd expect the reason why we reject in C++11 and C++14, but accept in C++98 and in C++17 onwards is: