Bug: clang - impl type cannot convert to projected type

[DHowett]

Version

2.0.230207.1 and earlier (tested 2.0.210825.3)

Summary

No response

Reproducible example

#include <winrt/Windows.Foundation.h>
#include <winrt/Windows.Foundation.Collections.h>

struct Stringy : public winrt::implements<Stringy, winrt::Windows::Foundation::IStringable> {
	Stringy() {}
	winrt::hstring ToString() {
		return L"FooBarBaz";
	}
};

int main()
{
	winrt::init_apartment();
	auto x = winrt::make_self<Stringy>();
	winrt::Windows::Foundation::IStringable broken{ *x };
	printf("Hello, %ls!\n", broken.ToString().c_str());
}

Expected behavior

works, works.

Actual behavior

Something is preventing the following conversion from taking place:

Stringy
 +--> producer_convert::operator producer_ref<IStringable>
       +--> IStringable

resulting in the following chain of error messages

1>main.cpp(18,50): error : no viable conversion from 'Stringy' to 'winrt::Windows::Foundation::IStringable'
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\impl\Windows.Foundation.1.h(170,35): message : candidate constructor (the implicit copy constructor) not viable: no known conversion from 'Stringy' to 'const winrt::Windows::Foundation::IStringable &' for 1st argument
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\impl\Windows.Foundation.1.h(170,35): message : candidate constructor (the implicit move constructor) not viable: no known conversion from 'Stringy' to 'winrt::Windows::Foundation::IStringable &&' for 1st argument
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\impl\Windows.Foundation.1.h(174,9): message : candidate constructor not viable: no known conversion from 'Stringy' to 'std::nullptr_t' for 1st argument
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\base.h(8095,9): message : candidate function
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\base.h(6749,9): message : candidate function
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\base.h(6754,9): message : candidate function
1>C:\src\ClangCppWinRTTest1\x64\Debug\Generated Files\winrt\impl\Windows.Foundation.1.h(170,35): message : passing argument to parameter here

Additional comments

If you write out the (verbose, impl types) conversion from Stringy to producer_ref<IStringable> explicitly, it works properly with no warning/error; n.b. the static_cast is using operator producer_ref<I>.

winrt::Windows::Foundation::IStringable works{
    static_cast<winrt::impl::producer_ref<winrt::Windows::Foundation::IStringable>>(*x)
};

[DHowett]

Noticed that this applies to constructor conversion only. Assignment works fine.

IStringable x = *broken; // works in both clang+msvc

However, this impacts all conversion/argument passing:

void DoSomethingWith(const IStringable& s);

DoSomethingWith(*broken); // fails in clang, works in MSVC

std::vector<IStringable> v;
v.emplace_back(*broken); // fails in clang, works in MSVC

Code trying to do this could switch to always creating a temporary of the projected type, but that would probably have a deleterious effect on productivity.

