Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Support paths when defining supertrait for [com_interface] #110

Merged
merged 5 commits into from
Nov 18, 2019

Conversation

Rantanen
Copy link
Contributor

Instead of having to use the IUnknown everywhere, this PR adds support for:

#[com_interface(...)]
trait Foo: com::interfaces::iunknown::IUnknown {}

Most of the bits were very straight forward. The one places where the implementation got a bit complex (and a bit fragile) is the #[derive(VTable)] expansion, which used to derive the interface ident from the field name (making assumptions such as all interfaces starting with I) - the new implementation assumes the type is defined as an associated type of the original interface:

  <dyn path::to::some::Interface as ...>::VTable
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ use this

This branch is in a conflict with #108 and will fail to build with #107. I'll merge things around once I have a better idea what PRs will land and in which order.

Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Lets add another test that makes sure use ... works. Perhaps we could have two modules one where use ... is used and one where it is not and then we can test that both work.

@Rantanen
Copy link
Contributor Author

Added test for use and relative path with super::. Was debating whether to write these in a different file or not, but given the tests are so closely related I just put them in the same file.

.. technically IUnknown is still a Path. :)

@Rantanen
Copy link
Contributor Author

Also rebased on top of the new master branch to resolve the conflicts with the expect branch.

@Rantanen Rantanen requested a review from rylev November 18, 2019 17:01
Copy link
Contributor

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@rylev rylev merged commit dadad4d into microsoft:master Nov 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants