Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic conversion from WinRT class to implemented interface #2759

Closed
kaivol opened this issue Dec 29, 2023 · 2 comments · Fixed by #2775
Closed

Automatic conversion from WinRT class to implemented interface #2759

kaivol opened this issue Dec 29, 2023 · 2 comments · Fixed by #2775
Labels
enhancement New feature or request

Comments

@kaivol
Copy link
Contributor

kaivol commented Dec 29, 2023

Suggestion

Use case

Currently, the following code does not compile:

let vector: Collections::IVector<IStringable> = ...;
let uri = Uri::CreateUri(h!("https://github.com"))?;
vector.Append(&uri)?;
       ------  ^^^ the trait `CanInto<IStringable>` is not implemented for `Uri`
       |
       required by a bound introduced by this call
= note: required for `&windows::Foundation::Uri` to implement `IntoParam<windows::Foundation::IStringable, ReferenceType>`

Instead, one has to manually cast the Uri to a IStringable. This should not be necessary, because it is guaranteed that Uri implements IStringable.

I understand that Uri implements CanTryInto<IStringable>, which makes it compatible with TryIntoParam<IStringable> arguments, but TryIntoParam arguments only seem to be used for a few methods.

The same problem also exists for WinRT interfaces which require another interface.

Suggestion

I'm not sure if I'm missing something, but I would suggest getting rid of the distinction between CanInto/IntoParam and CanTryInto/TryIntoParam, replacing the latter with the former.
This would require changes to CanInto to support conversions using QueryInterface, but that should not be a problem, as the conversions are guaranteed to succeed.


I would be happy to hear any opinion on this; whether the proposal makes sense or if there are reasons for not doing this and keeping the current solution.

@kaivol kaivol added the enhancement New feature or request label Dec 29, 2023
@kennykerr
Copy link
Collaborator

It may be possible to combine TryIntoParam and IntoParam so long as we can do it without a run time cost to existing uses of IntoParam.

@kennykerr
Copy link
Collaborator

kennykerr commented Jan 5, 2024

This should fix it: #2775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants