From 2ceeb07b89848610002db7ac44b3e013ddcf144d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 04:55:29 +0000 Subject: [PATCH 1/2] Spell by-ref parameters from compiler attributes, not the interop In flag The raw In/Out parameter flags are interop marshalling directions, not C# modifiers. Both Metadata declaration paths spelled any In-flagged by-ref parameter as `in`, so the runtime's FindConnectionPoint([In] ref Guid riid) rendered as `in System.Guid riid`, a readonly reference callers cannot assign through, and RTS delegate and virtual-method shells gained a false modreq(InAttribute). ParameterByRefKinds (MetadataPrimitives) is now the one classification: IsReadOnlyAttribute -> in, RequiresLocationAttribute -> ref readonly, Out without In -> out, otherwise ref. MetadataDeclarationQuery and ApiSurfaceExtractor both use it. Fixes #3537. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016JAYbAeLyaZdJSXjXNjGCP --- .../ApiSurfaceExtractor.MethodSignatures.cs | 12 +-- .../MetadataDeclarationQuery.cs | 9 +-- .../ParameterByRefKind.cs | 71 +++++++++++++++++ .../CSharpDeclarationWriterTests.cs | 4 +- .../ApiSignatureModelTests.cs | 24 ++++++ .../MetadataDeclarationQueryTests.cs | 76 +++++++++++++++++++ 6 files changed, 177 insertions(+), 19 deletions(-) create mode 100644 src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs diff --git a/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs b/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs index a68a5d5256..9926c4afe3 100644 --- a/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs +++ b/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs @@ -444,16 +444,8 @@ private static (string? name, bool isParams, string? refKind, bool hasDefault, o handle, beforeRetain: beforeRetain, beforeMaterialize: beforeMaterialize); - // An interop-marshalled `ref` parameter sets both In and Out, so - // neither flag alone identifies a C# `out`/`in`. Spelling such a - // parameter `out` breaks definite assignment in the body. - bool isOut = (param.Attributes & System.Reflection.ParameterAttributes.Out) != 0; - bool isIn = (param.Attributes & System.Reflection.ParameterAttributes.In) != 0; - string? refKind = isOut && !isIn - ? "out" - : isIn && !isOut - ? "in" - : null; + // Applied only when the signature type is by-reference. + string? refKind = ParameterByRefKinds.Classify(reader, param, beforeMaterialize).CSharpModifier(); bool hasDefault = (param.Attributes & System.Reflection.ParameterAttributes.HasDefault) != 0; object? defaultValue = null; diff --git a/src/ILInspector.Metadata/MetadataDeclarationQuery.cs b/src/ILInspector.Metadata/MetadataDeclarationQuery.cs index 45c9c81120..a9c5c5eb40 100644 --- a/src/ILInspector.Metadata/MetadataDeclarationQuery.cs +++ b/src/ILInspector.Metadata/MetadataDeclarationQuery.cs @@ -858,13 +858,8 @@ static ParameterInfo GetParameterInfo(MetadataReader reader, ParameterHandleColl var attributes = parameter.GetCustomAttributes(); var isParams = AttributeReader.HasAttribute(reader, attributes, "System.ParamArrayAttribute") || AttributeReader.HasAttribute(reader, attributes, KnownAttributeNames.ParamCollectionAttribute); - var isOut = (parameter.Attributes & ParameterAttributes.Out) != 0; - var isIn = (parameter.Attributes & ParameterAttributes.In) != 0; - var refKind = isOut && !isIn - ? "out" - : isIn && !isOut - ? "in" - : null; + // Applied only when the signature type is by-reference. + var refKind = ParameterByRefKinds.Classify(reader, parameter).CSharpModifier(); var defaultParameter = (parameter.Attributes & ParameterAttributes.HasDefault) != 0 ? parameter : (Parameter?)null; diff --git a/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs b/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs new file mode 100644 index 0000000000..0097847176 --- /dev/null +++ b/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs @@ -0,0 +1,71 @@ +using System.Reflection; +using System.Reflection.Metadata; + +namespace ILInspector.Metadata; + +/// The C# passing mode of a parameter whose signature type is by-reference. +public enum ParameterByRefKind +{ + /// ref: a mutable reference, including interop [In] ref and [In, Out] ref. + Ref, + + /// out: the Out flag without In. + Out, + + /// in: marked with IsReadOnlyAttribute. + In, + + /// ref readonly: marked with RequiresLocationAttribute. + RefReadOnly, +} + +/// +/// The one classification of a by-reference parameter's C# passing mode from metadata. +/// The raw In and Out parameter flags are interop marshalling directions, +/// not C# modifiers: [In] ref and [In, Out] ref are mutable ref +/// parameters. C# in and ref readonly are identified only by their +/// compiler attributes (both compilers also set the In flag). +/// +public static class ParameterByRefKinds +{ + const string IsReadOnlyAttribute = "System.Runtime.CompilerServices.IsReadOnlyAttribute"; + const string RequiresLocationAttribute = "System.Runtime.CompilerServices.RequiresLocationAttribute"; + + /// + /// Classifies , whose signature type the caller has + /// already established is by-reference. + /// + public static ParameterByRefKind Classify( + MetadataReader reader, + Parameter parameter, + Action? beforeMaterialize = null) + { + foreach (var handle in parameter.GetCustomAttributes()) + { + string? name = AttributeDecoder.GetAttributeTypeName( + reader, + reader.GetCustomAttribute(handle).Constructor, + beforeMaterialize); + if (name == IsReadOnlyAttribute) + return ParameterByRefKind.In; + if (name == RequiresLocationAttribute) + return ParameterByRefKind.RefReadOnly; + } + + var attributes = parameter.Attributes; + return (attributes & ParameterAttributes.Out) != 0 + && (attributes & ParameterAttributes.In) == 0 + ? ParameterByRefKind.Out + : ParameterByRefKind.Ref; + } + + /// The C# declaration modifier for . + public static string CSharpModifier(this ParameterByRefKind kind) + => kind switch + { + ParameterByRefKind.Out => "out", + ParameterByRefKind.In => "in", + ParameterByRefKind.RefReadOnly => "ref readonly", + _ => "ref", + }; +} diff --git a/tests/ILInspector.CSharp.Tests/CSharpDeclarationWriterTests.cs b/tests/ILInspector.CSharp.Tests/CSharpDeclarationWriterTests.cs index 7d2af9273d..adb101602e 100644 --- a/tests/ILInspector.CSharp.Tests/CSharpDeclarationWriterTests.cs +++ b/tests/ILInspector.CSharp.Tests/CSharpDeclarationWriterTests.cs @@ -1334,7 +1334,7 @@ public void AbbreviatedMemberDeclaration_PreservesParameterModifiers() { Name = "M", Kind = "method", - Signature = "void M(ref int value, out string text, in long source, params byte[] bytes)" + Signature = "void M(ref int value, out string text, in long source, ref readonly System.Guid id, params byte[] bytes)" }; var declaration = CSharpDeclarationWriter.RenderMemberDeclaration( @@ -1342,7 +1342,7 @@ public void AbbreviatedMemberDeclaration_PreservesParameterModifiers() member, new CSharpDeclarationOptions { AbbreviateSignature = true }); - Assert.Equal("public void M(ref int, out string, in long, params byte[])", declaration); + Assert.Equal("public void M(ref int, out string, in long, ref readonly System.Guid, params byte[])", declaration); } [Fact] diff --git a/tests/ILInspector.Metadata.Tests/ApiSignatureModelTests.cs b/tests/ILInspector.Metadata.Tests/ApiSignatureModelTests.cs index 0ee6382072..44f6a40bd9 100644 --- a/tests/ILInspector.Metadata.Tests/ApiSignatureModelTests.cs +++ b/tests/ILInspector.Metadata.Tests/ApiSignatureModelTests.cs @@ -370,6 +370,22 @@ public void MethodSignatureModel_SuppressesCompilerReservedRefReadonlyParameterA Assert.Empty(member.SignatureModel.Parameters[0].Attributes); } + /// + /// The API surface spells by-ref passing from compiler attributes, not the raw + /// interop In flag (#3537), so [In] ref stays a mutable ref. + /// + [Fact] + public void MethodSignatureModel_SpellsByRefParameterPassingFromCompilerAttributes() + { + var member = GetMember(nameof(ApiSignatureFixtures), nameof(ApiSignatureFixtures.MethodWithByRefPassing)); + + Assert.NotNull(member.SignatureModel); + Assert.Equal( + ["ref", "ref", "ref", "out", "in", "ref readonly"], + member.SignatureModel.Parameters.Select(parameter => parameter.Modifier)); + Assert.Contains("ref int interopIn, ref int interopInOut, out int result, in int readOnly, ref readonly int location", member.Signature); + } + [Fact] public void MethodSignatureModel_EscapesCharArguments() { @@ -836,6 +852,14 @@ public void MethodWithRefReadonlyParameter(ref readonly int value) { } + public void MethodWithByRefPassing( + ref int plain, + [System.Runtime.InteropServices.In] ref int interopIn, + [System.Runtime.InteropServices.In, System.Runtime.InteropServices.Out] ref int interopInOut, + out int result, + in int readOnly, + ref readonly int location) => result = plain + interopIn + interopInOut + readOnly + location; + public void MethodWithQuoteCharAttribute([ParameterCharMarker('\'')] string value) { } diff --git a/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs b/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs index 842fd24daf..861d4eed96 100644 --- a/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs +++ b/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs @@ -41,6 +41,74 @@ public void MethodDeclaration_ExposesAccessibilityModifiersParametersAndReturnAt Assert.Equal("2", declaration.Signature.Parameters[1].DefaultValueText); } + /// + /// The raw In/Out parameter flags are interop marshalling directions: + /// [In] ref and [In, Out] ref are mutable ref parameters (#3537). + /// C# in and ref readonly are identified by their compiler attributes. + /// + [Theory] + [InlineData(0, "ref")] + [InlineData(1, "ref")] + [InlineData(2, "ref")] + [InlineData(3, "out")] + [InlineData(4, "in")] + [InlineData(5, "ref readonly")] + public void MethodDeclaration_SpellsByRefParameterPassingFromCompilerAttributes(int index, string modifier) + { + var type = GetTypeDefinition(typeof(MetadataDeclarationQueryFixtures)); + var method = GetMethod(type, nameof(MetadataDeclarationQueryFixtures.ByRefPassing)); + + var declaration = MetadataDeclarationQuery.GetMethod(Reader, type, method); + + Assert.Equal(modifier, declaration.Signature.Parameters[index].Modifier); + Assert.Equal("int", declaration.Signature.Parameters[index].Type); + } + + /// + /// The motivating real asset (#3537): the runtime declares + /// FindConnectionPoint([In] ref Guid riid, out IConnectionPoint ppCP), which was + /// rendered as in System.Guid riid, a readonly reference callers cannot assign through. + /// + [Fact] + public void MethodDeclaration_SpellsCoreLibInteropInRefParameterAsRef() + { + using var peReader = new PEReader(File.OpenRead( + typeof(System.Runtime.InteropServices.ComTypes.IConnectionPointContainer).Assembly.Location)); + var reader = peReader.GetMetadataReader(); + var (type, method) = FindMethod( + reader, + "System.Runtime.InteropServices.ComTypes", + "IConnectionPointContainer", + "FindConnectionPoint"); + + var declaration = MetadataDeclarationQuery.GetMethod(reader, type, method); + + Assert.Equal("ref", declaration.Signature.Parameters[0].Modifier); + Assert.Equal("out", declaration.Signature.Parameters[1].Modifier); + } + + static (TypeDefinition Type, MethodDefinition Method) FindMethod( + MetadataReader reader, + string @namespace, + string typeName, + string methodName) + { + foreach (var typeHandle in reader.TypeDefinitions) + { + var type = reader.GetTypeDefinition(typeHandle); + if (reader.GetString(type.Namespace) != @namespace || reader.GetString(type.Name) != typeName) + continue; + foreach (var methodHandle in type.GetMethods()) + { + var method = reader.GetMethodDefinition(methodHandle); + if (reader.GetString(method.Name) == methodName) + return (type, method); + } + } + + throw new InvalidOperationException($"Method '{@namespace}.{typeName}.{methodName}' was not found."); + } + [Fact] public void InitOnlySetter_PreservesReturnModifierDistinction() { @@ -615,6 +683,14 @@ public class MetadataDeclarationQueryFixtures public decimal DecimalDefault(decimal amount = 5m) => amount; + public void ByRefPassing( + ref int plain, + [In] ref int interopIn, + [In, Out] ref int interopInOut, + out int result, + in int readOnly, + ref readonly int location) => result = plain + interopIn + interopInOut + readOnly + location; + public int Count() => _count; public int @class() => 0; From 801e3de7e120a8080316a3ff74946a0d742df3a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:50:09 +0000 Subject: [PATCH 2/2] Follow Roslyn precedence for out-only by-ref parameters; exclude ref readonly union case constructors Round 1 review of #10033: - ParameterByRefKinds.Classify now reads Out without In as out before consulting IsReadOnlyAttribute or RequiresLocationAttribute, matching Roslyn's metadata reader. A patched-flag theory covers both attributes. - JsonUnionWireRules.CaseConstructors excludes "ref readonly" alongside ref, in and out, so a [Union] convention type with a ref readonly constructor keeps its by-value case set. - Caller comments now say the classification runs for every parameter row and is consumed only for by-ref signature types. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016JAYbAeLyaZdJSXjXNjGCP --- .../JsonUnionWireRules.cs | 2 +- .../ApiSurfaceExtractor.MethodSignatures.cs | 2 +- .../MetadataDeclarationQuery.cs | 2 +- .../ParameterByRefKind.cs | 20 ++++++---- .../JsonUnionWireTests.cs | 20 ++++++++++ .../MetadataDeclarationQueryTests.cs | 39 +++++++++++++++++++ 6 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/ILInspector.JsExportSurface/JsonUnionWireRules.cs b/src/ILInspector.JsExportSurface/JsonUnionWireRules.cs index 57519d2231..fb62627ba4 100644 --- a/src/ILInspector.JsExportSurface/JsonUnionWireRules.cs +++ b/src/ILInspector.JsExportSurface/JsonUnionWireRules.cs @@ -11,7 +11,7 @@ internal static IEnumerable CaseConstructors(ApiType type) => && !member.IsStatic && member.Accessibility is null or "public" && member.SignatureModel is { Parameters.Count: 1 } signature - && signature.Parameters[0].Modifier is not ("ref" or "in" or "out")); + && signature.Parameters[0].Modifier is not ("ref" or "in" or "out" or "ref readonly")); internal static JsExportUnion Describe( ApiType type, diff --git a/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs b/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs index 9926c4afe3..626e882591 100644 --- a/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs +++ b/src/ILInspector.Metadata/ApiSurfaceExtractor.MethodSignatures.cs @@ -444,7 +444,7 @@ private static (string? name, bool isParams, string? refKind, bool hasDefault, o handle, beforeRetain: beforeRetain, beforeMaterialize: beforeMaterialize); - // Applied only when the signature type is by-reference. + // Classified for every parameter row; consumed only when the signature type is by-reference. string? refKind = ParameterByRefKinds.Classify(reader, param, beforeMaterialize).CSharpModifier(); bool hasDefault = (param.Attributes & System.Reflection.ParameterAttributes.HasDefault) != 0; diff --git a/src/ILInspector.Metadata/MetadataDeclarationQuery.cs b/src/ILInspector.Metadata/MetadataDeclarationQuery.cs index a9c5c5eb40..7c3c729daf 100644 --- a/src/ILInspector.Metadata/MetadataDeclarationQuery.cs +++ b/src/ILInspector.Metadata/MetadataDeclarationQuery.cs @@ -858,7 +858,7 @@ static ParameterInfo GetParameterInfo(MetadataReader reader, ParameterHandleColl var attributes = parameter.GetCustomAttributes(); var isParams = AttributeReader.HasAttribute(reader, attributes, "System.ParamArrayAttribute") || AttributeReader.HasAttribute(reader, attributes, KnownAttributeNames.ParamCollectionAttribute); - // Applied only when the signature type is by-reference. + // Classified for every parameter row; consumed only when the signature type is by-reference. var refKind = ParameterByRefKinds.Classify(reader, parameter).CSharpModifier(); var defaultParameter = (parameter.Attributes & ParameterAttributes.HasDefault) != 0 ? parameter diff --git a/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs b/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs index 0097847176..4aec14660b 100644 --- a/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs +++ b/src/ILInspector.MetadataPrimitives/ParameterByRefKind.cs @@ -26,6 +26,12 @@ public enum ParameterByRefKind /// parameters. C# in and ref readonly are identified only by their /// compiler attributes (both compilers also set the In flag). /// +/// +/// Precedence follows Roslyn's metadata reader: Out without In is +/// out before either attribute is consulted, then IsReadOnlyAttribute +/// is in, then RequiresLocationAttribute is ref readonly, and +/// everything else is ref. +/// public static class ParameterByRefKinds { const string IsReadOnlyAttribute = "System.Runtime.CompilerServices.IsReadOnlyAttribute"; @@ -40,6 +46,11 @@ public static ParameterByRefKind Classify( Parameter parameter, Action? beforeMaterialize = null) { + var flags = parameter.Attributes & (ParameterAttributes.In | ParameterAttributes.Out); + if (flags == ParameterAttributes.Out) + return ParameterByRefKind.Out; + + bool requiresLocation = false; foreach (var handle in parameter.GetCustomAttributes()) { string? name = AttributeDecoder.GetAttributeTypeName( @@ -48,15 +59,10 @@ public static ParameterByRefKind Classify( beforeMaterialize); if (name == IsReadOnlyAttribute) return ParameterByRefKind.In; - if (name == RequiresLocationAttribute) - return ParameterByRefKind.RefReadOnly; + requiresLocation |= name == RequiresLocationAttribute; } - var attributes = parameter.Attributes; - return (attributes & ParameterAttributes.Out) != 0 - && (attributes & ParameterAttributes.In) == 0 - ? ParameterByRefKind.Out - : ParameterByRefKind.Ref; + return requiresLocation ? ParameterByRefKind.RefReadOnly : ParameterByRefKind.Ref; } /// The C# declaration modifier for . diff --git a/tests/ILInspector.JsExportSurface.Tests/JsonUnionWireTests.cs b/tests/ILInspector.JsExportSurface.Tests/JsonUnionWireTests.cs index 22ff248ddd..5300b14014 100644 --- a/tests/ILInspector.JsExportSurface.Tests/JsonUnionWireTests.cs +++ b/tests/ILInspector.JsExportSurface.Tests/JsonUnionWireTests.cs @@ -167,6 +167,26 @@ public void Build_DoesNotDropAnUnreadableConstructorFromTheCaseSet() Assert.Null(union.IncludesNull); } + /// + /// Case constructors are by-value; every by-ref passing mode the API surface spells, + /// including ref readonly, stays out of the case set. + /// + [Theory] + [InlineData("ref")] + [InlineData("in")] + [InlineData("out")] + [InlineData("ref readonly")] + public void CaseConstructors_ExcludeEveryByRefPassingMode(string modifier) + { + ApiSurface api = Extract(nameof(UnionExports.GetScalar)); + ApiType type = Assert.Single(api.Types, type => type.Name == nameof(ScalarUnion)); + ApiMember[] constructors = JsonUnionWireRules.CaseConstructors(type).ToArray(); + ApiMember byRef = constructors[0]; + byRef.SignatureModel!.Parameters[0].Modifier = modifier; + + Assert.Equal(constructors[1..], JsonUnionWireRules.CaseConstructors(type)); + } + [Theory] [InlineData(nameof(UnionExports.GetOrdinary))] [InlineData(nameof(UnionExports.GetPlain))] diff --git a/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs b/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs index 861d4eed96..ab0417db59 100644 --- a/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs +++ b/tests/ILInspector.Metadata.Tests/MetadataDeclarationQueryTests.cs @@ -1,6 +1,7 @@ using System.Reflection; using System.Reflection.Emit; using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; using System.Reflection.PortableExecutable; using System.Runtime.InteropServices; using ILInspector.Metadata; @@ -64,6 +65,44 @@ public void MethodDeclaration_SpellsByRefParameterPassingFromCompilerAttributes( Assert.Equal("int", declaration.Signature.Parameters[index].Type); } + /// + /// Roslyn reads Out without In as out before it consults + /// IsReadOnlyAttribute or RequiresLocationAttribute. The compiler never + /// emits that combination, so the fixture's in and ref readonly rows are + /// patched to carry only the Out flag. + /// + [Theory] + [InlineData(4)] + [InlineData(5)] + public void MethodDeclaration_SpellsOutOnlyFlagAsOutBeforeCompilerAttributes(int index) + { + byte[] image = File.ReadAllBytes(typeof(MetadataDeclarationQueryTests).Assembly.Location); + var type = GetTypeDefinition(typeof(MetadataDeclarationQueryFixtures)); + var method = GetMethod(type, nameof(MetadataDeclarationQueryFixtures.ByRefPassing)); + var parameterHandle = method.GetParameters() + .Single(handle => Reader.GetParameter(handle).SequenceNumber == index + 1); + int row = MetadataTokens.GetRowNumber(parameterHandle); + int flagsOffset = PeReader.PEHeaders.MetadataStartOffset + + Reader.GetTableMetadataOffset(TableIndex.Param) + + (row - 1) * Reader.GetTableRowSize(TableIndex.Param); + System.Buffers.Binary.BinaryPrimitives.WriteUInt16LittleEndian( + image.AsSpan(flagsOffset), + (ushort)ParameterAttributes.Out); + + using var patched = new PEReader(new MemoryStream(image)); + var reader = patched.GetMetadataReader(); + var (patchedType, patchedMethod) = FindMethod( + reader, + typeof(MetadataDeclarationQueryFixtures).Namespace!, + nameof(MetadataDeclarationQueryFixtures), + nameof(MetadataDeclarationQueryFixtures.ByRefPassing)); + + var declaration = MetadataDeclarationQuery.GetMethod(reader, patchedType, patchedMethod); + + Assert.Equal("out", declaration.Signature.Parameters[index].Modifier); + Assert.Equal("ref", declaration.Signature.Parameters[0].Modifier); + } + /// /// The motivating real asset (#3537): the runtime declares /// FindConnectionPoint([In] ref Guid riid, out IConnectionPoint ppCP), which was