Add type name to FieldDef and FieldRef #3200
Conversation
- Add element to structs. - Update check for structs size. - CLR_RT_Assembly::FindFieldDef now takes TypeSpec index parameter. - CLR_RT_Assembly::FindFieldDef is also checking for type name. - Update calleers accordingly.
josesimoes
added
the
Area: Interpreter
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes update the method signature of Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant CLR_RT_Assembly
Caller->>CLR_RT_Assembly: FindFieldDef(tsIndex, name, base, sig, index)
CLR_RT_Assembly-->>Caller: returns bool (found/not found)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/CLR/Include/nanoCLR_Runtime.h(1 hunks)src/CLR/Include/nanoCLR_Types.h(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-10-12T19:00:39.000Z
Learning: When working with `nanoCLR_GetNativeAssemblyInformation`, fixed-size assembly names are required, so code that deals with variable-length names cannot be used.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-09-25T11:28:38.536Z
Learning: When working with `nanoCLR_GetNativeAssemblyInformation`, fixed-size assembly names are required, so code that deals with variable-length names cannot be used.
src/CLR/Include/nanoCLR_Runtime.h (6)
undefined
<retrieved_learning>
Learnt from: josesimoes
PR: #3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-10-12T19:00:39.000Z
Learning: When working with nanoCLR_GetNativeAssemblyInformation, fixed-size assembly names are required, so code that deals with variable-length names cannot be used.
</retrieved_learning>
<retrieved_learning>
Learnt from: josesimoes
PR: #3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-09-25T11:28:38.536Z
Learning: When working with nanoCLR_GetNativeAssemblyInformation, fixed-size assembly names are required, so code that deals with variable-length names cannot be used.
</retrieved_learning>
<retrieved_learning>
Learnt from: josesimoes
PR: #3172
File: src/CLR/Core/CLR_RT_HeapBlock.cpp:899-900
Timestamp: 2025-05-14T16:27:02.573Z
Learning: The CLR_RT_TypeDescriptor type in nanoFramework doesn't have a GetElementType() API for extracting array element types.
</retrieved_learning>
<retrieved_learning>
Learnt from: josesimoes
PR: #3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-10-08T15:52:09.445Z
Learning: In nanoCLR_GetNativeAssemblyInformation, there is no need to return the number of bytes written, as the memory buffer is zeroed, making the string buffer terminated.
</retrieved_learning>
<retrieved_learning>
Learnt from: josesimoes
PR: #3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-09-25T11:28:38.536Z
Learning: In nanoCLR_GetNativeAssemblyInformation, there is no need to return the number of bytes written, as the memory buffer is zeroed, making the string buffer terminated.
</retrieved_learning>
<retrieved_learning>
Learnt from: josesimoes
PR: #3172
File: src/CLR/Core/TypeSystem.cpp:4556-4589
Timestamp: 2025-05-14T17:33:51.546Z
Learning: When parsing TypeSpec signatures in nanoFramework, the first Advance() call consumes the VAR/MVAR token, followed by additional Advance() calls to walk to the specific generic parameter position.
</retrieved_learning>
src/CLR/Include/nanoCLR_Types.h (7)
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-09-25T11:28:38.536Z
Learning: When working with `nanoCLR_GetNativeAssemblyInformation`, fixed-size assembly names are required, so code that deals with variable-length names cannot be used.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-10-12T19:00:39.000Z
Learning: When working with `nanoCLR_GetNativeAssemblyInformation`, fixed-size assembly names are required, so code that deals with variable-length names cannot be used.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3190
File: src/CLR/Core/TypeSystem.cpp:0-0
Timestamp: 2025-06-26T09:16:55.184Z
Learning: In nanoFramework's CLR attribute parsing (src/CLR/Core/TypeSystem.cpp), the sentinel value 0xFFFF in string tokens represents a null string. When encountered, this should result in a true null reference (using SetObjectReference(nullptr)) rather than an empty string instance, and the boxing operation should be skipped via early return.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3074
File: src/CLR/Core/GarbageCollector_Info.cpp:107-167
Timestamp: 2025-01-22T03:38:57.394Z
Learning: In nanoFramework's memory management code, DataSize() validation is comprehensively handled through CLR_RT_HeapCluster::ValidateBlock() and other caller code. Additional size checks in ValidateCluster() are redundant as the validation is already performed at multiple levels.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-10-08T15:52:09.445Z
Learning: In `nanoCLR_GetNativeAssemblyInformation`, there is no need to return the number of bytes written, as the memory buffer is zeroed, making the string buffer terminated.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3023
File: targets/netcore/nanoFramework.nanoCLR/nanoCLR_native.cpp:191-225
Timestamp: 2024-09-25T11:28:38.536Z
Learning: In `nanoCLR_GetNativeAssemblyInformation`, there is no need to return the number of bytes written, as the memory buffer is zeroed, making the string buffer terminated.
Learnt from: josesimoes
PR: nanoframework/nf-interpreter#3172
File: src/CLR/Core/CLR_RT_HeapBlock.cpp:899-900
Timestamp: 2025-05-14T16:27:02.573Z
Learning: The CLR_RT_TypeDescriptor type in nanoFramework doesn't have a GetElementType() API for extracting array element types.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALX)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALTHREE)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (3)
src/CLR/Include/nanoCLR_Types.h (2)
1064-1064: Verified:sizeOf_CLR_RECORD_FIELDREFupdated correctly to 8 bytesThe
CLR_RECORD_FIELDREFstruct now includes four 2-byte fields—type,name,encodedOwner, andsignature—for a total of 8 bytes, matching the updated constant.• src/CLR/Include/nanoCLR_Types.h:
–#define sizeOf_CLR_RECORD_FIELDREF 8(line 1064)
– Struct definition (lines 1107–1122)
1067-1067: Verified size constant matches struct definition.The
CLR_RECORD_FIELDDEFstruct now defines five 2-byte fields—type,name,signature,defaultValue, andflags—totaling 10 bytes, so the updatedsizeOf_CLR_RECORD_FIELDDEFvalue of 10 is correct and is enforced by the existingCT_ASSERT.src/CLR/Include/nanoCLR_Runtime.h (1)
1445-1449: No Breaking Change – New TypeSpec-Based Overload Added
This PR introduces a secondFindFieldDefoverload taking aconst CLR_RT_TypeSpec_Index*, while keeping the originalconst CLR_RECORD_TYPEDEF*overload intact. All existing call sites still compile against the original signature, and the new overload is already used where needed (e.g. in TypeSystem.cpp around line 3122). No updates to existing callers are required.Likely an incorrect or invalid review comment.
Automated fixes for code style.
…2681-2a44-427f-89ab-e39222501042 Code style fixes for nanoframework/nf-interpreter PR#3200
|
mscorlib Unit tests failing because of MDP version being used for post compile still not having the corresponding changes in the fields metadata. |
Description
Motivation and Context
Span<T>.[build with MDP buildId 56518]
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit
New Features
Other Changes