Skip to content

Commit af067cc

Browse files
jevansaksjevansaks
authored
Fix IDispatch property returns with built-in COM (#1593)
* Fix IDispatch property returns with built-in COM * Also apply to setter.
1 parent eb7f870 commit af067cc

4 files changed

Lines changed: 79 additions & 7 deletions

File tree

‎src/Microsoft.Windows.CsWin32/Generator.Com.cs‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ FunctionPointerParameterSyntax ToFunctionPointerParameter(ParameterSyntax p)
363363

364364
// We can declare this method as a property accessor if it represents a property.
365365
// We must also confirm that the property type is the same in both cases, because sometimes they aren't (e.g. IUIAutomationProxyFactoryEntry.ClassName).
366-
if (methodDefinition.Generator.TryGetPropertyAccessorInfo(methodDefinition, originalIfaceName, context, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType) &&
366+
if (methodDefinition.Generator.TryGetPropertyAccessorInfo(methodDefinition, originalIfaceName, context, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType, out MarshalAsAttribute? propertyMarshalAsAttribute) &&
367367
declaredProperties.Contains(propertyName.Identifier.ValueText))
368368
{
369369
if (emulateMemberFunctionCallConv)
@@ -541,7 +541,7 @@ StatementSyntax InvokeVtblAndThrow() => ExpressionStatement(InvocationExpression
541541

542542
if (ccwThisParameter is not null && !ccwMethodsToSkip.Contains(methodDefHandle))
543543
{
544-
if (this.TryGetPropertyAccessorInfo(methodDefinition, originalIfaceName, context, out propertyName, out accessorKind, out propertyType) &&
544+
if (this.TryGetPropertyAccessorInfo(methodDefinition, originalIfaceName, context, out propertyName, out accessorKind, out propertyType, out propertyMarshalAsAttribute) &&
545545
ifaceDeclaredProperties!.Contains(propertyName.Identifier.ValueText))
546546
{
547547
switch (accessorKind)
@@ -889,11 +889,25 @@ static ExpressionSyntax ThisPointer(PointerTypeSyntax? typedPointer = null)
889889
// Adding an accessor to a property later than the very next row would screw up the virtual method table ordering.
890890
// We must also confirm that the property type is the same in both cases, because sometimes they aren't (e.g. IUIAutomationProxyFactoryEntry.ClassName).
891891
// Don't do this if we are using GeneratedComInterface because that doesn't support properties yet. https://github.com/dotnet/runtime/issues/96502
892-
if (methodDefinition.Generator.TryGetPropertyAccessorInfo(methodDefinition, actualIfaceName, context, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType) &&
892+
if (methodDefinition.Generator.TryGetPropertyAccessorInfo(methodDefinition, actualIfaceName, context, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType, out MarshalAsAttribute? propertyMarshalAsAttribute) &&
893893
declaredProperties.Contains(propertyName.Identifier.ValueText))
894894
{
895895
AccessorDeclarationSyntax accessor = AccessorDeclaration(accessorKind.Value).WithSemicolonToken(Semicolon);
896896

897+
if (propertyMarshalAsAttribute is not null)
898+
{
899+
if (accessorKind == SyntaxKind.GetAccessorDeclaration)
900+
{
901+
accessor = accessor.AddAttributeLists(
902+
AttributeList().WithTarget(AttributeTargetSpecifier(Token(SyntaxKind.ReturnKeyword))).AddAttributes(MarshalAs(propertyMarshalAsAttribute, null)));
903+
}
904+
else
905+
{
906+
accessor = accessor.AddAttributeLists(
907+
AttributeList().WithTarget(AttributeTargetSpecifier(Token(SyntaxKind.ParamKeyword))).AddAttributes(MarshalAs(propertyMarshalAsAttribute, null)));
908+
}
909+
}
910+
897911
if (members.Count > 0 && members[members.Count - 1] is PropertyDeclarationSyntax lastProperty && lastProperty.Identifier.ValueText == propertyName.Identifier.ValueText)
898912
{
899913
// Add the accessor to the existing property declaration.
@@ -1240,7 +1254,7 @@ private ISet<string> GetDeclarableProperties(IEnumerable<QualifiedMethodDefiniti
12401254
foreach (QualifiedMethodDefinition methodDefinition in methods)
12411255
{
12421256
rowIndex++;
1243-
if (methodDefinition.Generator.TryGetPropertyAccessorInfo(methodDefinition, ifaceName, context, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType))
1257+
if (methodDefinition.Generator.TryGetPropertyAccessorInfo(methodDefinition, ifaceName, context, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType, out MarshalAsAttribute? propertyMarshalAsAttribute))
12441258
{
12451259
if (badProperties.Contains(propertyName.Identifier.ValueText))
12461260
{
@@ -1276,11 +1290,19 @@ void ReportBadProperty()
12761290
return goodProperties.Count == 0 ? ImmutableHashSet<string>.Empty : new HashSet<string>(goodProperties.Keys, StringComparer.Ordinal);
12771291
}
12781292

1279-
private bool TryGetPropertyAccessorInfo(QualifiedMethodDefinition qmd, string ifaceName, Context context, [NotNullWhen(true)] out IdentifierNameSyntax? propertyName, [NotNullWhen(true)] out SyntaxKind? accessorKind, [NotNullWhen(true)] out TypeSyntax? propertyType)
1293+
private bool TryGetPropertyAccessorInfo(
1294+
QualifiedMethodDefinition qmd,
1295+
string ifaceName,
1296+
Context context,
1297+
[NotNullWhen(true)] out IdentifierNameSyntax? propertyName,
1298+
[NotNullWhen(true)] out SyntaxKind? accessorKind,
1299+
[NotNullWhen(true)] out TypeSyntax? propertyType,
1300+
out MarshalAsAttribute? marshalAsAttribute)
12801301
{
12811302
propertyName = null;
12821303
accessorKind = null;
12831304
propertyType = null;
1305+
marshalAsAttribute = null;
12841306

12851307
if (!this.canDeclareProperties)
12861308
{
@@ -1333,7 +1355,9 @@ private bool TryGetPropertyAccessorInfo(QualifiedMethodDefinition qmd, string if
13331355

13341356
Parameter propertyTypeParameter = qmd.Reader.GetParameter(parameters.Skip(1).Single());
13351357
TypeHandleInfo propertyTypeInfo = signature.ParameterTypes[0];
1336-
propertyType = propertyTypeInfo.ToTypeSyntax(syntaxSettings, GeneratingElement.Property, propertyTypeParameter.GetCustomAttributes().QualifyWith(qmd.Generator), propertyTypeParameter.Attributes).Type;
1358+
TypeSyntaxAndMarshaling propertyTypeSyntax = propertyTypeInfo.ToTypeSyntax(syntaxSettings, GeneratingElement.Property, propertyTypeParameter.GetCustomAttributes().QualifyWith(qmd.Generator), propertyTypeParameter.Attributes);
1359+
marshalAsAttribute = propertyTypeSyntax.MarshalAsAttribute;
1360+
propertyType = propertyTypeSyntax.Type;
13371361

13381362
if (isGetter)
13391363
{

‎test/GenerationSandbox.Tests/ComRuntimeTests.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,30 @@ public void IWbemServices_GetObject_Works()
155155

156156
Assert.NotNull(pInParamsSignature);
157157
}
158+
159+
[Fact]
160+
public void CanCallIDispatchOnlyMethods()
161+
{
162+
Assert.SkipUnless(RuntimeInformation.IsOSPlatform(OSPlatform.Windows), "Test calls Windows-specific APIs");
163+
164+
var shellWindows = (IShellWindows)new ShellWindows();
165+
166+
var serviceProvider = (IServiceProvider)shellWindows.FindWindowSW(
167+
PInvoke.CSIDL_DESKTOP,
168+
pvarLocRoot: null,
169+
ShellWindowTypeConstants.SWC_DESKTOP,
170+
phwnd: out _,
171+
ShellWindowFindWindowOptions.SWFO_NEEDDISPATCH);
172+
173+
serviceProvider.QueryService(PInvoke.SID_STopLevelBrowser, typeof(IShellBrowser).GUID, out var shellBrowserAsObject);
174+
var shellBrowser = (IShellBrowser)shellBrowserAsObject;
175+
176+
shellBrowser.QueryActiveShellView(out var shellView);
177+
178+
var iid_IDispatch = new Guid("00020400-0000-0000-C000-000000000046");
179+
shellView.GetItemObject((uint)_SVGIO.SVGIO_BACKGROUND, iid_IDispatch, out var folderViewAsObject);
180+
var folderView = (IShellFolderViewDual)folderViewAsObject;
181+
182+
_ = folderView.Application; // Throws InvalidOleVariantTypeException "Specified OLE variant is invalid"
183+
}
158184
}

‎test/GenerationSandbox.Tests/NativeMethods.txt‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,8 @@ WbemLocator
9696
CLSCTX
9797
CoSetProxyBlanket
9898
RPC_C_AUTHN_LEVEL
99-
RPC_C_IMP_LEVEL
99+
RPC_C_IMP_LEVEL
100+
IShellFolderViewDual
101+
SID_STopLevelBrowser
102+
IShellBrowser
103+
_SVGIO

‎test/Microsoft.Windows.CsWin32.Tests/COMTests.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,24 @@ public void AssociatedEnumOnMethodParameters()
248248
Assert.Equal("SFVS_SELECT", Assert.IsType<QualifiedNameSyntax>(parameter.Type).Right.Identifier.ValueText);
249249
}
250250

251+
[Fact]
252+
public void IShellFolderViewDual_ApplicationPropertyHasMarshalAsAttribute()
253+
{
254+
this.GenerateApi("IShellFolderViewDual");
255+
256+
InterfaceDeclarationSyntax ifaceSyntax = Assert.Single(this.FindGeneratedType("IShellFolderViewDual").OfType<InterfaceDeclarationSyntax>());
257+
PropertyDeclarationSyntax applicationProperty = Assert.Single(ifaceSyntax.Members.OfType<PropertyDeclarationSyntax>(), p => p.Identifier.ValueText == "Application");
258+
AccessorDeclarationSyntax getAccessor = Assert.Single(applicationProperty.AccessorList!.Accessors, a => a.Kind() == SyntaxKind.GetAccessorDeclaration);
259+
260+
// Check that the return attribute list has MarshalAs attribute
261+
AttributeSyntax marshalAsAttr = Assert.Single(FindAttribute(getAccessor.AttributeLists, "MarshalAs"));
262+
263+
// Verify it's using the correct UnmanagedType for IDispatch
264+
Assert.NotNull(marshalAsAttr.ArgumentList);
265+
Assert.Contains(marshalAsAttr.ArgumentList.Arguments, arg =>
266+
arg.Expression is MemberAccessExpressionSyntax { Name: IdentifierNameSyntax { Identifier: { ValueText: "IDispatch" } } });
267+
}
268+
251269
[Theory, CombinatorialData]
252270
public void InterestingUnmarshaledComInterfaces(
253271
[CombinatorialValues(

0 commit comments

Comments
 (0)