Skip to content

Commit

Permalink
Merge pull request #794 from microsoft/wildcardPreserveSig
Browse files Browse the repository at this point in the history 
Allow PreserveSig to apply to all APIs
  • Loading branch information
@AArnott
AArnott committed Nov 19, 2022
2 parents 54b8abf + b462731 commit 07fe12c
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 14 deletions.
23 changes: 13 additions & 10 deletions src/Microsoft.Windows.CsWin32/Generator.Com.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ private static Guid DecodeGuidFromAttribute(CustomAttribute guidAttribute)
private TypeDeclarationSyntax DeclareInterfaceAsStruct(TypeDefinitionHandle typeDefHandle, ImmutableStack<QualifiedTypeDefinitionHandle> baseTypes, Context context)
{
TypeDefinition typeDef = this.Reader.GetTypeDefinition(typeDefHandle);
IdentifierNameSyntax ifaceName = IdentifierName(this.GetMangledIdentifier(this.Reader.GetString(typeDef.Name), context.AllowMarshaling, isManagedType: true));
string originalIfaceName = this.Reader.GetString(typeDef.Name);
IdentifierNameSyntax ifaceName = IdentifierName(this.GetMangledIdentifier(originalIfaceName, context.AllowMarshaling, isManagedType: true));
IdentifierNameSyntax vtblFieldName = IdentifierName("lpVtbl");
var members = new List<MemberDeclarationSyntax>();
var vtblMembers = new List<MemberDeclarationSyntax>();
Expand All @@ -107,6 +108,7 @@ private TypeDeclarationSyntax DeclareInterfaceAsStruct(TypeDefinitionHandle type
HashSet<string> helperMethodsInStruct = new();
ISet<string> declaredProperties = this.GetDeclarableProperties(
allMethods.Select(qh => qh.Reader.GetMethodDefinition(qh.MethodHandle)),
originalIfaceName,
allowNonConsecutiveAccessors: true);
foreach (QualifiedMethodDefinitionHandle methodDefHandle in allMethods)
{
Expand Down Expand Up @@ -154,7 +156,7 @@ private TypeDeclarationSyntax DeclareInterfaceAsStruct(TypeDefinitionHandle type

// We can declare this method as a property accessor if it represents a property.
// We must also confirm that the property type is the same in both cases, because sometimes they aren't (e.g. IUIAutomationProxyFactoryEntry.ClassName).
if (this.TryGetPropertyAccessorInfo(methodDefinition.Method, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType) &&
if (this.TryGetPropertyAccessorInfo(methodDefinition.Method, originalIfaceName, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType) &&
declaredProperties.Contains(propertyName.Identifier.ValueText))
{
StatementSyntax ThrowOnHRFailure(ExpressionSyntax hrExpression) => ExpressionStatement(InvocationExpression(
Expand Down Expand Up @@ -425,7 +427,7 @@ private TypeDeclarationSyntax DeclareInterfaceAsStruct(TypeDefinitionHandle type

var members = new List<MemberDeclarationSyntax>();
var friendlyOverloads = new List<MethodDeclarationSyntax>();
ISet<string> declaredProperties = this.GetDeclarableProperties(allMethods.Select(this.Reader.GetMethodDefinition), allowNonConsecutiveAccessors: false);
ISet<string> declaredProperties = this.GetDeclarableProperties(allMethods.Select(this.Reader.GetMethodDefinition), actualIfaceName, allowNonConsecutiveAccessors: false);

foreach (MethodDefinitionHandle methodDefHandle in allMethods)
{
Expand All @@ -441,7 +443,7 @@ private TypeDeclarationSyntax DeclareInterfaceAsStruct(TypeDefinitionHandle type
// Even if it could be represented as a property accessor, we cannot do so if a property by the same name was already declared in anything other than the previous row.
// Adding an accessor to a property later than the very next row would screw up the virtual method table ordering.
// We must also confirm that the property type is the same in both cases, because sometimes they aren't (e.g. IUIAutomationProxyFactoryEntry.ClassName).
if (this.TryGetPropertyAccessorInfo(methodDefinition, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType) && declaredProperties.Contains(propertyName.Identifier.ValueText))
if (this.TryGetPropertyAccessorInfo(methodDefinition, actualIfaceName, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType) && declaredProperties.Contains(propertyName.Identifier.ValueText))
{
AccessorDeclarationSyntax accessor = AccessorDeclaration(accessorKind.Value).WithSemicolonToken(Semicolon);

Expand Down Expand Up @@ -659,21 +661,22 @@ private bool UsePreserveSigForComMethod(MethodDefinition methodDefinition, Metho
{
return !IsHresult(signature.ReturnType)
|| (methodDefinition.ImplAttributes & MethodImplAttributes.PreserveSig) == MethodImplAttributes.PreserveSig
|| this.options.ComInterop.PreserveSigMethods.Contains("*")
|| this.FindInteropDecorativeAttribute(methodDefinition.GetCustomAttributes(), CanReturnMultipleSuccessValuesAttribute) is not null
|| this.FindInteropDecorativeAttribute(methodDefinition.GetCustomAttributes(), CanReturnErrorsAsSuccessAttribute) is not null
|| this.options.ComInterop.PreserveSigMethods.Contains($"{ifaceName}.{methodName}")
|| this.options.ComInterop.PreserveSigMethods.Contains(ifaceName.ToString());
}

private ISet<string> GetDeclarableProperties(IEnumerable<MethodDefinition> methods, bool allowNonConsecutiveAccessors)
private ISet<string> GetDeclarableProperties(IEnumerable<MethodDefinition> methods, string ifaceName, bool allowNonConsecutiveAccessors)
{
Dictionary<string, (TypeSyntax Type, int Index)> goodProperties = new(StringComparer.Ordinal);
HashSet<string> badProperties = new(StringComparer.Ordinal);
int rowIndex = -1;
foreach (MethodDefinition methodDefinition in methods)
{
rowIndex++;
if (this.TryGetPropertyAccessorInfo(methodDefinition, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType))
if (this.TryGetPropertyAccessorInfo(methodDefinition, ifaceName, out IdentifierNameSyntax? propertyName, out SyntaxKind? accessorKind, out TypeSyntax? propertyType))
{
if (badProperties.Contains(propertyName.Identifier.ValueText))
{
Expand Down Expand Up @@ -709,7 +712,7 @@ void ReportBadProperty()
return goodProperties.Count == 0 ? ImmutableHashSet<string>.Empty : new HashSet<string>(goodProperties.Keys, StringComparer.Ordinal);
}

private bool TryGetPropertyAccessorInfo(MethodDefinition methodDefinition, [NotNullWhen(true)] out IdentifierNameSyntax? propertyName, [NotNullWhen(true)] out SyntaxKind? accessorKind, [NotNullWhen(true)] out TypeSyntax? propertyType)
private bool TryGetPropertyAccessorInfo(MethodDefinition methodDefinition, string ifaceName, [NotNullWhen(true)] out IdentifierNameSyntax? propertyName, [NotNullWhen(true)] out SyntaxKind? accessorKind, [NotNullWhen(true)] out TypeSyntax? propertyType)
{
propertyName = null;
accessorKind = null;
Expand All @@ -729,7 +732,9 @@ private bool TryGetPropertyAccessorInfo(MethodDefinition methodDefinition, [NotN
return false;
}

if ((methodDefinition.ImplAttributes & MethodImplAttributes.PreserveSig) == MethodImplAttributes.PreserveSig)
MethodSignature<TypeHandleInfo> signature = methodDefinition.DecodeSignature(SignatureHandleProvider.Instance, null);
string methodName = this.Reader.GetString(methodDefinition.Name);
if (this.UsePreserveSigForComMethod(methodDefinition, signature, ifaceName, methodName))
{
return false;
}
Expand All @@ -740,15 +745,13 @@ private bool TryGetPropertyAccessorInfo(MethodDefinition methodDefinition, [NotN
return false;
}

string methodName = this.Reader.GetString(methodDefinition.Name);
const string getterPrefix = "get_";
const string setterPrefix = "put_";
bool isGetter = methodName.StartsWith(getterPrefix, StringComparison.Ordinal);
bool isSetter = methodName.StartsWith(setterPrefix, StringComparison.Ordinal);

if (isGetter || isSetter)
{
MethodSignature<TypeHandleInfo> signature = methodDefinition.DecodeSignature(SignatureHandleProvider.Instance, null);
if (!IsHresult(signature.ReturnType))
{
return false;
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Windows.CsWin32/settings.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
"type": "object",
"properties": {
"preserveSigMethods": {
"description": "Identifies methods or interfaces that should be generated as [PreserveSig].",
"description": "Identifies methods or interfaces that should be generated as [PreserveSig]. Use * to always generate PreserveSig methods.",
"type": "array",
"items": {
"type": "string",
"uniqueItems": true,
"pattern": "^[\\w_]+(?:\\.[\\w_]+)?$"
"pattern": "^[\\w_]+(?:\\.[\\w_]+)?$|^\\*$"
}
},
"useIntPtrForComOutPointers": {
Expand Down
6 changes: 5 additions & 1 deletion test/GenerationSandbox.Unmarshalled.Tests/GeneratedForm.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#pragma warning disable CA1812 // dead code

using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.System.Com;
using Windows.Win32.System.Com.Events;
Expand All @@ -18,8 +19,11 @@ private static unsafe void COMStructsPreserveSig()

// Default is non-preservesig
VARIANT v = o.GetPublisherProperty(null);
BSTR bstr = o.MethodName;

// NativeMethods.json opts into PreserveSig for this particular method.
// NativeMethods.json opts into PreserveSig for these particular methods.
HRESULT hr = o.GetSubscriberProperty(null, out v);
hr = o.get_MachineName(out SysFreeStringSafeHandle sh);
o.MachineName = bstr;
}
}
3 changes: 2 additions & 1 deletion test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"allowMarshaling": false,
"comInterop": {
"preserveSigMethods": [
"IEventSubscription.GetSubscriberProperty"
"IEventSubscription.GetSubscriberProperty",
"IEventSubscription.get_MachineName"
]
}
}

0 comments on commit 07fe12c

Please sign in to comment.