Differentiate MethodTables that are/aren't visible to reflection
#112782
Conversation
There was a problem hiding this comment.
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs:67
- Confirm that using the 'Marked' property on the constructed type symbol is the correct indicator to decide whether to skip emitting the object node for reflection-invisible generic definitions. An incorrect check here may result in the wrong emission state.
return factory.ConstructedTypeSymbol(_type).Marked;
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/EETypeNode.cs:1138
- Ensure that comparing 'this' with factory.MaximallyConstructableType(_type) reliably distinguishes between using a constructed versus necessary type symbol. An unexpected outcome here could lead to incorrect dependency emission.
IEETypeNode typeDefNode = factory.MaximallyConstructableType(_type) == this ?
| @@ -591,7 +591,7 @@ private IEETypeNode CreateNecessaryTypeNode(TypeDesc type) | |||
| { | |||
| if (type.IsGenericDefinition) | |||
| { | |||
| return new GenericDefinitionEETypeNode(this, type); | |||
| return new ReflectionInvisibleGenericDefinitionEETypeNode(this, type); | |||
There was a problem hiding this comment.
Review whether treating all generic definitions as reflection invisible on the necessary type path is intentional. This change could inadvertently hide types intended for reflection, so ensure it aligns with downstream requirements.
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ConstructedEETypeNode.cs
Show resolved
Hide resolved
...clr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/GenericDefinitionEETypeNode.cs
Show resolved
Hide resolved
MichalStrehovsky
force-pushed
the
fix112029
branch
from
2c40921 to
a73abd2
Compare
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #112029