It looks like in all the tests, none seem to exercise this (though I've only looked at roughly 40% of them, so I am guessing based on the minimal hit rate for make_self 😸)

[DHowett]

You know, this works in llvm/clang 16 which just hit release candidate status. I'll close it out. Thanks!

[kennykerr]

Hi Dustin, the problem is that this requires a conversion but some versions of Clang won't call a conversion operator with the brace syntax and instead insists that it is a constructor whereas MSVC will try both. You could use assignment, which should always work:

auto x = winrt::make_self<Stringy>();
winrt::Windows::Foundation::IStringable not_broken = *x;

By the way, init_apartment is rarely needed.

[DHowett]

(I was actually mistaken in saying that Clang 16 changed the behavior; I'm gonna reopen it for now for the discussion!)

Thanks Kenny Kerr (@kennykerr), that makes sense.
The bit that worries me is the places where a conversion by assignment is unwieldy or difficult to insert, such as in argument passing or in-place construction.

I kept init_apartment from the original template I used to make the repro. Thanks for the tip!

[kennykerr]

Do you have an example? The following seems to work fine in Clang:

#include "winrt/Windows.Foundation.h"

using namespace winrt;
using namespace Windows::Foundation;

struct Stringy : implements<Stringy, IStringable> {
    hstring ToString() {
        return L"Stringy";
    }
};

void call(IStringable const& call) {
    printf("call %ls\n", call.ToString().c_str());
}

int main() {
    auto x = make_self<Stringy>();
    IStringable copy = *x;
    printf("copy %ls\n", copy.ToString().c_str());
    call(*x);
}

[DHowett]

Thanks, sorry about the lack of clarity. I've refined the test case a bit.

#include <winrt/Windows.Foundation.h>

using namespace winrt;
using namespace winrt::Windows::Foundation;

struct Stringy : public winrt::implements<Stringy, IStringable> {
	Stringy() {}
	winrt::hstring ToString() {
		return L"FooBarBaz";
	}
};

void consume(IStringable const& c) { printf("Hello %ls!\r\n", c.ToString().c_str()); }
void consume_rv(IStringable&& c) { printf("Hello %ls!\r\n", c.ToString().c_str()); }

#define TEST_CASE 1

int main()
{
	auto x = winrt::make_self<Stringy>();
	IStringable working = *x;
#if TEST_CASE == 1 // BROKEN
	IStringable broken{ *x };
#elif TEST_CASE == 2 // WORKING (!)
	consume(*x);
#elif TEST_CASE == 3 // BROKEN
	std::vector<IStringable> v;
	v.emplace_back(*x);
#elif TEST_CASE == 4 // BROKEN (!)
	std::vector<IStringable> v;
	v.push_back(*x);
#elif TEST_CASE == 5 // BROKEN
	IStringable foo{ nullptr };
	foo = *x;
#elif TEST_CASE == 6 // BROKEN
	consume_rv(*x);
#endif
}

1. FAIL The original case, established as not working
2. PASS I originally thought argument passing was broken, but it seems to work.
3. FAIL Emplace construction fails; this is case 1 but somebody else is calling the constructor
4. FAIL I didn't expect this. I had to add a test case 6 to figure out why...
5. FAIL Assignment didn't work! Copy construction by assignment works, operator= doesn't work.
6. FAIL This is probably why push_back fails, and why I thought other argument passing cases failed. Rvalue references.

[DHowett]

(Each one of these test cases is distilled from something in the Windows Terminal source that failed -- I am trying to bootstrap building it with Clang, and we're at about 75% of projects building. The ones that aren't are suffering from this pretty acutely.)

[kennykerr]

Well, I'm no language lawyer and can't begin to explain why some work and some don't. C++ is just nuts. 😜

Generally speaking, I think it mostly comes down to whether a particular compiler will accept one or more chained conversions in order to arrive at the desired type and this just isn't implemented very consistently so if you need to support a wider range of compilers you end up having to be far more verbose in the way you write your expressions. The rationale is that hiding such conversions can inject costly operations that really ought to be explicitly requested. In your failing cases, for example, an AddRef is required but Clang doesn't want to inject that unknowingly, so you need to explicitly convert the implementation into a IStringable value, not just a reference, before proceeding.

[DHowett]

Fair enough!

In the interest of figuring out exactly what's going on, I stripped things out until it stopped reproing...

struct Final {
	Final(void* ptr) {}
};

struct First {
//                 vvvvv (1)
	operator Final const() const noexcept
	{
		return { (void*)this };
	}
};

void IsBrokenP() {
	First* fi;
	Final fn{ *fi };
}

With the const labelled 1, it fails. Without it? Works perfectly fine and generates the same LLVM assembly as "with const and using Final fn=*fi;"¹

At this point... I think what this is is a Clang bug. 🤣

Footnotes

1. With the exception of the mangled name of the conversion operator, of course. ↩

[DHowett]

Nevermind, it's a C++ language standard bug. That was a wild ride.

[kennykerr]

[DHowett]

Ha!

So, I've been thinking about this a bit more. The conversion operator in question produces a const producer_ref that's convertible to its base type const I. Far and away, the majority of folks are going to take the return from that operator and (1) store it in a non-const local or (2) pass it to a function taking const& I as a parameter.

If we were to change the return type from producer_ref<I> const to just producer_ref<I>, it would still be able to bind in case 2 and it would not be erroneously ignored for rvalue reference conversion for case 1.

Given that the WinRT runtime doesn't participate in constness, I don't think we'd be losing anything. It would be strictly correctness-widening.

There's a good summary on StackOverflow that explains what's going on. Excerpted:

In this case the conversion is Bar -> const Foo -> Foo via the user-defined constructor for the move constructor. For the copy constructor it is Bar -> const Foo.

However in the copy-initialization of the temporary, const-qualifier conversions are "subsumed" in the initialization. [over.ics.ref]/2 Therefore the move constructor sequence is not worse than that of the copy constructor.

In such a case the rvalue reference is a tie-breaker and so the move constructor is chosen in overload resolution.

However, a const Foo cannot be bound to a non-const rvalue reference. Therefore the program is ill-formed. ([dcl.init.ref]/5.4.3, DR 1604)

Would you be willing to accept a change that de-const-qualifies operator producer_ref<I>? The member can remain const, of course.