Simplify handling Span in the cctor interpreter#114487
Simplify handling Span in the cctor interpreter#114487MichalStrehovsky merged 2 commits intodotnet:mainfrom
Span in the cctor interpreter#114487Conversation
This is two things: * Now that we allow byrefs in more places, we can simplify handling spans (less special casing since more things just naturally fall out). * Handle RVA statics within the interpreter. They can be naturally used with Spans now.
ghost
added
the
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs:2810
- [nitpick] Consider extracting the hardcoded field name '_length' (and similarly '_reference') into a named constant to improve maintainability and reduce magic strings.
if (field.Name == "_length")
src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs:3226
- [nitpick] Verify that the new method name 'GetArrayData' clearly represents its purpose as a replacement for the previous TryGetReadOnlySpan approach, or consider renaming for better clarity.
public ByRefValue GetArrayData()
|
|
||
| Value fieldValue; | ||
| if (field.OwningType == _type) | ||
| if (field.HasRva) |
There was a problem hiding this comment.
Consider adding an inline comment to clarify the constraints on RVA statics (e.g. why fields must be IsInitOnly and without a static constructor) to aid future maintenance.
| public override bool TryLoad(TypeDesc type, out Value value) | ||
| { | ||
| if (!type.IsPrimitive | ||
| if (!type.IsValueType |
There was a problem hiding this comment.
Review whether switching the condition from 'IsPrimitive' to 'IsValueType' covers all intended scenarios without unintentionally allowing unsupported types.
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This is two things:
Fixes #114455.
Cc @dotnet/ilc-contrib @Sergio0694