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

GeneratedComClass doesn't apply to inherited classes #94357

Closed
jtschuster opened this issue Nov 3, 2023 · 6 comments · Fixed by #105277
Closed

GeneratedComClass doesn't apply to inherited classes #94357

jtschuster opened this issue Nov 3, 2023 · 6 comments · Fixed by #105277
Labels
area-System.Runtime.InteropServices in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@jtschuster
Copy link
Member

Placing GeneratedComClass on a base class doesn't enable deriving types to work with source generated COM.

Should we check for base classes that have the IComExposedDetails as well here?
https://github.com/dotnet/runtime/blob/df6fdefa27068126794b253d4d822706221a92db/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/IComExposedDetails.cs#L21C49-L21C49

	[GeneratedComInterface]
	[Guid("c5356681-8f8c-423a-959c-2d91f17215d3")]
	internal partial interface IFoo
	{
		int get();
	}

	[GeneratedComClass]
	[Guid("9f826481-4d92-4499-ab26-c40d9f77435b")]
	internal partial class FooBase : IFoo
	{
		public int get() => 42;
	}

	internal class Foo<T> :FooBase
	{
	}
int Main()
{
                        var cw = new StrategyBasedComWrappers();
			var foo = new Foo<int>();
			nint iptr = cw.GetOrCreateComInterfaceForObject(foo, CreateComInterfaceFlags.None);
			IFoo f = (IFoo)cw.GetOrCreateObjectForComInstance(iptr, CreateObjectFlags.None); // Fails - iptr does not implement IFoo
}
@ghost
Copy link

ghost commented Nov 3, 2023

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

Placing GeneratedComClass on a base class doesn't enable deriving types to work with source generated COM.

Should we check for base classes that have the IComExposedDetails as well here?
https://github.com/dotnet/runtime/blob/df6fdefa27068126794b253d4d822706221a92db/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/IComExposedDetails.cs#L21C49-L21C49

	[GeneratedComInterface]
	[Guid("c5356681-8f8c-423a-959c-2d91f17215d3")]
	internal partial interface IFoo
	{
		int get();
	}

	[GeneratedComClass]
	[Guid("9f826481-4d92-4499-ab26-c40d9f77435b")]
	internal partial class FooBase : IFoo
	{
		public int get() => 42;
	}

	internal class Foo<T> :FooBase
	{
	}
int Main()
{
                        var cw = new StrategyBasedComWrappers();
			var foo = new Foo<int>();
			nint iptr = cw.GetOrCreateComInterfaceForObject(foo, CreateComInterfaceFlags.None);
			IFoo f = (IFoo)cw.GetOrCreateObjectForComInstance(iptr, CreateObjectFlags.None); // Fails - iptr does not implement IFoo
}
Author: jtschuster
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: -

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 3, 2023
@AaronRobinsonMSFT
Copy link
Member

I'd like to understand this scenario before we go after it. I have some questions about how overrides are expected to and what benefits we gain as opposed to just having people place it on the most derived type.

@jtschuster
Copy link
Member Author

The more I think about it, I'm not sure there's a great way to do this without breaking trimming and AOT. Maybe we should just require it on all classes that are exposed to COM.

@jkoritzinsky
Copy link
Member

There's also no efficient way to detect this scenario in a source generator. We should just require it on all classes that are exposed to COM.

@AaronRobinsonMSFT
Copy link
Member

There's also no efficient way to detect this scenario in a source generator. We should just require it on all classes that are exposed to COM.

Nice. Does it make sense to detect that if the immediate base class has GeneratedComClass that we emit a warning? I can imagine people doing odd things with managed inheritance for COM class implementation but I'm not sure that is something we should encourage.

@jkoritzinsky
Copy link
Member

Yes, we can write an analyzer that detects that case and emits a warning in an efficient way.

@agocke agocke added this to AppModel Nov 28, 2023
@AaronRobinsonMSFT AaronRobinsonMSFT added this to the 9.0.0 milestone Jan 2, 2024
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 2, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jul 22, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Runtime.InteropServices in-pr There is an active PR which will close this issue when it is merged
Projects
Archived in project
3 participants