Simplify branching in ILImporter.Scanner.cs with Debug.Assert

[Copilot]

Description

Replace explicit else if (targetMethod.AcquiresInstMethodTableFromThis()) with else { Debug.Assert(...); } to tighten the invariant that shared generic methods acquire context via exactly one of three mutually exclusive paths: method dictionary, type dictionary, or this pointer.

Changes

File: src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs

In the directCall path within exactContextNeedsRuntimeLookup:

// Before
else if (targetMethod.AcquiresInstMethodTableFromThis())
{
    _dependencies.Add(_factory.ShadowNonConcreteMethod(concreteMethod), reason);
}

// After
else
{
    Debug.Assert(targetMethod.AcquiresInstMethodTableFromThis());
    _dependencies.Add(_factory.ShadowNonConcreteMethod(concreteMethod), reason);
}

Aligns with existing pattern in CorInfoImpl.GetGenericRuntimeLookupKind. No functional behavior change.

Testing

• ILCompiler.Compiler builds with no warnings or errors
• Code review passed
• Security checks passed

Original prompt

Goal: Simplify branching in ILImporter.Scanner.cs by replacing the explicit else if (targetMethod.AcquiresInstMethodTableFromThis()) with a plain else that begins with Debug.Assert(targetMethod.AcquiresInstMethodTableFromThis()). This matches the invariant that, in shared generic code, if a method does not require a method dictionary or a type (method table) dictionary argument, it must acquire the generic context from this.

Background:

• The logic for selecting how a method acquires generic context is a mutually exclusive triad:
  • RequiresInstMethodDescArg() for method dictionary
  • RequiresInstMethodTableArg() for type/method table dictionary
  • AcquiresInstMethodTableFromThis() for this pointer
• This exclusivity is documented in CoreCLR VM (MethodDesc comments) and mirrored in managed TypeExtensions. Other places in the codebase (e.g., CorInfoImpl.GetGenericRuntimeLookupKind) already use the else + Debug.Assert(...) pattern for this invariant.

Change scope:

• File: src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs
• Location: In the directCall path, inside the exactContextNeedsRuntimeLookup branch, under the condition if (targetMethod.IsSharedByGenericInstantiations && !resolvedConstraint && !referencingArrayAddressMethod).
• Replace:
  • else if (targetMethod.AcquiresInstMethodTableFromThis()) { _dependencies.Add(_factory.ShadowNonConcreteMethod(concreteMethod), reason); }
    with:
  • else { Debug.Assert(targetMethod.AcquiresInstMethodTableFromThis()); _dependencies.Add(_factory.ShadowNonConcreteMethod(concreteMethod), reason); }

Rationale:

• Tightens the invariant and aligns with existing patterns (e.g., CorInfoImpl.GetGenericRuntimeLookupKind).
• Improves readability by asserting the expected condition while avoiding redundant conditional branching.

Exact edit (before/after):

Before:

                else if (exactContextNeedsRuntimeLookup)
                {
                    if (targetMethod.IsSharedByGenericInstantiations && !resolvedConstraint && !referencingArrayAddressMethod)
                    {
                        ISymbolNode instParam = null;

                        if (!_canonMethod.IsSharedByGenericInstantiations)
                        {
                            // Some handemitted IL helpers will call __Canon methods directly from unshared context.
                            // This is fine, we just don't report any dependencies for the exact instantiation.
                        }
                        else if (targetMethod.RequiresInstMethodDescArg())
                        {
                            instParam = GetGenericLookupHelper(ReadyToRunHelperId.MethodDictionary, runtimeDeterminedMethod);
                        }
                        else if (targetMethod.RequiresInstMethodTableArg())
                        {
                            instParam = GetGenericLookupHelper(ReadyToRunHelperId.TypeHandle, runtimeDeterminedMethod.OwningType);
                        }
                        else if (targetMethod.AcquiresInstMethodTableFromThis())
                        {
                            _dependencies.Add(_factory.ShadowNonConcreteMethod(concreteMethod), reason);
                        }

                        if (instParam != null)
                        {
                            _dependencies.Add(instParam, reason);
                        }

                        _dependencies.Add(_factory.CanonicalEntrypoint(targetMethod), reason);
                    }
                    else
                    {
                        // ... unchanged code ...
                    }
                }

After:

                else if (exactContextNeedsRuntimeLookup)
                {
                    if (targetMethod.IsSharedByGenericInstantiations && !resolvedConstraint && !referencingArrayAddressMethod)
                    {
                        ISymbolNode instParam = null;

                        if (!_canonMethod.IsSharedByGenericInstantiations)
                        {
                            // Some handemitted IL helpers will call __Canon methods directly from unshared context.
                            // This is fine, we just don't report any dependencies for the exact instantiation.
                        }
                        else if (targetMethod.RequiresInstMethodDescArg())
                        {
                            instParam = GetGenericLookupHelper(ReadyToRunHelperId.MethodDictionary, runtimeDeterminedMethod);
                        }
           ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- START COPILOT CODING AGENT TIPS -->
---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).

[MichalStrehovsky]

@sbomer could you have a look please? I wasn't sure about this one in your PR so I didn't bring it up, but I just made copilot try it and it works.

[Copilot]

Pull request overview

This pull request simplifies conditional branching logic in the ILCompiler scanner by replacing an explicit else if (targetMethod.AcquiresInstMethodTableFromThis()) check with a plain else block containing a Debug.Assert. This tightens the invariant that shared generic methods acquire their generic context through exactly one of three mutually exclusive paths.

Changes:

• Replaced explicit conditional check with assertion to enforce mutually exclusive invariant for generic context acquisition methods
• Aligns code pattern with existing implementations in Compilation.cs and CorInfoImpl.RyuJit.cs

[sbomer]

Thanks, LGTM!