Skip to content

Consolidate by-ref parameter classification on ParameterByRefKinds (Decompiler, then Analysis) #10164

Open

Description

@richlander

Summary

#10033 (fixes #3537) added ParameterByRefKinds.Classify in ILInspector.MetadataPrimitives. It is now the single rule for a by-ref parameter's C# passing mode, using Roslyn's precedence:

Metadata Mode
Out flag without In out
IsReadOnlyAttribute in
RequiresLocationAttribute ref readonly
otherwise ref

#3537 noted that this rule had four independent copies. Two Metadata declaration paths now use the shared classifier. Three copies remain:

Location State Difference from the shared rule
ILInspector.Decompiler MethodDefinitionFacts.ReadParameterRefKinds / ClassifyByRefParameter (live; used by IrImporter and CrossAssemblyTypeResolver) own copy checks RequiresLocationAttribute / IsReadOnlyAttribute before Out-only
ILInspector.Decompiler IrImporter.ReadParameterRefKinds / ClassifyByRefParameter / HasReadOnlyRefAttribute dead (no callers) same attribute-first order
ILInspector.Analysis MemberResolver (around lines 222-235) → ParameterDirection for ResourceEffectSelectorBinding own copy classifies the raw In-only flag as ParameterDirection.In (found in the #10033 round 2 review)

Consequence

  • Decompiler. A by-ref parameter whose row carries only the Out flag plus IsReadOnlyAttribute or RequiresLocationAttribute is spelled at the call site as a readonly (bare) argument. Roslyn reads that parameter as out. The compiler never emits this combination, but it is valid ECMA-335 metadata and can arrive from untrusted assemblies. Declaration spelling (after Spell by-ref parameters from compiler attributes, not the interop In flag #10033) and call-site spelling then disagree for the same parameter.
  • Analysis. An interop [In] ref parameter is treated as an In-direction parameter for selector matching, rather than a mutable ref.

Plan (one owner per slice)

  1. Decompiler: MethodDefinitionFacts.ReadParameterRefKinds adopts ParameterByRefKinds.Classify, mapping in and ref readonly to ArgumentRefKind.In, out to Out, and ref to Ref, and keeping HasRefReadOnlyParameters. Delete the dead IrImporter copy. Covered by:
    • tests on compiled passing modes;
    • patched Out-only rows;
    • the CoreLib FindConnectionPoint real asset.
  2. Analysis: MemberResolver's direction rule adopts the shared classifier. This needs its own look at what ParameterDirection.In means for selector consumers before it changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions