From 32dba291b8340ce69d3e492965047c3d264e64e2 Mon Sep 17 00:00:00 2001 From: Rich Lander Date: Wed, 17 Jun 2026 23:28:48 -0700 Subject: [PATCH] Recover ref/out/in on generic-instance and ref-readonly call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A call on a constructed generic type resolves as a MemberReference with a TypeSpecification parent, which carries no parameter rows — so out/in were lost and rendered as ref (CS1620/CS1615), e.g. every `Dictionary<,>.TryGetValue(out v)` call. Recover them by resolving the MemberRef back to the underlying generic MethodDef (signature-blob match within the declaring TypeDef) and reading its parameter rows. Also fixes how by-ref kinds are classified and spelled: - `ref readonly` parameters (RequiresLocationAttribute) join `in` (IsReadOnlyAttribute) as readonly references — rendered without a mutable-ref keyword, valid for a readonly/rvalue argument. Previously a `ref readonly` ctor like `ReadOnlySpan(ref readonly T)` was spelled `ref this` on a readonly receiver (CS1605). - `out`/`ref` arguments now require a genuine assignable lvalue; a cast (unbox) is not one, so `out (T)x` (CS0206) falls back to the default spelling. `in` still accepts a value form. Validated with the harness defect-diff: vs main, REGRESSED (none), IMPROVED CS1620 x2 and CS1605 x1. (The four spurious CS1615 from internal-overload calls are a compile-check shell artifact addressed separately by importing internals.) Tests: ILInspector.Decompiler.Tests 427 pass (a generic-instance out/in call site and the RefKindBox fixture added). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ControlFlowGraphTests.cs | 24 +++++++ .../IrImporterTests.cs | 16 +++++ .../Pipeline/Ir/CSharpPrinter.cs | 27 +++++-- .../Pipeline/Ir/IrImporter.cs | 70 ++++++++++++++++++- 4 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/ILInspector.Decompiler.Tests/ControlFlowGraphTests.cs b/src/ILInspector.Decompiler.Tests/ControlFlowGraphTests.cs index 16c8584c..b0d19714 100644 --- a/src/ILInspector.Decompiler.Tests/ControlFlowGraphTests.cs +++ b/src/ILInspector.Decompiler.Tests/ControlFlowGraphTests.cs @@ -1246,6 +1246,30 @@ public static int RefKindCallSites(int a) InHelper(in r); return r + o; } + + // Calls on a constructed generic instance resolve as MemberReferences + // (TypeSpec parent) that carry no parameter rows, so the out/in keyword must + // be recovered from the underlying generic MethodDef, not the MemberRef. + public static int GenericRefKindCallSites(int a) + { + var box = new RefKindBox(); + box.TryGet(out int value); + box.Put(in a); + return value; + } +} + +public sealed class RefKindBox +{ + T _value = default!; + + public bool TryGet(out T value) + { + value = _value; + return true; + } + + public void Put(in T value) => _value = value; } public interface IJoinShape diff --git a/src/ILInspector.Decompiler.Tests/IrImporterTests.cs b/src/ILInspector.Decompiler.Tests/IrImporterTests.cs index 7053dfc9..b31afbaf 100644 --- a/src/ILInspector.Decompiler.Tests/IrImporterTests.cs +++ b/src/ILInspector.Decompiler.Tests/IrImporterTests.cs @@ -1045,6 +1045,22 @@ public void CallSite_RefKinds_PrintRefOutAndBareIn() Assert.DoesNotContain("InHelper(out", output); } + [Fact] + public void CallSite_RefKinds_RecoveredForGenericInstanceCalls() + { + // A call on a constructed generic type is a MemberRef (TypeSpec parent) + // with no parameter rows; the keyword is recovered from the underlying + // generic MethodDef. Without that, `out` would render as `ref` (CS1620). + using var source = MetadataSource.Open(typeof(CfgSampleClass).Assembly.Location); + string output = PrintWithPasses(typeof(CfgSampleClass).FullName!, nameof(CfgSampleClass.GenericRefKindCallSites), source); + + Assert.Contains("TryGet(out ", output); + // The `in` argument is passed without a keyword. + Assert.Contains("Put(", output); + Assert.DoesNotContain("Put(ref", output); + Assert.DoesNotContain("Put(out", output); + } + [Fact] public void BooleanMaterialization_SelectWithIntLiteral_DeclaresBoolSlot() { diff --git a/src/ILInspector.Decompiler/Pipeline/Ir/CSharpPrinter.cs b/src/ILInspector.Decompiler/Pipeline/Ir/CSharpPrinter.cs index 48103943..775aa756 100644 --- a/src/ILInspector.Decompiler/Pipeline/Ir/CSharpPrinter.cs +++ b/src/ILInspector.Decompiler/Pipeline/Ir/CSharpPrinter.cs @@ -1147,14 +1147,15 @@ string Arguments(IEnumerable arguments, IReadOnlyList par { if (parameter is not { Kind: TypeRefKind.ByRef } || refKind == ArgumentRefKind.Value) return null; - if (ArgumentPlace(argument) is not { } place) + // `in` accepts a value argument (the compiler introduces a temporary), so + // any place- or value-spelling works and the keyword stays implicit. + if (refKind == ArgumentRefKind.In) + return ArgumentPlace(argument); + // `out`/`ref` require a genuine assignable lvalue; a cast (unbox) is not + // one (`out (T)x` is CS0206), so leave those to the default spelling. + if (ArgumentLvalue(argument) is not { } place) return null; - return refKind switch - { - ArgumentRefKind.Out => $"out {place}", - ArgumentRefKind.In => place, - _ => $"ref {place}", - }; + return refKind == ArgumentRefKind.Out ? $"out {place}" : $"ref {place}"; } /// @@ -1172,6 +1173,18 @@ string Arguments(IEnumerable arguments, IReadOnlyList par _ => null, }; + /// + /// The subset of that is a genuine assignable + /// lvalue — what out/ref demand. Excludes the + /// cast form (an lvalue only `in` can accept, as a value). + /// + string? ArgumentLvalue(IrExpression argument) => argument switch + { + LoadLocalAddress or LoadArgumentAddress or LoadFieldAddress or LoadElementAddress => Deref(argument), + LoadLocal or LoadArgument or LoadIndirect or Call or CallIndirect => Expression(argument), + _ => null, + }; + /// /// target?.Member: the member's receiver child is the target, and the /// member's name/arguments form the suffix after ?. Mirrors the diff --git a/src/ILInspector.Decompiler/Pipeline/Ir/IrImporter.cs b/src/ILInspector.Decompiler/Pipeline/Ir/IrImporter.cs index b0781ad7..4f6e0b48 100644 --- a/src/ILInspector.Decompiler/Pipeline/Ir/IrImporter.cs +++ b/src/ILInspector.Decompiler/Pipeline/Ir/IrImporter.cs @@ -1243,6 +1243,10 @@ [.. signature.ParameterTypes.Select(p => p.Instantiate(typeArguments, []))], || memberName.StartsWith("set_", StringComparison.Ordinal) || memberName.StartsWith("op_", StringComparison.Ordinal) || memberName is ".ctor" or ".cctor", + // A same-assembly call on a generic type instance is a + // MemberRef (TypeSpec parent), so its ref/out/in would + // otherwise be lost; recover it from the underlying MethodDef. + ParameterRefKinds = MemberReferenceRefKinds(reader, member, memberName, signature.ParameterTypes), }; } case HandleKind.MethodSpecification: @@ -1296,9 +1300,69 @@ static ImmutableArray ReadParameterRefKinds(MetadataReader read return ImmutableArray.Create(kinds); } + /// + /// Recovers ref/out/in for a callee referenced as a MemberReference by + /// resolving it back to its MethodDef parameter rows — the case that matters + /// is a same-assembly call on a generic type instance (TypeSpec parent), + /// where the keyword would otherwise be dropped. Returns empty (the printer + /// keeps its default spelling) when the declaring type is not a same-assembly + /// definition or no signature-matching method is found. + /// + static ImmutableArray MemberReferenceRefKinds(MetadataReader reader, MemberReference member, string memberName, ImmutableArray parameterTypes) + { + bool anyByRef = false; + foreach (var p in parameterTypes) + if (p.Kind == TypeRefKind.ByRef) { anyByRef = true; break; } + if (!anyByRef || DeclaringTypeDefinition(reader, member.Parent) is not { } typeHandle) + return []; + + var memberSignature = reader.GetBlobBytes(member.Signature); + foreach (var methodHandle in reader.GetTypeDefinition(typeHandle).GetMethods()) + { + var method = reader.GetMethodDefinition(methodHandle); + if (reader.GetString(method.Name) != memberName) + continue; + // A MemberRef's signature is encoded in the same generic-definition + // terms (!0/!1) as the MethodDef's, so the blobs match byte-for-byte + // for the referenced method — a robust overload-safe match. + if (reader.GetBlobBytes(method.Signature).AsSpan().SequenceEqual(memberSignature)) + return ReadParameterRefKinds(reader, method, parameterTypes); + } + return []; + } + + /// + /// The same-assembly a MemberRef's parent + /// names: the parent directly, or the generic type definition behind a + /// GENERICINST TypeSpecification. Null for a TypeReference (the type is + /// in another assembly, so its parameter rows are not available here). + /// + static TypeDefinitionHandle? DeclaringTypeDefinition(MetadataReader reader, EntityHandle parent) + { + switch (parent.Kind) + { + case HandleKind.TypeDefinition: + return (TypeDefinitionHandle)parent; + case HandleKind.TypeSpecification: + var blob = reader.GetBlobReader(reader.GetTypeSpecification((TypeSpecificationHandle)parent).Signature); + if (blob.ReadByte() != (byte)SignatureTypeCode.GenericTypeInstance) + return null; + blob.ReadByte(); // ELEMENT_TYPE_CLASS / ELEMENT_TYPE_VALUETYPE + var underlying = blob.ReadTypeHandle(); + return underlying.Kind == HandleKind.TypeDefinition ? (TypeDefinitionHandle)underlying : null; + default: + return null; + } + } + static ArgumentRefKind ClassifyByRefParameter(MetadataReader reader, System.Reflection.Metadata.Parameter parameter) { - if (HasReadOnlyAttribute(reader, parameter.GetCustomAttributes())) + // `in` (IsReadOnlyAttribute) and `ref readonly` (RequiresLocationAttribute) + // both take a readonly reference; the call site spells them without a + // mutable-ref keyword (treated as In — rendered bare), which is valid for + // a readonly or rvalue argument. Both also set the raw In flag (as do + // interop-marshalled `ref` params), so the flag cannot detect them. + if (HasReadOnlyRefAttribute(reader, parameter.GetCustomAttributes())) return ArgumentRefKind.In; var attributes = parameter.Attributes; if ((attributes & System.Reflection.ParameterAttributes.Out) != 0 @@ -1309,11 +1373,11 @@ static ArgumentRefKind ClassifyByRefParameter(MetadataReader reader, System.Refl // Pure-SRM attribute presence check (the decompiler Pipeline stays SRM-only, // so it does not reach into the Metadata AttributeReader). - static bool HasReadOnlyAttribute(MetadataReader reader, CustomAttributeHandleCollection attributes) + static bool HasReadOnlyRefAttribute(MetadataReader reader, CustomAttributeHandleCollection attributes) { foreach (var handle in attributes) if (AttributeTypeName(reader, reader.GetCustomAttribute(handle).Constructor) - is ("System.Runtime.CompilerServices", "IsReadOnlyAttribute")) + is ("System.Runtime.CompilerServices", "IsReadOnlyAttribute" or "RequiresLocationAttribute")) return true; return false; }