Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix constrained call corner cases #111178

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MichalStrehovsky
Copy link
Member

Fixes dotnet/runtimelab#1431.
Fixes #98582.
Fixes #98581.

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (5)
  • src/tests/issues.targets: Language not supported
  • src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/GenericDictionaryCell.cs: Evaluated as low risk
  • src/coreclr/tools/Common/Compiler/TypeExtensions.cs: Evaluated as low risk
  • src/coreclr/tools/aot/ILCompiler.Compiler/IL/ILImporter.Scanner.cs: Evaluated as low risk
  • src/coreclr/tools/Common/TypeSystem/Common/MetadataVirtualMethodAlgorithm.cs: Evaluated as low risk
Comments suppressed due to low confidence (2)

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NativeLayoutVertexNode.cs:1835

  • Ensure that the new cases for GenericInstanceConstrainedMethod and NonGenericInstanceConstrainedMethod are covered by tests.
return (_constrainedMethod.HasInstantiation, _constrainedMethod.Signature.IsStatic) switch

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericLookupResult.cs:866

  • Ensure that the new behavior for resolving non-static constrained methods is covered by tests.
implMethod = instantiatedConstraintType.GetClosestDefType().ResolveVariantInterfaceMethodToVirtualMethodOnType(instantiatedConstrainedMethod);
@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MichalStrehovsky
Copy link
Member Author

/azp run runtime-nativeaot-outerloop

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this pull request Jan 9, 2025
The problem was always `// Note: We don't set the IsUnboxingStub flag on template methods (all template lookups performed at runtime are performed with this flag not set`, I tried working around it in the original fix, but looks like I actually need the function pointer in dotnet#111178 (the tests are not failing but I have a local test that does).

So trying an alternative approach that just deletes the weird code. It's possible this only had to be weird due to universal shared code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant