Fix create generic array #3158
Fix create generic array #3158
Conversation
- Fix packaging and versioning of win32 CLR. ***NO_CI***
- Fix condition to publish packages on buils from develop branch. ***NO_CI***
…into develop ***NO_CI***
…into develop ***NO_CI***
- Remove task publishing to private Azure feed. - Update condition to publish nanoCLR on develop builds. ***NO_CI***
- For consistency with generated code. ***NO_CI***
***NO_CI***
josesimoes
added
the
Area: Interpreter
WalkthroughThis update introduces extensive support for generics in the runtime, including new metadata structures, type and method specification handling, and generic instance management. It modernizes code style by replacing Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant AzurePipelines as Azure Pipelines
participant GitHub as GitHub API
participant MDPBuild as Metadata Processor Build
participant VS as Visual Studio
PR->>AzurePipelines: Trigger build (PR or branch)
AzurePipelines->>GitHub: Query PR details (if PR)
GitHub-->>AzurePipelines: Return PR body
AzurePipelines->>AzurePipelines: Check for [build with MDP buildId NNN] marker
AzurePipelines->>MDPBuild: Download MDP artifacts (if marker found)
MDPBuild-->>AzurePipelines: Provide artifacts
AzurePipelines->>VS: Copy MDP DLLs to MSBuild extension folder
AzurePipelines->>AzurePipelines: Continue pipeline with/without MDP as needed
sequenceDiagram
participant User as User Code
participant CLR as nanoCLR Runtime
participant Metadata as Metadata Structures
participant Heap as Heap Block Manager
User->>CLR: Instantiate generic type or method
CLR->>Metadata: Lookup type/method spec and generic params
CLR->>Heap: Create generic instance heap block
Heap->>CLR: Return reference to generic instance
CLR->>User: Provide instantiated generic object
Possibly related PRs
Suggested labels
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.31.1)src/CLR/Debugger/Debugger.cppsrc/CLR/Core/Execution.cppsrc/CLR/Core/Interpreter.cpp✨ Finishing Touches
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:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🔭 Outside diff range comments (3)
src/CLR/CorLib/corlib_native_System_Reflection_MethodBase.cpp (1)
110-133: 💡 Verification agent🧩 Analysis chain
Validate argument-count before calling
MakeCall()
argumentsCountis determined from the supplied array but there is no check that it matches the method’s formalargumentsCount(mdR->argumentsCount).
Passing an incorrect number of arguments will surface as a run-time fault inside the EE and can be extremely hard to diagnose from the managed side.+ if (argumentsCount != mdR->argumentsCount) + { + NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_PARAMETER); + }
🏁 Script executed:
#!/bin/bash # 1. Locate the definition of CLR_RECORD_METHODDEF to confirm the presence and type of argumentsCount rg -n "struct CLR_RECORD_METHODDEF" -A 5 # 2. Locate the signature/implementation of CLR_RT_Stack::MakeCall to check if it already validates argument count rg -n "CLR_RT_Stack::MakeCall" -A 5 # 3. Search for any internal argument-count checks within MakeCall rg -n "argumentsCount" -A 5 $(rg -l "CLR_RT_Stack::MakeCall")Length of output: 16302
#!/bin/bash # Search for MakeCall definition and usage rg -n "MakeCall(" -A 5 src/CLR rg -n "MakeCall " -A 5 src/CLR
Validate argument count before calling MakeCall()
We need to ensure the number of supplied arguments matches the method’s formal parameter count (
mdR->argumentsCount) to avoid obscure runtime faults.In src/CLR/CorLib/corlib_native_System_Reflection_MethodBase.cpp, inside the
if (stack.m_customState == 0)block, after you computeargumentsCount, insert:if (pArray) { args = (CLR_RT_HeapBlock *)pArray->GetFirstElement(); argumentsCount = pArray->m_numOfElements; } + if (argumentsCount != mdR->argumentsCount) + { + NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_PARAMETER); + } NANOCLR_CHECK_HRESULT(stack.MakeCall(md, &obj, args, argumentsCount));This check will catch mismatches early and return a clear
CLR_E_INVALID_PARAMETERrather than allowing a hard-to-debug EE fault.src/CLR/Debugger/Debugger.cpp (2)
1001-1006:⚠️ Potential issueWrong argument type passed to
AccessMemory()
AccessMemory()expectsuint32_t* errorCode, but here the address of the whole reply structure is forwarded, causing a compile-time error (or silent stack corruption with permissive compilers).- g_CLR_DBG_Debugger->AccessMemory(cmd->address, cmd->length, cmd->data, - AccessMemory_Write, &cmdReply); + g_CLR_DBG_Debugger->AccessMemory(cmd->address, cmd->length, cmd->data, + AccessMemory_Write, &cmdReply.ErrorCode);
2684-2702:⚠️ Potential issueSuccessful resize still returns
false
Debugging_Value_ResizeScratchPadsends a success reply but returnsfalse, which trips the messaging layer’s error handling.- WP_ReplyToCommand(msg, true, false, nullptr, 0); + WP_ReplyToCommand(msg, true, false, nullptr, 0); + return true;
🧹 Nitpick comments (12)
CMake/binutils.TI_SimpleLink.cmake (1)
16-17: Removed link-time optimization flagsThe link-time optimization (
-flto) flags have been removed from both Release and MinSizeRel build configurations. This will prevent the compiler from performing cross-translation unit optimizations during the linking phase.This change could lead to slightly larger binaries and potentially impact performance. However, it may also improve build times and reduce memory usage during compilation.
Consider documenting the reasoning behind this change to help future maintainers understand the tradeoff.
src/CLR/Core/CLR_RT_HeapBlock_WaitForObject.cpp (1)
32-37: Duplicated alignment assertionThe same assertion checking that the structure size is a multiple of 4 bytes appears twice, before and after a blank line.
Remove the duplicated assertion as it's already present at line 30:
_ASSERTE(sizeof(CLR_RT_HeapBlock_WaitForObject) % 4 == 0); CLR_UINT32 totLength = (CLR_UINT32)(sizeof(CLR_RT_HeapBlock_WaitForObject) + cObjects * sizeof(struct CLR_RT_HeapBlock)); - -_ASSERTE(sizeof(CLR_RT_HeapBlock_WaitForObject) % 4 == 0);src/CLR/Debugger/Debugger.cpp (1)
1750-1754: Potential memory leak on early returnWhen
GetInteropNativeAssemblies()fails it allocatesdatabut leavesfreeAllocFlagunset, thereforedatais never released.
Set the flag right after a successful allocation or free on the failure path.src/CLR/Include/nanoCLR_Debugging.h (1)
553-589: Wire-protocol breaking change: newm_tsfieldAdding
CLR_RT_TypeSpec_Index m_tschanges the on-wire size ofDebugging_Value.
All managed counterparts (nanoFramework.Tools.Debugger, VS-extension, python-tools, etc.) must be updated in lock-step or the debugger will mis-interpret every value packet from the device.Please confirm:
- Managed structures & serializers are already updated.
- Both sides use an explicit version negotiation (recommended) to avoid hard crashes when mixing old/new tools.
src/CLR/Core/Cache.cpp (1)
339-369:MoveEntryToTop()– insertion order changeThe two last writes were reordered:
list.m_next = index; entries[next].m_prev = index;Functionally this is still correct because
nextwas captured beforelist.m_nextis overwritten.
Just keep in mind that, if another thread could traverse the list concurrently (unlikely, but not impossible), writinglist.m_nextfirst temporarily breaks the bidirectional linkage (entries[next].m_prevstill points to the old head until the second write).If multithreaded access is ever introduced, swapping the two statements (as they were originally) would avoid this tiny window.
src/CLR/CorLib/corlib_native_System_Type.cpp (1)
638-642: Simplify the string-comparison logic for maintainability.The current double-negation
!strcmp(...) == falseis hard to parse. A direct inequality is clearer and less error-prone:- if (szText != nullptr && !strcmp(assm->GetString(md->name), szText) == false) + if (szText != nullptr && strcmp(assm->GetString(md->name), szText) != 0) continue;src/CLR/CorLib/corlib_native_System_GC.cpp (1)
23-41:ReRegisterForFinalizesuppresses finalization before re-registering.Calling
CLR_RT_HeapBlock_Finalizer::SuppressFinalize()first cancels any existing finalizer; the subsequentCreateInstance()re-adds it.
The extra suppression is redundant and may introduce race windows on multithreaded systems.- CLR_RT_HeapBlock_Finalizer::SuppressFinalize(pObj); - - NANOCLR_CHECK_HRESULT(desc.InitializeFromObject(stack.Arg0())); + // Ensure we have the latest type info + NANOCLR_CHECK_HRESULT(desc.InitializeFromObject(stack.Arg0()));Remove the suppression unless there is a proven need.
azure-pipelines-templates/check-mdp-for-build.yml (1)
1-1: Fix line ending in YAML fileThe file has incorrect line ending characters according to the static analysis.
Ensure the file uses consistent line endings (LF) as per YAMLlint recommendations. You can use git's
core.autocrlfconfiguration or an editor setting to normalize line endings.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
src/CLR/CorLib/corlib_native_System_Number.cpp (1)
109-111: Redundant cast chain insnprintfcall
ret = (int)(int)snprintf(...);performs two identical casts.
Only one is necessary and keeps the statement cleaner.- ret = (int)(int) + ret = (int)No functional change, just a small readability win.
src/CLR/Core/Interpreter.cpp (1)
85-116: Minor nit – local variabledtcan be madeconst
NanoCLRDataType dtis never mutated after initialisation. Marking itconstprevents accidental writes and communicates intent.- NanoCLRDataType dt = obj->DataType(); + const NanoCLRDataType dt = obj->DataType();src/CLR/Include/nanoCLR_Types.h (1)
557-574: Consider making the lookup tablesconstexprto avoid per-TU duplication
CLR_UncompressTypeToken,CLR_UncompressFieldToken, andCLR_UncompressMethodTokeneach embed astatic const NanoCLRTable c_lookup[ ]array.
Because these are defined in a header and the functions areinline, every translation unit that includes this header emits its own copy of the array. On memory-constrained targets this can add up quickly.You can mark them
constexprand rely on the compiler to fold the table away entirely, or switch to a smallswitch/ifstatement:-inline NanoCLRTable CLR_TypeFromTk(CLR_UINT32 tk) -{ - return (NanoCLRTable)(tk >> 24); -} - -inline CLR_UINT32 CLR_UncompressTypeToken(CLR_UINT32 tk) -{ - static const NanoCLRTable c_lookup[] = {TBL_TypeDef, TBL_TypeRef, TBL_TypeSpec, TBL_GenericParam}; - return CLR_TkFromType(c_lookup[(tk >> 14)], 0x3fff & tk); -} +inline CLR_UINT32 CLR_UncompressTypeToken(CLR_UINT32 tk) +{ + constexpr NanoCLRTable lookup[4] = {TBL_TypeDef, TBL_TypeRef, TBL_TypeSpec, TBL_GenericParam}; + return CLR_TkFromType(lookup[(tk >> 14)], tk & 0x3FFF); +}A
constexprarray is emitted at most once (in a COMDAT) and can even be optimized away entirely.src/CLR/Core/Execution.cpp (1)
1555-1580: Consider aligning helper with existing allocation helpersThe new helper mirrors
ExtractHeapBlocksForClassOrValueTypes, which is great for symmetry.
A couple of tiny nit-picks that improve safety/readability:
lengthis already expressed in heap blocks, not bytes, so a short explanatory
comment (as seen in the other helpers) would avoid confusion for future readers.flags |= CLR_RT_HeapBlock::HB_InitializeToZero;– this is harmless, but for
consistency with the other helpers you can remove the redundant re-assignment and
simply writeflags |= ….
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (107)
.devcontainer/All/Dockerfile.All(1 hunks).devcontainer/AzureRTOS/Dockerfile.AzureRTOS(1 hunks).devcontainer/ChibiOS/Dockerfile.ChibiOS(1 hunks).devcontainer/ESP32/Dockerfile.ESP32(1 hunks).devcontainer/FreeRTOS-NXP/Dockerfile.FreeRTOS-NXP(1 hunks).devcontainer/TI/Dockerfile.TI(1 hunks)CMake/Modules/AzureRTOS_EFM32GG11_GCC_options.cmake(1 hunks)CMake/Modules/FindNF_CoreCLR.cmake(2 hunks)CMake/Modules/TI_SimpleLink_CC13X2_GCC_options.cmake(1 hunks)CMake/binutils.AzureRTOS.cmake(1 hunks)CMake/binutils.TI_SimpleLink.cmake(1 hunks)InteropAssemblies/CLR_RT_InteropAssembliesTable.cpp.in(1 hunks)azure-pipelines-nightly.yml(4 hunks)azure-pipelines-templates/check-mdp-for-build.yml(1 hunks)azure-pipelines-templates/publish-cloudsmith.yml(1 hunks)azure-pipelines-templates/publish-nanoclr.yml(1 hunks)azure-pipelines.yml(12 hunks)src/CLR/CorLib/corlib_native.h(3 hunks)src/CLR/CorLib/corlib_native_System_AppDomain.cpp(12 hunks)src/CLR/CorLib/corlib_native_System_BitConverter.cpp(20 hunks)src/CLR/CorLib/corlib_native_System_Convert.cpp(20 hunks)src/CLR/CorLib/corlib_native_System_DateTime.cpp(3 hunks)src/CLR/CorLib/corlib_native_System_Delegate.cpp(6 hunks)src/CLR/CorLib/corlib_native_System_Exception.cpp(12 hunks)src/CLR/CorLib/corlib_native_System_GC.cpp(1 hunks)src/CLR/CorLib/corlib_native_System_Guid.cpp(1 hunks)src/CLR/CorLib/corlib_native_System_Number.cpp(9 hunks)src/CLR/CorLib/corlib_native_System_Object.cpp(2 hunks)src/CLR/CorLib/corlib_native_System_Random.cpp(1 hunks)src/CLR/CorLib/corlib_native_System_Reflection_Assembly.cpp(14 hunks)src/CLR/CorLib/corlib_native_System_Reflection_ConstructorInfo.cpp(2 hunks)src/CLR/CorLib/corlib_native_System_Reflection_FieldInfo.cpp(8 hunks)src/CLR/CorLib/corlib_native_System_Reflection_MethodBase.cpp(8 hunks)src/CLR/CorLib/corlib_native_System_Reflection_RuntimeFieldInfo.cpp(5 hunks)src/CLR/CorLib/corlib_native_System_Reflection_RuntimeMethodInfo.cpp(4 hunks)src/CLR/CorLib/corlib_native_System_RuntimeType.cpp(16 hunks)src/CLR/CorLib/corlib_native_System_Runtime_CompilerServices_RuntimeHelpers.cpp(1 hunks)src/CLR/CorLib/corlib_native_System_String.cpp(12 hunks)src/CLR/CorLib/corlib_native_System_Threading_Monitor.cpp(2 hunks)src/CLR/CorLib/corlib_native_System_Threading_Thread.cpp(9 hunks)src/CLR/CorLib/corlib_native_System_TimeSpan.cpp(2 hunks)src/CLR/CorLib/corlib_native_System_Type.cpp(25 hunks)src/CLR/CorLib/corlib_native_System_WeakReference.cpp(4 hunks)src/CLR/Core/CLR_RT_DblLinkedList.cpp(2 hunks)src/CLR/Core/CLR_RT_HeapBlock.cpp(36 hunks)src/CLR/Core/CLR_RT_HeapBlock_Array.cpp(11 hunks)src/CLR/Core/CLR_RT_HeapBlock_ArrayList.cpp(2 hunks)src/CLR/Core/CLR_RT_HeapBlock_BinaryBlob.cpp(3 hunks)src/CLR/Core/CLR_RT_HeapBlock_Delegate.cpp(3 hunks)src/CLR/Core/CLR_RT_HeapBlock_Delegate_List.cpp(6 hunks)src/CLR/Core/CLR_RT_HeapBlock_Finalizer.cpp(1 hunks)src/CLR/Core/CLR_RT_HeapBlock_GenericInstance.cpp(1 hunks)src/CLR/Core/CLR_RT_HeapBlock_Lock.cpp(1 hunks)src/CLR/Core/CLR_RT_HeapBlock_Queue.cpp(2 hunks)src/CLR/Core/CLR_RT_HeapBlock_Stack.cpp(2 hunks)src/CLR/Core/CLR_RT_HeapBlock_String.cpp(3 hunks)src/CLR/Core/CLR_RT_HeapBlock_Timer.cpp(4 hunks)src/CLR/Core/CLR_RT_HeapBlock_WaitForObject.cpp(3 hunks)src/CLR/Core/CLR_RT_HeapCluster.cpp(7 hunks)src/CLR/Core/CLR_RT_Interop.cpp(6 hunks)src/CLR/Core/CLR_RT_Memory.cpp(6 hunks)src/CLR/Core/CLR_RT_ObjectToEvent_Source.cpp(2 hunks)src/CLR/Core/CLR_RT_StackFrame.cpp(36 hunks)src/CLR/Core/CLR_RT_SystemAssembliesTable.cpp(2 hunks)src/CLR/Core/CLR_RT_UnicodeHelper.cpp(4 hunks)src/CLR/Core/Cache.cpp(15 hunks)src/CLR/Core/Checks.cpp(4 hunks)src/CLR/Core/Core.vcxproj(1 hunks)src/CLR/Core/Core.vcxproj.filters(1 hunks)src/CLR/Core/Execution.cpp(81 hunks)src/CLR/Core/FileStream/FileStream.cpp(4 hunks)src/CLR/Core/GarbageCollector.cpp(15 hunks)src/CLR/Core/GarbageCollector_Compaction.cpp(8 hunks)src/CLR/Core/GarbageCollector_ComputeReachabilityGraph.cpp(11 hunks)src/CLR/Core/GarbageCollector_Info.cpp(1 hunks)src/CLR/Core/Interpreter.cpp(56 hunks)src/CLR/Core/InterruptHandler/InterruptHandler.cpp(2 hunks)src/CLR/Core/NativeEventDispatcher/NativeEventDispatcher.cpp(9 hunks)src/CLR/Core/RPC/CLR_RT_HeapBlock_EndPoint.cpp(3 hunks)src/CLR/Core/RPC/RPC_stub.cpp(2 hunks)src/CLR/Core/Serialization/BinaryFormatter.cpp(64 hunks)src/CLR/Core/Serialization/BinaryFormatter_stub.cpp(3 hunks)src/CLR/Core/Streams.cpp(3 hunks)src/CLR/Core/StringTable.cpp(1 hunks)src/CLR/Core/Thread.cpp(41 hunks)src/CLR/Core/TypeSystemLookup.cpp(3 hunks)src/CLR/Core/Various.cpp(2 hunks)src/CLR/Core/corhdr_private.h(1 hunks)src/CLR/Debugger/Debugger.cpp(122 hunks)src/CLR/Debugger/Debugger_stub.cpp(1 hunks)src/CLR/Diagnostics/Diagnostics_stub.cpp(4 hunks)src/CLR/Diagnostics/Info.cpp(14 hunks)src/CLR/Diagnostics/Profile.cpp(5 hunks)src/CLR/Diagnostics/Profiler.cpp(30 hunks)src/CLR/Include/nanoCLR_Checks.h(4 hunks)src/CLR/Include/nanoCLR_Debugging.h(6 hunks)src/CLR/Include/nanoCLR_Interop.h(8 hunks)src/CLR/Include/nanoCLR_Profiling.h(5 hunks)src/CLR/Include/nanoCLR_Runtime.h(48 hunks)src/CLR/Include/nanoCLR_Runtime__HeapBlock.h(27 hunks)src/CLR/Include/nanoCLR_Runtime__Serialization.h(4 hunks)src/CLR/Include/nanoCLR_Types.h(21 hunks)src/CLR/Messaging/Messaging.cpp(16 hunks)src/CLR/Startup/CLRStartup.cpp(9 hunks)src/CLR/System.Math/nf_native_system_math.cpp(2 hunks)src/CLR/System.Math/nf_native_system_math_System_Math.cpp(23 hunks)src/DeviceInterfaces/Network/Enc28j60/enc28j60_lwip_driver.cpp(5 hunks)
⛔ Files not processed due to max files limit (51)
- src/DeviceInterfaces/Networking.Sntp/nf_networking_sntp.cpp
- src/DeviceInterfaces/System.Net/sys_net_native.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Net_NetworkInformation_NetworkInterface.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Net_NetworkInformation_Wireless80211Configuration.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Net_NetworkInformation_WirelessAPConfiguration.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Net_Security_CertificateManager.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Net_Security_SslNative.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Net_Sockets_NativeSocket.cpp
- src/DeviceInterfaces/System.Net/sys_net_native_System_Security_Cryptography_X509Certificates_X509Certificate2.cpp
- src/HAL/Include/nanoHAL.h
- src/HAL/Include/nanoHAL_Network.h
- src/HAL/Include/nanoHAL_ReleaseInfo.h
- src/PAL/AsyncProcCall/AsyncCompletions.cpp
- src/PAL/AsyncProcCall/AsyncContinuations.cpp
- src/PAL/COM/sockets/Sockets_debugger.cpp
- src/PAL/COM/sockets/sockets_lwip.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_accept_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_add_cert_auth_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_available_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_close_socket_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_connect_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_decode_private_key_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_exit_context_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_generic.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_generic_init_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_read_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_uninitialize_internal.cpp
- src/PAL/COM/sockets/ssl/MbedTLS/ssl_write_internal.cpp
- src/PAL/COM/sockets/ssl/ssl.cpp
- src/PAL/Events/nanoPAL_Events.cpp
- src/PAL/Events/nanoPAL_Events_driver.cpp
- src/PAL/FileSystem/nanoPAL_FileSystem.cpp
- src/PAL/Lwip/lwIP_Sockets.cpp
- src/PAL/Lwip/lwIP_Sockets_functions.cpp
- src/System.Device.Gpio/sys_dev_gpio_native.cpp
- src/System.Device.Gpio/sys_dev_gpio_native_System_Device_Gpio_GpioController.cpp
- src/System.Device.Gpio/sys_dev_gpio_native_System_Device_Gpio_GpioPin.cpp
- src/System.Device.Spi/nanoHAL_Spi.cpp
- src/System.Device.Spi/sys_dev_spi_native_System_Device_Spi_SpiDevice.cpp
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_Directory.cpp
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_DriveInfo.cpp
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_File.cpp
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeFileStream.cpp
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeFindFile.cpp
- src/System.IO.FileSystem/nf_sys_io_filesystem_System_IO_NativeIO.cpp
- src/System.IO.Ports/sys_io_ser_native.cpp
- src/System.IO.Ports/sys_io_ser_native_System_IO_Ports_SerialPort__.cpp
- src/System.Runtime.Serialization/nf_system_runtime_serialization.cpp
- src/System.Runtime.Serialization/nf_system_runtime_serialization_System_Runtime_Serialization_Formatters_Binary_BinaryFormatter.cpp
- src/nanoFramework.Graphics/Graphics/Core/Graphics.cpp
- src/nanoFramework.Graphics/Graphics/Core/GraphicsDriver.cpp
🧰 Additional context used
🧠 Learnings (1)
src/CLR/CorLib/corlib_native_System_Reflection_Assembly.cpp (2)
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.
🧬 Code Graph Analysis (12)
src/CLR/Core/CLR_RT_HeapBlock_Stack.cpp (2)
src/CLR/Core/CLR_RT_HeapBlock_Array.cpp (6)
CreateInstance(10-64)CreateInstance(10-13)CreateInstance(66-83)CreateInstance(66-69)CreateInstance(85-119)CreateInstance(85-90)src/CLR/Include/nanoCLR_Runtime__HeapBlock.h (12)
size(2776-2779)size(2776-2776)size(2853-2857)size(2853-2853)size(2896-2900)size(2896-2896)array(1356-1356)array(1878-1884)array(2838-2842)array(2838-2838)array(2891-2895)array(2891-2891)
src/CLR/Core/CLR_RT_HeapBlock_Delegate.cpp (1)
src/CLR/Include/nanoCLR_Runtime__HeapBlock.h (16)
reference(1816-1816)reference(1817-1817)reference(1818-1818)reference(1819-1819)reference(1820-1820)reference(1821-1821)reference(1836-1839)reference(1840-1840)reference(1841-1846)reference(1924-1927)reference(1949-1953)reference(1954-1954)reference(1959-1964)reference(1990-1995)reference(2037-2037)reference(2405-2405)
src/CLR/Core/CLR_RT_HeapBlock_Queue.cpp (1)
src/CLR/Core/CLR_RT_HeapBlock_Array.cpp (6)
CreateInstance(10-64)CreateInstance(10-13)CreateInstance(66-83)CreateInstance(66-69)CreateInstance(85-119)CreateInstance(85-90)
src/CLR/Messaging/Messaging.cpp (1)
src/CLR/WireProtocol/WireProtocol_Message.c (1)
WP_ReplyToCommand(117-165)
src/CLR/Core/Serialization/BinaryFormatter_stub.cpp (2)
src/CLR/Core/Serialization/BinaryFormatter.cpp (6)
FixNull(25-34)FixNull(25-25)GetDataType(200-204)GetDataType(200-200)GetDuplicate(2034-2047)GetDuplicate(2034-2034)src/CLR/Include/nanoCLR_Runtime__Serialization.h (7)
v(150-150)v(162-162)v(163-163)type(158-158)type(159-159)type(160-160)index(255-255)
src/CLR/Core/CLR_RT_UnicodeHelper.cpp (1)
src/CLR/Include/nanoCLR_Runtime.h (1)
m_wCharArray(1102-1105)
src/CLR/Include/nanoCLR_Interop.h (1)
src/CLR/Include/nanoCLR_Runtime.h (1)
assmIndex(4179-4179)
src/CLR/Core/CLR_RT_HeapBlock_Array.cpp (2)
src/CLR/Include/nanoCLR_Runtime__HeapBlock.h (50)
reference(1816-1816)reference(1817-1817)reference(1818-1818)reference(1819-1819)reference(1820-1820)reference(1821-1821)reference(1836-1839)reference(1840-1840)reference(1841-1846)reference(1924-1927)reference(1949-1953)reference(1954-1954)reference(1959-1964)reference(1990-1995)reference(2037-2037)reference(2405-2405)length(2002-2002)reflex(1347-1347)cls(1171-1171)cls(1350-1350)cls(1363-1363)cls(1364-1364)caller(2241-2241)caller(2242-2242)caller(2243-2248)assm(1348-1348)ref(1355-1355)ref(1359-1359)ref(1360-1360)ref(2146-2146)ref(2206-2206)ref(2307-2307)ref(2440-2440)ref(2456-2456)ref(2539-2539)ref(2540-2540)ref(2614-2614)ref(2713-2713)ptr(845-845)ptr(859-861)ptr(859-859)ptr(1009-1013)ptr(1009-1009)ptr(1373-1373)ptr(2341-2341)ptr(2391-2391)ptr(2562-2565)ptr(2562-2562)ptr(2566-2569)ptr(2566-2566)src/CLR/Include/nanoCLR_Runtime.h (50)
reflex(2097-2097)reflex(2334-2334)cls(2003-2008)cls(2009-2009)cls(2021-2024)cls(2025-2029)cls(2336-2336)cls(3519-3522)cls(3577-3580)cls(3618-3621)cls(4067-4067)dt(1588-1588)dt(2044-2044)dt(2333-2333)inst(2243-2243)inst(2244-2244)inst(2245-2245)inst(3998-4001)inst(4047-4047)tk(1566-1566)tk(2081-2081)tk(2105-2105)tk(2138-2138)tk(2166-2166)tk(2202-2202)caller(4057-4057)caller(4059-4059)assm(1203-1203)assm(1204-1204)assm(1206-1206)assm(1207-1207)assm(1208-1208)assm(1210-1210)assm(1211-1211)assm(1213-1213)assm(1215-1215)assm(1216-1216)assm(1388-1388)assm(1627-1627)assm(1639-1639)assm(1978-1978)assm(1979-1979)ref(863-863)ref(864-864)ref(865-865)ref(866-866)ptr(374-374)ptr(375-375)ptr(525-525)ptr(527-527)
src/CLR/Core/CLR_RT_Memory.cpp (1)
src/CLR/Include/nanoCLR_Runtime.h (5)
len(524-524)len(526-526)len(528-528)p(1568-1568)p(1569-1569)
src/CLR/Include/nanoCLR_Debugging.h (1)
src/CLR/Include/nanoCLR_Runtime.h (33)
indexAssm(619-622)indexAssm(619-619)indexAssm(643-646)indexAssm(643-643)indexAssm(672-675)indexAssm(672-672)indexAssm(701-704)indexAssm(701-701)indexAssm(730-733)indexAssm(730-730)indexAssm(759-762)indexAssm(759-759)indexAssm(788-791)indexAssm(788-788)indexAssm(817-820)indexAssm(817-817)inst(2243-2243)inst(2244-2244)inst(2245-2245)inst(3998-4001)inst(4047-4047)index(1463-1463)index(1471-1471)index(1570-1570)index(1571-1571)index(1637-1637)index(2065-2065)index(2078-2078)index(2098-2098)index(2135-2135)index(2163-2163)index(2198-2198)index(2219-2219)
src/CLR/Core/Cache.cpp (2)
src/CLR/Include/nanoCLR_Runtime.h (31)
en(2292-2292)index(1463-1463)index(1471-1471)index(1570-1570)index(1571-1571)index(1637-1637)index(2065-2065)LinkArraySize(3446-3446)LinkMRUArraySize(3447-3447)cls(2003-2008)cls(2009-2009)cls(2021-2024)cls(2025-2029)cls(2336-2336)cls(3519-3522)cls(3577-3580)cls(3618-3621)cls(4067-4067)md(1426-1430)md(1431-1434)md(2010-2014)md(2099-2099)md(2560-2560)PayloadArraySize(3448-3448)res(1227-1227)res(2294-2294)entries(3588-3588)node(2374-2374)node(3535-3535)node(3612-3612)ppBuffer(3624-3624)src/CLR/Core/CLR_RT_RuntimeMemory.cpp (6)
LinkArraySize(111-114)LinkArraySize(111-111)LinkMRUArraySize(115-118)LinkMRUArraySize(115-115)PayloadArraySize(119-122)PayloadArraySize(119-119)
src/CLR/Core/TypeSystemLookup.cpp (8)
src/CLR/Core/CLR_RT_HeapBlock.cpp (12)
Relocate_String(2509-2517)Relocate_String(2509-2509)Relocate_Obj(2519-2523)Relocate_Obj(2519-2519)Relocate_Cls(2525-2531)Relocate_Cls(2525-2525)Relocate_Ref(2533-2537)Relocate_Ref(2533-2533)Relocate_ArrayRef(2539-2543)Relocate_ArrayRef(2539-2539)Relocate_TransparentProxy(2546-2550)Relocate_TransparentProxy(2546-2546)src/CLR/Core/CLR_RT_HeapBlock_Array.cpp (2)
Relocate(167-177)Relocate(167-167)src/CLR/Core/Execution.cpp (2)
Relocate(436-459)Relocate(436-436)src/CLR/Core/TypeSystem.cpp (6)
Relocate(3568-3574)Relocate(3568-3568)Relocate(3946-3950)Relocate(3946-3946)Relocate(4907-4919)Relocate(4907-4907)src/CLR/Core/CLR_RT_HeapBlock_Delegate.cpp (2)
Relocate(83-87)Relocate(83-83)src/CLR/CorLib/corlib_native_System_WeakReference.cpp (2)
Relocate(18-25)Relocate(18-18)src/CLR/Core/CLR_RT_HeapBlock_Lock.cpp (4)
Relocate(235-239)Relocate(235-235)Relocate_Owner(241-244)Relocate_Owner(241-241)src/CLR/Core/RPC/RPC_stub.cpp (1)
CLR_RT_HeapBlock_EndPoint(22-27)
🪛 Cppcheck (2.10-2)
src/CLR/Core/CLR_RT_Interop.cpp
[error] 981-981: Null pointer dereference
(nullPointer)
🪛 YAMLlint (1.35.1)
azure-pipelines-templates/check-mdp-for-build.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
🔇 Additional comments (305)
InteropAssemblies/CLR_RT_InteropAssembliesTable.cpp.in (1)
22-22: Modernize terminator tonullptr.
Replacing the C-styleNULLwith C++11’snullptrenhances type safety and clarity for pointer arrays.src/CLR/Core/InterruptHandler/InterruptHandler.cpp (2)
48-50: Modernize pointer check in SpawnDispatcher
Replacing the C-styleNULLwithnullptrenhances type safety and aligns with C++11 best practices without altering behavior.
94-95: Usenullptrin interrupt queue loop
Switching fromNULLtonullptrfor the HAL record pointer check is consistent with the updated code style and safe in C++ contexts.src/CLR/Core/FileStream/FileStream.cpp (3)
36-38: Modernization: Usenullptrfor pointer checks
The update replaces the C-styleNULLwith C++11’snullptrwhen validating the driver fromFindVolume, improving type safety. The assignment inside theifis clear and properly parenthesized.
64-65: Modernization: UsenullptrinCreateInstancecall
Switching the third argument tonullptrinCLR_RT_HeapBlock_BinaryBlob::CreateInstancealigns with modern C++ guidelines and keeps pointer intent explicit.
89-91: Modernization: Replace C-style null checks withnullptr
Good refinement—checkingsdd->inputBufferandsdd->outputBufferagainstnullptrinstead ofNULLmakes the intent clearer and leverages the stronger typing ofnullptr.src/CLR/Core/CLR_RT_ObjectToEvent_Source.cpp (2)
38-40: Code modernization: Enhanced type safety with nullptr.Good update replacing NULL with nullptr for better type safety and modern C++ style.
59-59: Code modernization: Enhanced type safety with nullptr.Good update replacing NULL with nullptr for better type safety and modern C++ style.
src/CLR/Core/CLR_RT_SystemAssembliesTable.cpp (3)
12-16: Code modernization: Enhanced type safety with nullptr.Good update replacing NULL with nullptr in both the comment and the code. This improves type safety and follows modern C++ best practices.
21-21: Code modernization: Enhanced type safety with nullptr.Good update replacing NULL with nullptr in the null check for better type safety.
28-28: Code modernization: Enhanced type safety with nullptr.Good update replacing NULL with nullptr for the return statement.
src/CLR/Core/CLR_RT_UnicodeHelper.cpp (6)
519-519: Code modernization: Enhanced type safety with nullptr in Windows API call.Good update replacing NULL with nullptr in the WideCharToMultiByte API call parameters.
525-525: Code modernization: Enhanced type safety with nullptr in Windows API call.Good update replacing NULL with nullptr in the WideCharToMultiByte API call parameters.
547-547: Code modernization: Enhanced type safety with nullptr in Windows API call.Good update replacing NULL with nullptr in the MultiByteToWideChar API call parameters.
572-572: Code modernization: Enhanced type safety with nullptr in constructor.Good update replacing NULL with nullptr for the initial pointer value in the constructor.
617-617: Code modernization: Enhanced type safety with nullptr in condition check.Good update replacing NULL with nullptr in the pointer check.
622-622: Code modernization: Enhanced type safety with nullptr assignment.Good update replacing NULL with nullptr for the pointer assignment.
src/DeviceInterfaces/Network/Enc28j60/enc28j60_lwip_driver.cpp (6)
8-8: Style improvement: Consistent comment formatting.Good update adding a space after the comment slashes for better readability.
224-227: Code modernization and style improvement: Enhanced type safety with nullptr and added braces.Good updates:
- Replaced NULL with nullptr for better type safety
- Added braces around the if-statement body for better readability and to prevent future bugs
234-238: Style improvement: Added braces for conditional block.Good update adding braces around the if-statement body for better readability and to prevent future bugs.
272-272: Code modernization: Enhanced type safety with nullptr in function call.Good update replacing NULL with nullptr in the netif_add function call for better type safety.
302-305: Code modernization and style improvement: Enhanced type safety with nullptr and added braces.Good updates:
- Replaced NULL with nullptr for better type safety
- Added braces around the if-statement body for better readability and to prevent future bugs
317-317: Code modernization: Enhanced type safety with nullptr in function call.Good update replacing NULL with nullptr in the CPU_GPIO_EnableInputPin function call for better type safety.
.devcontainer/ChibiOS/Dockerfile.ChibiOS (1)
1-1: Approve devcontainer base image bump to v3.0
TheFROMdirective has been updated to use versionv3.0, aligning this ChibiOS container with other development environments..devcontainer/AzureRTOS/Dockerfile.AzureRTOS (1)
1-1: Approve devcontainer base image bump to v3.0
TheFROMdirective has been updated to use versionv3.0, keeping it consistent with the other Azure RTOS containers..devcontainer/FreeRTOS-NXP/Dockerfile.FreeRTOS-NXP (1)
1-1: Approve devcontainer base image bump to v3.0
TheFROMdirective now references versionv3.0, matching the standard across all FreeRTOS-NXP containers..devcontainer/TI/Dockerfile.TI (1)
1-1: Approve devcontainer base image bump to v3.0
TheFROMdirective has been updated tov3.0, standardizing this TI container with the rest of the dev environment images..devcontainer/All/Dockerfile.All (1)
1-1: Approve devcontainer base image bump to v3.0
TheFROMline now uses versionv3.0, ensuring consistency across the “All” devcontainer image and its platform-specific counterparts..devcontainer/ESP32/Dockerfile.ESP32 (1)
1-1: Approve modernization of ESP32 dev container base image
Updating the base image fromv2.37tov3.0ensures consistency with other.devcontainerconfigurations and brings in the latest tooling improvements.src/CLR/Core/RPC/RPC_stub.cpp (2)
26-26: Approve nullptr replacement inFindEndPoint
Replacing the C-styleNULLwith C++11’snullptrenhances type safety and is consistent with the rest of the codebase.
78-78: Approve nullptr replacement inFindMessage
The switch fromNULLtonullptris purely stylistic and aligns with modern C++ best practices.src/CLR/Core/Core.vcxproj (1)
33-33: Approve addition of generic instance source file
IncludingCLR_RT_HeapBlock_GenericInstance.cppin the project ensures that the new generic‐instance implementation is built.src/CLR/Core/Core.vcxproj.filters (1)
144-146: Approve filter entry for the new source file
AddingCLR_RT_HeapBlock_GenericInstance.cppunder “Source Files” keeps the Visual Studio project tree in sync with the build configuration.src/CLR/Core/CLR_RT_HeapBlock_Delegate.cpp (2)
18-18: Approve nullptr in delegate initialization
SwitchingSetObjectReference(NULL)toSetObjectReference(nullptr)is consistent with the C++11 modernization strategy.
58-58: Approve nullptr in delegate object member
Replacingm_object.SetObjectReference(NULL)withnullptraligns with improved pointer semantics.src/CLR/Core/GarbageCollector_Info.cpp (1)
273-274: Modernize pointer initialization to nullptr
Replacing legacy NULL with C++11 nullptr fornewRefandnewPtrimproves type safety and consistency with the rest of the codebase.src/CLR/CorLib/corlib_native_System_Random.cpp (1)
118-123: Use nullptr in CreateInstance invocation
Updated the null pointer arguments tonullptrinCLR_RT_HeapBlock_BinaryBlob::CreateInstancefor consistency with modern C++ standards.src/CLR/Core/CLR_RT_HeapBlock_Queue.cpp (2)
28-28: Clear object reference with nullptr
Usingnullptrinstead ofNULLinSetObjectReferencealigns this code with C++11 pointer best practices.
62-63: Use updated well-known type member
Changedg_CLR_RT_WellKnownTypes.m_Objecttog_CLR_RT_WellKnownTypes.Object, reflecting the renaming of the well-known type member for consistency across the codebase.src/CLR/Core/CLR_RT_HeapBlock_BinaryBlob.cpp (3)
26-26: Initializem_assemblyto nullptr
Replaced assignment fromNULLtonullptrform_assemblyto improve type safety and clarity.
55-55: Use nullptr in handler setup
Replaced legacy NULL handlers withnullptrinAllocate()to modernize pointer usage.
64-64: Reset handlers with nullptr in Release
UpdatedSetBinaryBlobHandlerscall to usenullptrfor consistency and readability.src/CLR/Core/StringTable.cpp (2)
42-42: Rename loop index variable toindex
Replacingidxwithindexenhances readability and aligns with naming conventions used elsewhere.
48-48: Use decrementedindexfor mapping strings
The updated variable nameindexis clearer when assigning descending offsets in the string table initialization.src/CLR/Debugger/Debugger_stub.cpp (1)
76-76: Good modernization of NULL to nullptrThe replacement of NULL with nullptr throughout the file follows modern C++ best practices and improves type safety.
Also applies to: 83-83, 90-90
src/CLR/System.Math/nf_native_system_math.cpp (1)
12-12: Appropriate use of nullptr in method lookup tableThe consistent replacement of NULL with nullptr in the method lookup table improves code quality by using the type-safe C++11 nullptr keyword instead of the C-style NULL macro.
Also applies to: 21-24, 32-32, 35-35, 41-41
src/CLR/Core/GarbageCollector_ComputeReachabilityGraph.cpp (1)
22-22: Appropriate modernization of null pointer checksThe changes consistently replace NULL with nullptr throughout the garbage collector implementation, following modern C++ best practices without altering behavior.
Also applies to: 25-25, 35-35, 61-61, 74-74, 107-107, 116-116, 150-150, 158-158, 177-177, 203-203, 267-267, 303-303
src/CLR/Core/CLR_RT_DblLinkedList.cpp (2)
14-14: Good modernization of NULL to nullptrThe replacement of NULL with nullptr follows modern C++ best practices and improves type safety.
77-80: Improved code style with braces around conditional blockThe changes not only update NULL to nullptr but also properly add braces around the conditional block, which is a good defensive programming practice.
src/CLR/Core/RPC/CLR_RT_HeapBlock_EndPoint.cpp (3)
39-39: Good modernization of null pointer usage.Replacing
NULLwithnullptris a good C++11 practice that improves type safety.
56-56: Good modernization of null pointer usage.Replacing
NULLwithnullptris a good C++11 practice that improves type safety.
137-137: Good modernization of null pointer usage.Replacing
NULLwithnullptris a good C++11 practice that improves type safety.src/CLR/Core/CLR_RT_HeapBlock_Lock.cpp (1)
141-141: Good modernization of null pointer usage.Replacing
NULLwithnullptris a good C++11 practice that improves type safety. This change aligns with the broader modernization effort across the codebase.src/CLR/CorLib/corlib_native_System_Runtime_CompilerServices_RuntimeHelpers.cpp (2)
28-28: Good standardization of member variable naming.Removing the
m_prefix from member variables (inst.m_target→inst.target) improves code consistency with the rest of the modernized codebase. This type of renaming is seen throughout the PR.
34-34: Good standardization of member variable naming.Renaming
inst.m_assmtoinst.assemblyimproves readability and follows the consistent naming pattern being applied throughout the codebase in this PR.src/CLR/CorLib/corlib_native_System_WeakReference.cpp (4)
77-77: Good modernization of null pointer usage.Replacing
NULLwithnullptris a good C++11 practice that improves type safety.
114-114: Good modernization of null pointer usage.Replacing
NULLwithnullptrfor object references and null checks improves code quality and type safety.Also applies to: 116-116
128-128: Good modernization of null pointer usage.Consistently replacing
NULLwithnullptrthroughout the binary formatter deserialization process improves code quality and readability.Also applies to: 134-134, 136-136
178-178: Good modernization of null pointer usage.Using
nullptrfor the null check in theIsAliveproperty getter follows modern C++ best practices.CMake/Modules/FindNF_CoreCLR.cmake (2)
52-52: Added support for generic instances to the buildThe inclusion of
CLR_RT_HeapBlock_GenericInstance.cppin the core source files directly supports the PR's objective of fixing the creation of array instances for generic types.
132-132: Consolidated native system information componentsThe change consolidates separate native implementation files into a single
nf_rt_native_nanoFramework_Runtime_Native_SystemInfo.cppfile. This refactoring simplifies the runtime native components organization.src/CLR/CorLib/corlib_native_System_Threading_Monitor.cpp (1)
13-13: Modernized pointer usage with nullptrReplacing
NULLwithnullptrimproves type safety and adheres to modern C++ standards. This is a good practice, making the code more robust against potential type-related errors.Also applies to: 15-15, 53-53, 55-55
src/CLR/Core/CLR_RT_HeapBlock_String.cpp (1)
18-18: Modernized pointer usage with nullptrReplacing
NULLwithnullptrin string handling operations improves type safety and follows modern C++ best practices. This change is consistent with the modernization efforts across the codebase.Also applies to: 33-33, 91-91
src/CLR/Core/CLR_RT_HeapBlock_ArrayList.cpp (2)
151-151: Modernized pointer usage with nullptrReplaced
NULLwithnullptrwhen clearing object references, improving type safety in theRemoveAtmethod.
183-183: Updated well-known type reference naming conventionChanged from using
g_CLR_RT_WellKnownTypes.m_Objecttog_CLR_RT_WellKnownTypes.Object, removing the Hungarian notation prefix. This improves naming consistency across the codebase.src/CLR/Core/NativeEventDispatcher/NativeEventDispatcher.cpp (7)
41-42: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
44-47: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
59-63: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
75-80: Modernized pointer handling and updated commentsUpdated NULL to nullptr in both the comments and code for better type safety.
189-195: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
245-248: Updated member variable naming conventionChanged from
m_call.m_targettom_call.target, removing the Hungarian notation prefix.This change aligns with the modern C++ naming convention adopted throughout the codebase.
302-303: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
src/CLR/Core/CLR_RT_HeapBlock_WaitForObject.cpp (2)
100-100: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
130-130: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
src/CLR/CorLib/corlib_native_System_Delegate.cpp (6)
29-30: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
48-49: Updated well-known type naming conventionChanged from
g_CLR_RT_WellKnownTypes.m_Delegatetog_CLR_RT_WellKnownTypes.Delegate, removing the Hungarian notation prefix.This change aligns with the modern C++ naming convention adopted throughout the codebase.
64-66: Updated well-known type naming conventionChanged from
g_CLR_RT_WellKnownTypes.m_Delegatetog_CLR_RT_WellKnownTypes.Delegate, removing the Hungarian notation prefix.This change aligns with the modern C++ naming convention adopted throughout the codebase.
78-80: Updated well-known type naming conventionChanged from
g_CLR_RT_WellKnownTypes.m_Delegatetog_CLR_RT_WellKnownTypes.Delegate, removing the Hungarian notation prefix.This change aligns with the modern C++ naming convention adopted throughout the codebase.
103-104: Updated well-known type naming conventionChanged from
g_CLR_RT_WellKnownTypes.m_MethodInfotog_CLR_RT_WellKnownTypes.MethodInfo, removing the Hungarian notation prefix.This change aligns with the modern C++ naming convention adopted throughout the codebase.
193-194: Modernized pointer handling with nullptrUpdated NULL to nullptr for better type safety.
src/CLR/Core/CLR_RT_HeapBlock_Delegate_List.cpp (1)
76-76: Great modernization! Usingnullptrimproves type safety.The code changes follow modern C++ best practices by replacing all occurrences of
NULLwithnullptr. This improves type safety and makes the code more idiomatic to modern C++ standards. Also, the member access change from.m_datato.dataaligns with the updated naming conventions in the codebase.Also applies to: 85-85, 104-104, 115-115, 117-117, 161-161, 183-183
azure-pipelines-templates/publish-cloudsmith.yml (1)
159-172: Good addition of the 'develop' branch to publish conditions.This change expands the CI/CD pipeline to allow publishing packages from both 'main' and 'develop' branches. This is a common pattern that helps with continuous delivery by allowing testing of development packages before they reach production.
CMake/Modules/TI_SimpleLink_CC13X2_GCC_options.cmake (1)
53-55:❓ Verification inconclusive
Removed Link-Time Optimization flags.
The change removes
-flto -fuse-linker-pluginflags and keeps only-Osfor size optimization. This may slightly increase binary size but could improve compatibility or solve issues with Link-Time Optimization on this platform.
🏁 Script executed:
#!/bin/bash # Search for any related changes to LTO flags in other platform configurations echo "Checking for similar LTO flag changes in other platform files..." rg -A 1 -B 1 "flto" "CMake/Modules/*.cmake"Length of output: 244
I wasn’t able to find any
fltooccurrences in the expected directory. Let’s broaden the search across all CMake files:
🏁 Script executed:
#!/bin/bash # Search for any Link-Time Optimization flags across all .cmake files echo "Searching for '-flto' in all CMake files..." rg -n --type cmake "flto" || true echo "Searching for '-fuse-linker-plugin' in all CMake files..." rg -n --type cmake "fuse-linker-plugin" || trueLength of output: 7390
Confirm intentional LTO removal for TI_SimpleLink_CC13X2
The patch drops
-flto -fuse-linker-plugin, leaving only-Osin
CMake/Modules/TI_SimpleLink_CC13X2_GCC_options.cmake(lines 53–55). All other platform modules still append LTO flags:• CMake/Modules/CHIBIOS_*_GCC_options.cmake
• CMake/Modules/FreeRTOS_IMXRT10xx_GCC_options.cmake
• CMake/Modules/TI_SimpleLink_CC32xx_GCC_options.cmake
• …and othersPlease verify that removing LTO here is deliberate (e.g., due to compatibility issues) and, if so, consider documenting why this platform differs.
src/CLR/CorLib/corlib_native_System_Reflection_ConstructorInfo.cpp (1)
27-28: Improved code with modern C++ semantics and better variable naming.The changes modernize the code by:
- Using
nullptrinstead ofNULLfor better type safety- Renaming
numArgstoArgumentsCountfor better readability- Updating struct member access from
.m_data.m_methodto.data.methodto match new naming conventionsThese changes align with the PR's goal of improving code quality without altering functionality.
Also applies to: 30-30, 40-41, 43-43
azure-pipelines-nightly.yml (2)
286-292: Build target ST_NUCLEO64_F091RC has been disabledThe code comments out the ST_NUCLEO64_F091RC build target in the STM32 matrix, effectively disabling it from the nightly builds. This appears to be an intentional change to exclude this specific target from the automated builds.
297-297: Version counter naming updated to reflect new versioning schemeThe REVISION counter variables have been consistently updated across all platform builds from using
1_12_4_versioncounterto2_0_0_versioncounter. This change suggests a major version upgrade from 1.12.4 to 2.0.0.Also applies to: 585-585, 668-668, 720-720
src/CLR/Core/Streams.cpp (3)
32-32: Modernized code by replacing NULL with nullptrThe change replaces the C-style NULL macro with C++11's nullptr keyword, improving type safety in pointer assignment.
98-98: Updated well-known type referenceThe code now uses
g_CLR_RT_WellKnownTypes.UInt8instead of the previousm_UInt8naming pattern, which is part of a broader naming convention update for consistency.
206-206: Modernized null pointer checkThe null pointer check now uses the C++11 nullptr keyword instead of the C-style NULL macro, consistent with modern C++ practices.
src/CLR/CorLib/corlib_native_System_Object.cpp (1)
38-38: Improved variable naming and updated member access styleMultiple improvements have been made to the GetType___SystemType method:
- Variable
idxrenamed to the more descriptiveindex- Member access updated to remove
m_prefixes (e.g.,kindinstead ofm_kind)- Well-known type reference updated to use
TypeStaticinstead ofm_TypeStaticThese changes improve readability and maintain consistency with the modern naming conventions being applied across the codebase.
Also applies to: 48-50, 54-54, 61-61, 64-64
CMake/binutils.AzureRTOS.cmake (1)
12-14:❓ Verification inconclusive
Removed Link-Time Optimization from Release configurations
The
-fltoflag has been removed from both Release and MinSizeRel build configurations. This change likely improves build reliability at the cost of potentially larger binary size and slightly reduced runtime performance.
🌐 Web query:
What are the tradeoffs of disabling Link-Time Optimization (LTO) in embedded systems?💡 Result:
Tradeoffs of Disabling Link-Time Optimization (LTO) in Embedded Systems
Disabling Link-Time Optimization (LTO) in embedded systems comes with several noteworthy tradeoffs, balancing between build complexity and performance or code size benefits.
Advantages of Disabling LTO
- Faster Build Times and Lower Memory Usage: LTO requires analyzing and optimizing the entire program at link time, making both compilation and linking significantly slower and more memory-intensive—especially for large projects[6]. Disabling LTO can make incremental builds much faster and reduce peak memory requirements, which is especially valuable on developer workstations with limited resources or for rapid development cycles.
- Simplicity and Predictability: Without LTO, the build process is generally more straightforward. Each compilation unit is optimized in isolation, reducing the chances of encountering toolchain compatibility issues, subtle bugs, or the need for special attributes (e.g., marking ISRs as "used" to avoid their accidental removal)[8]. Debugging can also be easier, as it is simpler to correlate the compiled output to the source code.
- Improved Compatibility and Fewer Surprises: Some third-party libraries, legacy codebases, or specialized startup code might not be compatible with LTO, leading to runtime errors, crashes, or erratic behavior that are difficult to diagnose[8]. Disabling LTO can avoid these pitfalls, especially with toolchains or hardware with imperfect LTO support.
Disadvantages of Disabling LTO
- Larger Code Size: LTO enables aggressive dead code elimination and function inlining across module boundaries, which can significantly reduce the final binary size—a critical benefit in memory-constrained embedded applications[3][8]. Without LTO, the linker cannot see or optimize away unused code across different files, resulting in larger binaries.
- Lower Runtime Performance: LTO’s cross-unit optimizations (function inlining, improved code locality, etc.) can result in binaries that are not only smaller, but also a few percent faster, which can be significant in time-critical embedded tasks[6][7]. Disabling LTO forgoes these gains.
- Missed Cross-Module Optimizations: Optimizations that require knowledge of the entire program—such as merging identical constants, better inlining, and improved function placement for cache locality—are only possible with LTO[1][6][7]. Without it, cross-module opportunities are lost.
Embedded-Specific Impact
- Code Size Pressure: Many embedded systems have strict flash or RAM limits. Disabling LTO may cause the firmware to exceed available memory, forcing developers to more aggressively hand-optimize code or refactor to fit[3][8].
- Reliability vs. Aggressiveness: While LTO can introduce subtle bugs (e.g., by removing or inlining ISRs incorrectly if not marked properly), disabling it may smooth the path for debugging and integration—useful during initial bring-up or with less mature toolchains[5][8].
Summary Table
Tradeoff With LTO Without LTO Build Speed & Memory Slower, higher memory usage Faster, less memory Binary Size Smaller Larger Runtime Performance Faster Potentially slower Optimization Scope Whole-program (cross-module) Module-local only Debuggability More complex Easier Risk of Subtle Bugs Higher (if code is not LTO-clean) Lower Conclusion
Disabling LTO in embedded systems can make builds faster and more predictable at the expense of losing potentially significant code size and runtime performance optimizations. In environments where every byte or cycle counts, these lost optimizations may force additional manual tuning or compromises. Conversely, for debugging and rapid development, or when facing LTO toolchain compatibility issues, disabling LTO can be the pragmatic choice[6][8]. The right decision depends on the specific requirements and constraints of the embedded project.
Citations:
- 1: https://developer.arm.com/documentation/101458/latest/Optimize/Link-Time-Optimization--LTO-/What-is-Link-Time-Optimization--LTO-
- 2: https://developer.arm.com/documentation/100748/latest/Writing-Optimized-Code/Optimizing-across-modules-with-Link-Time-Optimization/Restrictions-with-Link-Time-Optimization
- 3: http://www.diva-portal.org/smash/get/diva2:665922/FULLTEXT01.pdf
- 4: https://e2e.ti.com/support/tools/code-composer-studio-group/ccs/f/code-composer-studio-forum/237382/optimizer-information-file-for-link-time-optimization
- 5: https://community.st.com/t5/stm32-mcus-products/linker-time-optimization-introduces-glitch-noise-into-timer-dma/td-p/253072
- 6: https://johnnysswlab.com/link-time-optimizations-new-way-to-do-compiler-optimizations/
- 7: https://convolv.es/guides/lto/
- 8: https://community.nxp.com/t5/-/-/m-p/475825
- 9: https://forum.juce.com/t/link-time-optimization-flag-not-working-for-visual-studio/25127
- 10: https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html
Removed Link-Time Optimization from Release/MinSizeRel
The
-fltoflag was dropped from both Release and MinSizeRel in CMake/binutils.AzureRTOS.cmake (lines 12–14). Disabling LTO:
- Speeds up and simplifies builds (lower memory use, fewer toolchain surprises)
- Increases binary size (no cross-module dead-code elimination)
- May reduce runtime performance (no whole-program inlining or cache-locality optimizations)
Please verify that the resulting firmware size and performance remain within your embedded constraints before merging.
src/CLR/Core/CLR_RT_HeapCluster.cpp (8)
31-31: Good modernization to usenullptrinstead ofNULLThis change improves type safety and aligns with modern C++ standards.
34-35: Good modernization usingnullptrfor pointer initializationConsistently replacing
NULLwithnullptrthroughout the codebase enhances type safety.
39-39: Appropriate use ofnullptrfor pointer assignmentThis change is in line with modern C++ standards.
82-82: Good initialization withnullptrinstead ofNULLThis change improves code clarity and type safety.
112-113: Updated member variable naming conventionThe change from
m_locationtolocationandm_sizetosizein the heap structure accesses improves code consistency and readability by removing Hungarian notation.
132-135: Updated member naming convention for heap structureConsistent updates from
m_locationtolocationandm_sizetosizethroughout the codebase.
217-217: Good use ofnullptrin linked list manipulationThis change enhances type safety when managing linked list nodes.
288-288: Appropriate use ofnullptrfor tail node's next pointerThis change maintains consistency in the use of modern C++ practices.
src/CLR/Core/CLR_RT_Memory.cpp (5)
25-29: Updated member variable naming conventionThe change from
m_locationtolocationandm_sizetosizein the heap structure accesses improves code consistency and readability by removing Hungarian notation.
41-45: Appropriate member access patternConsistently accessing heap structure members with the updated naming convention.
48-48: Good modernization to usenullptrinstead ofNULLThis change improves type safety for return values.
130-130: Good initialization withnullptrinstead ofNULLInitializing pointer variables with
nullptris a modern C++ best practice.
201-205: Improved code structure with proper bracesAdding braces around the single-line if statement enhances code clarity and prevents potential bugs when the code is modified in the future.
src/CLR/CorLib/corlib_native_System_DateTime.cpp (3)
200-200: Updated type name fromCLR_DataTypetoNanoCLRDataTypeThis change aligns with the broader refactoring to standardize naming conventions throughout the codebase.
208-208: Appropriate use ofnullptrfor return valuesConsistently replacing
NULLwithnullptrenhances type safety across the codebase.Also applies to: 222-222, 243-243
238-238: Updated member variable naming conventionChanging from
m_datatodatain both the local object and well-known types improves code consistency by removing Hungarian notation.CMake/Modules/AzureRTOS_EFM32GG11_GCC_options.cmake (1)
65-65: Removed Link-Time Optimization flagsThe removal of
-flto -fuse-linker-pluginflags simplifies the build process for Release and MinSizeRel configurations. This change might help avoid potential compatibility issues with Link-Time Optimization.src/CLR/Core/Various.cpp (2)
92-92: Good modernization: NULL to nullptrThis change improves type safety by using the modern C++ nullptr keyword instead of the C-style NULL macro.
43-43: Well-known type reference updatedCorrectly updated from
g_CLR_RT_WellKnownTypes.m_Objecttog_CLR_RT_WellKnownTypes.Object, removing them_prefix as part of the codebase's modernization effort. This change aligns with other similar updates across the project and supports the PR's goal of fixing generic array creation.src/CLR/Core/CLR_RT_HeapBlock_Stack.cpp (3)
29-29: Good modernization: NULL to nullptrThis change improves type safety by using the modern C++ nullptr keyword instead of the C-style NULL macro.
61-61: Well-known type reference updatedCorrectly updated from
g_CLR_RT_WellKnownTypes.m_Objecttog_CLR_RT_WellKnownTypes.Object, removing them_prefix as part of the codebase's modernization effort. This change aligns with other similar updates across the project and supports the PR's goal of fixing generic array creation.
65-65: Improved sizeof usageGood simplification of
sizeof(struct CLR_RT_HeapBlock)to justsizeof(CLR_RT_HeapBlock), removing the redundantstructkeyword. This makes the code cleaner and more modern.src/CLR/Diagnostics/Profile.cpp (1)
23-23: Consistent modernization: NULL to nullptrAll NULL occurrences have been systematically replaced with nullptr throughout the file, which improves type safety and follows modern C++ best practices. This is a good change that makes the code more robust.
Also applies to: 42-43, 114-114, 172-172, 174-175, 178-178, 204-204
azure-pipelines-templates/publish-nanoclr.yml (1)
55-58: Enhanced CI/CD pipeline conditionThe condition for uploading nanoCLR to Cloudsmith has been expanded to include both 'develop' and 'main' branches, instead of just 'main'. This provides more flexibility in the deployment workflow and ensures that builds from the development branch can also be automatically published.
src/CLR/CorLib/corlib_native_System_BitConverter.cpp (2)
66-66: Well-known type member naming updated correctly.The code now uses the modern member naming convention by removing the
m_prefix from well-known type references. This change is consistent with the broader codebase modernization effort.Also applies to: 82-82, 102-102, 126-126, 146-146, 162-162, 178-178, 194-194, 210-210, 226-226
255-255: Modern C++ null pointer usage applied.Updated null pointer initialization from
NULLtonullptr, improving type safety and aligning with modern C++ standards.Also applies to: 280-280, 309-309, 340-340, 371-371, 401-401, 432-432, 463-463, 492-492, 521-521
src/CLR/CorLib/corlib_native_System_Reflection_RuntimeMethodInfo.cpp (4)
20-20: Method signature parser initialization updated correctly.Updated to use the new member naming convention by removing the
m_prefix from method descriptor members.
28-28: Well-known type reference modernized.The code now uses the modern member naming convention by removing the
m_prefix from TypeStatic.
42-42: Modernized null pointer usage.Updated NULL to nullptr in both code and comments to align with modern C++ standards.
Also applies to: 64-64
81-81: Well-known type reference updated correctly.Properly modernized the Object type reference by removing the
m_prefix.src/CLR/CorLib/corlib_native_System_String.cpp (2)
209-209: Modernized null pointer initialization.Updated all null pointer initializations from
NULLtonullptr, improving type safety and aligning with modern C++ standards.Also applies to: 255-255, 453-453, 608-609, 867-867, 908-908, 1046-1046, 1057-1057, 1062-1062, 1143-1143
212-212: Well-known type references updated correctly.The code now uses the modern member naming convention by removing the
m_prefix from well-known type references.Also applies to: 1221-1221
src/CLR/Diagnostics/Diagnostics_stub.cpp (2)
185-188: Added support for generic types in METHOD diagnostics.The
CLR_RT_DUMP::METHODfunction has been extended with an additional parameter to support generic types, which directly addresses the PR objective to properly handle generic type arrays. The unused parameter is correctly marked to avoid compiler warnings.This change is the key implementation for the PR objective to "allow the creation of an array for a generic type" as mentioned in the PR summary.
238-238: Modernized null pointer returns.Updated return statements to use
nullptrinstead ofNULLfor better type safety and consistency with modern C++ standards.Also applies to: 250-250, 407-407
src/CLR/Messaging/Messaging.cpp (1)
44-423: Code modernization improvements look goodThe changes throughout this file consistently replace NULL with nullptr and use auto * for cleaner type declarations, which aligns with modern C++ best practices. These improvements enhance type safety and readability without changing the code's functionality.
src/CLR/Core/Checks.cpp (2)
10-27: Good addition of stack validation for generic operationsThe new
VerifyStackOKmethod provides crucial validation for stack operations by checking both available space and pointer boundaries. This is essential for safely working with generic types and arrays, as it prevents stack overflows and invalid memory access during generic instantiation.
38-69: Consistent modernization of pointer comparisonsThe updates to use
nullptrinstead ofNULLimprove code consistency and follow modern C++ standards.src/CLR/CorLib/corlib_native_System_Reflection_RuntimeFieldInfo.cpp (1)
18-111: Consistent member naming convention improvementsThe changes in this file appropriately update member variable access by removing the
m_prefix and modernize NULL references to nullptr. This improves code readability and maintains consistency with the broader modernization effort across the codebase.src/CLR/Core/CLR_RT_HeapBlock_Finalizer.cpp (1)
19-23: Good consistency in member variable namingThe changes in this file continue the pattern of improving member variable naming by removing the
m_prefix and using more descriptive names (e.g.,firstMethod,virtualMethodCount,assemblyinstead ofm_methods_First,m_vMethods_Num,m_assm). This enhances code readability and maintains naming consistency throughout the codebase.src/CLR/CorLib/corlib_native_System_Threading_Thread.cpp (8)
51-52: Great modernization of NULL to nullptrThe code now uses
nullptrinstead ofNULL, which aligns with modern C++ practices and improves type safety.
171-172: Good modernization in commentThe comment now correctly refers to
nullptrinstead ofNULL, maintaining consistency with the code changes.
180-181: Good modernization of NULL to nullptrThe update improves code safety and readability by using C++11's
nullptrkeyword.
275-276: Good use of nullptr with logical conditionsThe condition is now written in a cleaner, more modern C++ way using
nullptr.
334-335: Good modernization of NULL to nullptrThe update improves code safety and readability by using C++11's
nullptrkeyword.
461-462: Good modernization in assertThe assertion now uses
nullptrinstead ofNULL, maintaining consistency throughout the codebase.
491-492: Well-known type reference updatedChanged from
g_CLR_RT_WellKnownTypes.m_Threadtog_CLR_RT_WellKnownTypes.Thread, removing the Hungarian notation prefix as part of the broader code modernization effort.
570-571: Good modernization of NULL to nullptrThe update improves code safety and readability by using C++11's
nullptrkeyword.src/CLR/CorLib/corlib_native_System_Guid.cpp (3)
14-21: Code formatting improved for consistencyFunction signature and variable declarations have been reformatted for better style consistency, improving readability.
23-24: Well-known type reference updatedChanged from
g_CLR_RT_WellKnownTypes.m_UInt8tog_CLR_RT_WellKnownTypes.UInt8, removing the Hungarian notation prefix as part of the broader code modernization effort.
27-28: Code simplificationThe NextBytes call is now written in a simpler, more direct form.
src/CLR/Include/nanoCLR_Runtime__Serialization.h (6)
73-74: Modernized NULL reference to nullptr in commentComment updated to use the C++11
nullptrinstead ofNULL. This maintains consistency with the code modernization throughout the codebase.
158-159: Data type name changeReturn type changed from
CLR_DataTypetoNanoCLRDataType, which appears to be part of a larger type renaming effort for consistency and clarity.
223-224: Improved variable namingChanged member name from
m_idxtom_indexfor better readability and consistency with the naming standards being applied across the codebase.
229-230: Improved variable namingChanged member name from
m_idxtom_indexfor better readability and consistency with the naming standards being applied across the codebase.
250-251: Improved parameter alignmentThe function signature has been reformatted for better alignment and readability, without changing functionality.
255-256: Improved parameter namingChanged parameter name from
idxtoindexfor better readability and consistency with the naming standards being applied across the codebase.src/CLR/Core/corhdr_private.h (4)
41-44: Improved enum formatting and commentsThe enum entries have been reformatted with aligned comments and consistent spacing, improving readability and maintainability.
47-65: Enhanced comments for element typesComments for various element types have been expanded and clarified, particularly for
ELEMENT_TYPE_VALUETYPE,ELEMENT_TYPE_CLASS,ELEMENT_TYPE_VAR,ELEMENT_TYPE_ARRAY,ELEMENT_TYPE_GENERICINST, andELEMENT_TYPE_TYPEDBYREF. These improvements help developers understand the purpose and usage of each type.
67-86: Improved formatting for element typesContinued formatting improvements with better aligned comments and spacing for integer types, function pointers, object types, arrays, and method variables.
88-112: Reorganized modifier types with consistent formattingThe modifier-related element types have been regrouped at the end of the enum with consistent comment formatting, improving code organization and readability.
src/CLR/CorLib/corlib_native_System_Reflection_FieldInfo.cpp (11)
28-28: Naming convention update: 'm_target' to 'target'This change aligns with the codebase's update to modern naming conventions by removing Hungarian notation prefixes.
39-42: Added missing braces and updated to use nullptrAdding braces around if statement bodies improves safety against future maintenance errors, and using
nullptrinstead ofNULLis a C++11 best practice providing type safety.
48-51: Added missing braces and updated to use nullptrSimilar improvement as above, adding braces for the if statement and using
nullptrfor proper null pointer checks.
55-60: Updated assertions to use nullptrUpdated assertions to use
nullptrinstead ofNULL, which follows modern C++ standards.
79-79: Updated assertion with nullptr comparisonModernized assertion using
nullptrfor better type safety.
119-119: Naming convention update: 'm_target->flags' to 'target->flags'Updated member access to align with the new naming convention removing the Hungarian notation prefix.
123-126: Added missing braces and updated to use nullptrAdded braces around if statement body and updated NULL comparison to use nullptr.
137-137: Updated member access: 'CrossReference().m_offset' to 'CrossReference().offset'Consistent with the codebase-wide update to modern naming conventions removing Hungarian notation prefixes.
149-149: Updated pointer initialization to use nullptrChanged
NULLtonullptrfor variable initialization, following modern C++ standards.
170-170: Updated comment to use nullptr terminologyUpdated comment to match the code's use of
nullptrinstead ofNULL.
187-187: Updated well-known type reference namingChanged from
g_CLR_RT_WellKnownTypes.m_Objecttog_CLR_RT_WellKnownTypes.Objectto match updated member naming conventions.src/CLR/System.Math/nf_native_system_math_System_Math.cpp (1)
219-219: Modernized variable declarations using autoChanged explicit float variable declarations to use
autowith type inference, making the code more concise while maintaining the same behavior through explicit casting. This is a C++11 feature that improves readability and maintainability.Also applies to: 260-260, 290-290, 320-320, 351-352, 382-382, 412-412, 442-442, 472-472, 503-504, 533-533, 563-563, 593-593, 623-623, 651-652, 688-688, 734-734, 775-775, 805-805, 835-835, 865-865, 895-895, 922-922
src/CLR/Include/nanoCLR_Profiling.h (3)
9-10: Added clang-format directivesAdded formatting directives to control clang-format processing of this file, which helps maintain consistent formatting across the codebase.
Also applies to: 224-225
24-26: Updated terminology in commentsChanged "typedef-idx" and "method-idx" to "typedef-index" and "method-index" in comments for consistency with code changes.
216-216: Updated type: CLR_IDX to CLR_INDEXChanged the type of
m_currentAssemblyfromCLR_IDXtoCLR_INDEXto align with type naming standardization across the codebase.src/CLR/Core/Thread.cpp (3)
50-53: Comprehensive nullptr modernization and code style improvementsMultiple updates throughout the file to:
- Replace
NULLwithnullptrfor all pointer comparisons and assignments- Add braces around single-statement if bodies for better safety
- Update variable names (e.g.,
numArgstoArgumentsCount) for clarity- Fix comments to use "nullptr" terminology
These changes follow modern C++ best practices, improve code safety and readability, and align with the broader code modernization effort across the codebase.
Also applies to: 62-65, 68-70, 87-87, 123-126, 225-225, 264-264, 330-346, 487-491, 505-505, 521-521, 614-619, 621-624, 644-644, 824-825, 1006-1006, 1029-1029, 1045-1045, 1055-1055, 1164-1164, 1209-1209, 1252-1252, 1391-1391, 1417-1421
182-182: Updated member access: 'm_target->numArgs' to 'target->argumentsCount'This change combines two naming convention updates: removing the Hungarian notation prefix and using a more descriptive variable name.
184-184: Updated member access naming patternChanged from
m_target->flagstotarget->flagsto align with the new naming convention.src/CLR/Core/CLR_RT_Interop.cpp (4)
149-149: LGTM: Comment updated for consistencyThe comment was updated to use
nullptrinstead ofNULL, which aligns with the modernization of the codebase to use C++11 features.
168-168: LGTM: Modernized pointer initializationUpdated to use
nullptrinstead ofNULLfor better type safety in C++11.
205-205: LGTM: Modernized pointer initializationUpdated to use
nullptrinstead ofNULLfor better type safety in C++11.
811-811: LGTM: Modernized pointer initializationUpdated to use
nullptrinstead ofNULLfor better type safety in C++11.src/CLR/CorLib/corlib_native_System_Convert.cpp (8)
62-62: LGTM: Modernized pointer initializationUpdated to use
nullptrinstead ofNULLfor better type safety in C++11.
530-530: LGTM: Modernized pointer initializationUpdated to use
nullptrinstead ofNULLfor better type safety in C++11.
542-542: LGTM: Modernized pointer comparisonUpdated pointer comparisons to use
nullptrinstead ofNULLfor better type safety in C++11.Also applies to: 548-548, 554-554, 560-560
596-599: LGTM: Modernized pointer initializations and type usageUpdated to use
nullptrinstead ofNULLfor pointer initializations and changed some variable types touint8_tfor improved type consistency.
621-621: LGTM: Modernized pointer comparisonUpdated to use
nullptrinstead ofNULLin conditional checks for better type safety.Also applies to: 740-740
700-700: LGTM: Modernized pointer comparisonUpdated to use
nullptrinstead ofNULLin conditional check for better type safety.
764-765: LGTM: Modernized code formattingUpdated code formatting while maintaining existing functionality.
964-964: LGTM: Modernized return valuesUpdated to return
nullptrinstead ofNULLacross numerous functions for better type safety in C++11.Also applies to: 986-986, 1010-1010, 1021-1021, 1030-1030, 1039-1039, 1048-1048, 1057-1057, 1066-1066, 1076-1076, 1094-1094
src/CLR/CorLib/corlib_native_System_TimeSpan.cpp (3)
202-202: LGTM: Updated data type nameUpdated the type from
CLR_DataTypetoNanoCLRDataType, which is part of the broader type system modernization.
210-210: LGTM: Modernized null pointer handlingUpdated to use
nullptrinstead ofNULLfor better type safety in C++11.Also applies to: 231-231
226-226: LGTM: Updated member variable naming conventionUpdated to use
datainstead ofm_data, removing the Hungarian notation prefix as part of the codebase modernization.src/CLR/CorLib/corlib_native_System_AppDomain.cpp (5)
16-16: LGTM: Modernized pointer initializationsUpdated to use
nullptrinstead ofNULLfor pointer initializations, improving type safety in C++11.Also applies to: 104-104, 107-107, 262-262
74-74: LGTM: Modernized pointer comparisonsUpdated to use
nullptrinstead ofNULLin function parameters and conditional checks for better type safety.Also applies to: 120-120
40-40: LGTM: Improved variable namingUpdated variable names from
idxtoindexand related access patterns for improved readability and consistency throughout the codebase.Also applies to: 84-84, 89-89, 275-275, 276-276, 282-282
229-229: LGTM: Updated member variable naming conventionUpdated to remove the
m_prefix from member variables and well-known types, aligning with modern C++ naming conventions.Also applies to: 279-279, 291-291
249-249: LGTM: Improved preprocessor directive commentsEnhanced preprocessor directive comments for improved code readability.
Also applies to: 333-333
src/CLR/Core/Serialization/BinaryFormatter_stub.cpp (5)
19-19: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.
27-27: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.
72-77: Type system update: CLR_DataType to NanoCLRDataType.Good update to the return type from
CLR_DataTypetoNanoCLRDataTypewith corresponding cast update. This aligns with the type system enhancements for generics support.
351-353: Improved naming convention.Good change from
idxtoindexfor better code readability and consistency with modern naming conventions. Also properly documenting unused parameter with(void)index.
356-356: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.src/CLR/Core/CLR_RT_HeapBlock_Timer.cpp (5)
15-15: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.
102-104: Code modernization: NULL to nullptr check.Good replacement of the legacy C-style
NULLwith modern C++nullptrin the assertion and condition check.
118-118: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.
120-124: Improved naming convention.Good update from Hungarian-style
numArgsto more descriptiveArgumentsCount. The changes also correctly update the related member accesses.
248-248: Macro usage consistency.Good replacement of
FAULT_ON_NULL_ARGwithFAULT_ON_NULLfor delegate pointer validation, ensuring consistency in error handling.src/CLR/CorLib/corlib_native_System_Exception.cpp (13)
15-30: Member name standardization.Good update to remove the
m_prefix from well-known type members in the exception lookup entries. This improves code readability and consistency with the broader naming convention changes in the codebase.
53-53: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.
69-71: Improved code structure.Good addition of braces around the single-line conditional to improve code clarity and prevent potential future bugs if more statements are added.
73-73: Member name standardization.Good update to remove the
m_prefix from well-known type member. This improves code readability and consistency.
83-83: Method signature update.Good update to the
BuildMethodNamecall to include an additionalnullptrparameter, aligning with changes to the method signature.
124-124: Member name standardization.Good update to remove the
m_prefix from well-known type member inIsInstanceOfcall.
160-160: Member name standardization.Good update to remove the
m_prefix from well-known type member. This improves code readability and consistency.
190-190: Member name standardization.Good update to remove the
m_prefix from well-known type member inIsInstanceOfcall.
209-209: Member name standardization.Good update to remove the
m_prefix from well-known type member inCreateInstancecall.
225-225: Member name standardization.Good update to remove the
m_prefix from well-known type member inCreateInstancecall.
257-257: Member name standardization.Good update to remove the
m_prefix from well-known type member inCreateInstancecall.
268-268: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity in the return statement.
290-290: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity in the return statement.src/CLR/Include/nanoCLR_Interop.h (8)
21-21: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity.
88-92: Enhanced macro flexibility.Good update to the
NANOCLR_MSG1_SET_AND_LEAVEmacro to support variadic arguments (...) instead of a single argument, allowing more flexible formatting.
207-207: Updated comment style.Good update to use
nullptrinstead ofNULLin the comment for consistency with the code.
301-301: Code modernization: NULL to nullptr.Good replacement of the legacy C-style
NULLwith modern C++nullptrfor better type safety and clarity in the constructor initialization.
522-522: Updated comment style.Good update to use
nullptrinstead ofNULLin the comment for consistency with the code.
552-552: Updated comment style.Good update to use
nullptrinstead ofNULLin the comment for consistency with the code.
586-586: Updated comment style.Good update to use
nullptrinstead ofNULLin the comment for consistency with the code.
671-671: Improved naming convention.Good update from
assmIdxandmethodIdxto more descriptiveassmIndexandmethodIndexfor better code readability and consistency with modern naming conventions.src/CLR/CorLib/corlib_native_System_RuntimeType.cpp (12)
16-16: Modernized code by replacing NULL with nullptrThis change follows modern C++ best practices by using
nullptrinstead ofNULL. This is part of a broader effort to modernize the codebase with C++11 features.
19-20: Improved naming consistencyChanged variable name from
idxto more descriptiveindexand updated member access style by removingm_prefix fromAssembly(). These changes improve code readability and maintain consistent naming conventions.
24-26: Updated well-known type reference and reflection setterModernized access to well-known types by removing
m_prefix and simplified the reflection setter to use the index object directly.
86-96: Modernized function parameters with nullptrUpdated function call parameters to use
nullptrinstead ofNULLfor better type safety and code clarity.
114-114: Simplified function call with nullptrUpdated function call to use
nullptrinstead ofNULLfor the string parameter, consistent with modern C++ practices.
124-124: Modernized pointer initializationReplaced
NULLwithnullptrfor initial pointer value, which is a safer and more type-aware alternative in modern C++.
137-137: Updated member access styleChanged member access from
td.m_assmtotd.Assembly()andtd.m_targettotd.target, maintaining the new naming convention throughout the codebase.
154-157: Updated well-known type reference and reflection setterModernized access to well-known types and simplified the reflection setter with consistent naming conventions.
272-292: Modernized pointer usage in CustomAttributesNativeUpdated null pointer references with
nullptrand improved comments to match modern C++ practices.
361-361: Updated null check with nullptrChanged
val == NULLtoval == nullptrfor consistent null pointer checking throughout the codebase.
378-379: Modernized object reference handlingUpdated to use
nullptrfor null object references and aligned with the new coding style for object handling.
388-388: Updated assembly and method index accessModified access path from
instanceTypeDef.m_assm->m_idxtoinstanceTypeDef.assembly->assemblyIndexand updated method index access, maintaining the new naming conventions.src/CLR/Startup/CLRStartup.cpp (9)
72-76: Modernized member access and null pointer checksUpdated member access from
assm->m_szNametoassm->nameand improved null pointer checks usingnullptrinstead ofNULL. This change follows modern C++ practices and makes the code more consistent with the rest of the codebase.
79-92: Updated member variable naming conventionsChanged member access patterns to follow the new naming convention by removing the
m_prefix. This makes the code more consistent with modern C++ coding standards.
101-101: Updated native code member accessChanged from
assm->m_nativeCodetoassm->nativeCodeto align with the new naming conventions throughout the codebase.
191-209: Modernized pointer initialization and null checksUpdated pointer initialization to use
nullptrand modified null checks to use the more type-safenullptrcomparison instead ofNULL.
246-252: Improved null pointer checkingUpdated pointer check to use
nullptrinstead ofNULLfor better type safety and consistency with modern C++ practices.
300-307: Updated flag setting with new enumeration namesChanged from using flag constants like
c_Deployedto enum values likeDeployed, making the code more readable and maintainable by using the actual flag names.
316-344: Modernized pointer handling in free region loopUpdated all NULL references to
nullptrfor consistent pointer handling throughout the codebase. This follows modern C++ best practices.
403-403: Updated function call parameter to nullptrChanged from passing
NULLto passingnullptrin theExecutefunction call, consistent with modern C++ practices.
422-422: Updated event broadcast with nullptr parameterChanged from passing
NULLto passingnullptrin theCLR_EE_DBG_EVENT_BROADCASTfunction call, consistent with the rest of the codebase updates.src/CLR/Core/GarbageCollector_Compaction.cpp (7)
129-129: Modernized pointer assignmentUpdated from
NULLtonullptrfor pointer assignment, following modern C++ best practices.
316-320: Updated null checks in free region handlingChanged from using
NULLtonullptrfor null checks and assignments, maintaining consistency with the rest of the codebase.
331-337: Improved pointer handling with nullptrUpdated pointer checks and assignments to use
nullptrinstead ofNULLfor better type safety and consistent code style.
449-449: Updated function call with nullptr parameterChanged from passing
NULLto passingnullptrin theHeap_Relocate_Passfunction call, consistent with modern C++ practices.
483-489: Added important pointer boundary checksAdded critical assertions to verify that pointers lie within valid heap memory boundaries. These checks help prevent memory corruption and improve debugging by catching pointer issues early.
494-495: Enhanced pointer validation within loopAdded assertions to ensure that the pointer remains within valid heap boundaries during iteration, strengthening memory safety.
590-591: Added validation for relocated addressesAdded important assertions to verify that the newly computed destination address remains within valid heap boundaries, preventing potential memory corruption during relocation.
src/CLR/Core/CLR_RT_HeapBlock_GenericInstance.cpp (2)
1-6: Proper file header with licensing informationThe file includes the standard copyright and license header used across the nanoFramework codebase, ensuring proper attribution and licensing information.
21-79: Commented-out implementation codeThe file contains extensive commented-out code that appears to be adapted from delegate implementation, suggesting how generic instances might be implemented in the future.
This commented code serves as a reference for future implementation. Before finalizing, ensure the actual implementation is appropriate for generic instances rather than delegates. The commented code seems to be based on delegate implementation patterns.
src/CLR/Core/CLR_RT_StackFrame.cpp (8)
34-36: Code modernization: Field naming convention changesThe field naming convention in CoreCLR structures has been updated by removing the Hungarian notation prefix
m_from member variables. This change is consistent with modern C++ coding standards.
113-114: Code modernization: NULL replaced with nullptrUpdated NULL pointers to use the C++11 standard nullptr keyword, improving type safety and code clarity.
158-163: Member renaming for consistency: RVA changed to rvaThe method definition's RVA (Relative Virtual Address) member has been renamed for consistency with the overall naming convention changes throughout the codebase.
189-200: Improved variable naming for clarityRenamed
flagstosyncFlagsto better indicate the variable's purpose, improving code readability.
254-257: Consistent method naming: argumentsCountUpdated parameter naming from
numArgstoargumentsCountfor consistency with other naming changes throughout the codebase.
345-348: Member renaming: retVal to retValDataTypeThe method definition's return value datatype member has been renamed from
retValtoretValDataTypeto better express its purpose.
942-950: Modernized conditional check with nullptrUpdated the null check on the exception dereference to use nullptr, providing better type safety and adherence to modern C++ standards.
1292-1301: Updated return type for clarity in NotImplementedStub methodThe method signature is now more consistent with the naming convention changes. The check for
retValDataTypeinstead ofretValaligns with the renaming patterns throughout the file.src/CLR/Core/GarbageCollector.cpp (4)
10-11: Code modernization: Static member initialization with nullptrUpdated NULL initialization to use the C++11 standard nullptr keyword, improving type safety.
517-518: Code modernization: Null pointer handlingUpdated NULL assignment to use the C++11 standard nullptr keyword, maintaining the pattern throughout the codebase.
708-709: Updated member name references in CheckMultipleBlocks callsChanged
numArgstoargumentsCountandnumLocalstolocalsCountto align with the renaming convention applied throughout the codebase.
755-759: Consistent member naming in inlined frame handlingUpdated the member references in
CheckMultipleBlockscalls for inline frames to match the renamed member variables in the target structures.src/CLR/Core/CLR_RT_HeapBlock_Array.cpp (7)
27-30: Updated member access pattern in ReflectionDef_IndexChanged
m_kindtokindto follow the new naming convention that removes Hungarian notation prefixes from member variables.
32-38: Consistent member field naming in ReflectionDef_IndexUpdated member accesses from
m_levelstolevelsandm_data.m_typetodata.typefollowing the field renaming pattern throughout the codebase.
47-48: Updated data type name and member accessChanged
CLR_DataTypetoNanoCLRDataTypeandm_targettotarget, reflecting both the type rename and member naming convention update.
85-91: Added caller parameter to enable generic array creationAdded
const CLR_RT_MethodDef_Instance *callerparameter to theCreateInstancemethod, addressing the main issue in this PR for supporting generic type arrays.
101-112: Using the new caller parameter in ResolveTokenThe added
callerparameter is now passed toResolveToken, allowing proper resolution of generic type tokens by providing the necessary method context.
133-135: Consistent type and member namingChanged
CLR_DataTypetoNanoCLRDataTypeand updated member access patterns (m_levelstolevels), maintaining the consistent renaming conventions.
156-157: Updated member access in NewObjectFromIndex callChanged
reflex.m_data.m_typetoreflex.data.typeto align with the field renaming conventions throughout the codebase.src/CLR/Diagnostics/Profiler.cpp (9)
28-33: Modern pointer initialization and castingUpdated the null initialization to use nullptr instead of NULL, and used proper casting for the timestamp calculation. These changes improve type safety and code clarity.
67-71: Updated heap member access patternsChanged
s_CLR_RT_Heap.m_locationtos_CLR_RT_Heap.locationandm_sizetosize, consistently applying the member naming conventions.
163-173: Modernized function parameter passingUpdated the last parameter in all
DumpRootcalls from NULL to nullptr, maintaining the pattern of using modern C++ nullptr throughout the codebase.
258-259: Updated data type nameChanged
CLR_DataTypetoNanoCLRDataType, maintaining consistency with the type renaming pattern throughout the codebase.
339-341: Consistent member access in ReflectionDataUpdated member access from
m_data.m_typetodata.typeandm_levelstolevelsin array reflection data, following the consistent renaming pattern.
351-355: Modern C++ type declaration and member accessUsing
auto*for cleaner type declarations and updated member access patterns for consistency with the naming conventions throughout the codebase.
404-406: Updated member variable accesses in stack frameChanged
m_call.m_target->numArgstom_call.target->argumentsCountandnumLocalstolocalsCountto maintain consistency with the naming conventions.
579-582: Improved type casting for timestamp calculationEnhanced readability and correctness of the timestamp calculation by using explicit type casts and properly accounting for bit shifts.
1425-1426: Modern C++ pointer declarationUsing
auto*for pointer declaration instead of explicit C-style cast, improving code readability and maintainability.src/CLR/Core/CLR_RT_HeapBlock.cpp (1)
297-320: Validate reflection construction for generic parametersNice addition! Please double-check that
genericParam.CrossReference().classTypeDefalways carries the concrete type definition you intend to expose through the reflection object:
- For an un-bound generic type parameter (
!0,!1, …) the cross-reference usually points to the declaring type, not to the constraint (if any).- If a constraint exists, the runtime may expect
REFLECTION_TYPEto reference that constrained type instead of the declaring one.A quick sanity-check (e.g. boxing the resulting
System.Typefrom managed code) would confirm that the correct type metadata is surfaced.src/CLR/Core/Serialization/BinaryFormatter.cpp (1)
70-73: Early reset prevents stale pointers – good catchClearing
m_value/m_typebefore the null-check avoids leaking stale references when the supplied heap-block turns out to benull. Looks correct to me.src/CLR/Core/TypeSystemLookup.cpp (2)
38-41: Confirm flag set for new generic data-types.
DATATYPE_VAR,DATATYPE_GENERICINST, andDATATYPE_MVARare created withoutDT_REF.
If these heap blocks ever hold references, the GC won’t trace them, causing leaks or crashes.Please verify whether these types should include
DT_REF(and possiblyDT_CLASS) in theirflags.
99-102:❓ Verification inconclusive
Enum / lookup-table synchronisation.
The three new rows assume the underlying enum values appear exactly here.
Ifenum DataTypewas not updated to insert the same entries at the matching positions, the lookup table and runtime will diverge (wrong metadata → hard-to-trace faults).Double-check the
DATATYPE_*enumeration innanoCLR_Runtime.hand regenerate this table if needed.
🏁 Script executed:
#!/bin/bash # Locate and inspect enum DataType to verify the new rows align with the enum order header=$(fd -t f nanoCLR_Runtime.h | head -n1) echo "Inspecting enum DataType in: $header" rg -n 'enum DataType' -A100 "$header"Length of output: 282
🏁 Script executed:
#!/bin/bash # Search the entire repository for DATATYPE_ enum entries to locate and inspect the enum definition rg -n 'DATATYPE_' -C3Length of output: 72222
🏁 Script executed:
#!/bin/bash # Inspect the DataType enum definition in the runtime header to verify member order header="src/CLR/Include/nanoCLR_Runtime.h" echo "Showing 20 lines before and 10 lines after DATATYPE_VAR in $header" rg -n 'DATATYPE_VAR' -B20 -A10 "$header"Length of output: 302
Verify enum/lookup‐table synchronization
Please ensure that the DataType enumeration in src/CLR/Include/nanoCLR_Runtime.h includes DATATYPE_VAR, DATATYPE_GENERICINST and DATATYPE_MVAR in the exact positions that match the entries added in the lookup table at src/CLR/Core/TypeSystemLookup.cpp (lines 99–102). If the enum wasn’t updated alongside these new rows, regenerate the c_CLR_RT_DataTypeLookup table to prevent metadata mismatches.
• TypeSystemLookup.cpp: new rows at lines 99–102
• nanoCLR_Runtime.h: enum DataType definition (verify ordering and positions of the three entries)src/CLR/CorLib/corlib_native_System_GC.cpp (1)
84-94: Possible GC statistics drift when compacting.
Run()decrementsm_numberOfGarbageCollectionsimmediately before a compaction.
IfPerformHeapCompaction()triggers an internal GC, the counter may underflow or become inconsistent.Consider letting
PerformHeapCompaction()adjust the counter itself, or capture the return value ofPerformGarbageCollection()after all optional actions are complete.src/CLR/CorLib/corlib_native.h (4)
598-603: GC method reorganization and additionsThe GC methods have been reordered and new functionality has been added with
EnableGCMessagesandRunmethods. This enhances the garbage collection capabilities to better support generic instance management.
657-660: Updated type names to improve API consistencyType names have been updated from
CLR_DataTypetoNanoCLRDataTypeto align with the updated type system definitions. This change improves naming consistency across the codebase.
666-667: Updated return type to match the renamed data typeThe return type was updated to use the renamed
NanoCLRDataTypeto ensure consistency with the updated type system.
998-1003: Repositioned UInt32 struct declarationThe
Library_corlib_native_System_UInt32struct declaration was removed and re-added without changes. This reorganization helps maintain a consistent structure in the file.azure-pipelines-templates/check-mdp-for-build.yml (1)
1-94:✅ Verification successful
Well-structured pipeline template for conditional MDP download and installation
This new template provides a clean, modular approach to conditionally download and deploy specific MDP builds for PR builds. The implementation correctly:
- Extracts the MDP build ID from the PR description
- Downloads the MDP artifact only when needed
- Copies it to the MSBuild extensions directory for nanoFramework
🌐 Web query:
What is MDP (Metadata Processor) in the context of .NET nanoFramework?💡 Result:
The Metadata Processor (MDP) is a crucial tool in the .NET nanoFramework ecosystem that transforms standard .NET assemblies into a format compatible with nanoFramework's execution environment on microcontrollers. It serves as a bridge between the standard .NET compilation process and the specific requirements of the nanoFramework runtime.
Core Functions of the Metadata Processor
The Metadata Processor performs several essential tasks during the build process of a C# project for nanoFramework:
- Parses the PE (Portable Executable) files generated by the Roslyn compiler[1]
- Generates PDBX files for debugging purposes[1]
- Creates stubs for native code integration[1]
- Transforms standard .NET assemblies with Intermediate Language (IL) code into Portable Executable (PE) code that can be interpreted by nanoFramework's execution engine[5]
The tool effectively strips away unnecessary elements from the standard .NET assemblies, optimizing them for the constrained resources of microcontroller units (MCUs). For example, it reduces enums to their strict minimum, essentially converting them to simple numbers[5].
Availability and Implementation
The Metadata Processor is available in two main packages:
MSBuild Task Package: The primary implementation is packaged as
nanoFramework.Tools.MetadataProcessor.MsBuildTask, which integrates directly into the .NET nanoFramework build system and is distributed with Visual Studio extensions. This is the most common way developers interact with the Metadata Processor[1][2].CLI Package: For specialized use cases where the build task cannot be used (such as runtime code generation), a command-line interface version is available as
nanoFramework.Tools.MetadataProcessor.CLI. This version is packaged as content that can be distributed with projects[1].The current version (2.0) is a C# application adapted from the original work of Oleg Rakhmatulin, replacing the previous version (1.0) which was a Visual C++ application adapted from .NETMF toolbox[1].
Role in the nanoFramework Architecture
The Metadata Processor plays a vital role in the nanoFramework's architecture by enabling C# code to run on resource-constrained microcontrollers. .NET nanoFramework is designed to run on devices with extremely limited resources - sometimes as little as 520KB of SRAM with a 240MHz processor[7].
Within this architecture, the Metadata Processor ensures that the compiled .NET code is optimized for these constraints while still maintaining the core functionality that allows developers to use familiar C# syntax and .NET patterns. This transformation is essential for making the framework viable on microcontroller platforms like ESP32, STM32, and other ARM Cortex-M based processors[7][9].
Citations:
- 1: https://github.com/nanoframework/metadata-processor
- 2: https://www.nuget.org/packages/nanoFramework.Tools.MetadataProcessor.MsBuildTask/
- 3: https://github.com/nanoframework/metadata-processor/blob/main/.editorconfig
- 4: https://nanoframework.net
- 5: https://nanoframework.net/vs-code-extension-for-net-nanoframework/
- 6: https://www.youtube.com/watch?v=TLYqRdmmj5k
- 7: https://devblogs.microsoft.com/dotnet/show-dotnet-build-your-own-unit-test-platform-the-true-story-of-net-nanoframework/
- 8: https://www.youtube.com/watch?v=pxXgU3XcRr4
- 9: https://learn.microsoft.com/en-us/samples/dotnet/samples/compare-nanoframework-with-dotnet-core-iot/
- 10: https://docs.nanoframework.net/api/System.html
Approve conditional MDP pipeline in azure-pipelines-templates/check-mdp-for-build.yml
The Metadata Processor (MDP) transforms standard .NET assemblies into a format compatible with the .NET nanoFramework runtime on microcontrollers. This pipeline template cleanly and conditionally handles MDP builds for pull-request validation:
- Extracts the MDP build ID from the PR description
- Downloads the MDP artifact only when
DownloadMDPis true and a valid build ID is present- Copies the resulting DLLs into the MSBuild nanoFramework extensions directory
No further changes needed.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
src/CLR/Include/nanoCLR_Checks.h (4)
11-12: Added clang-format directives to preserve code layoutThe addition of
clang-format off/onmarkers helps maintain consistent formatting while preserving intentional code layout in this section. This is especially important for maintaining the readability of aligned function declarations.Also applies to: 72-73
24-25: Simplified DECL_POSTFIX macro for cleaner codeThe
DECL_POSTFIXmacro has been simplified to a single-line empty block{}, which improves code readability while maintaining the same functionality.Also applies to: 41-42
27-34: Improved method declaration alignment and added support for genericsMethod declarations have been aligned for better readability, and the
METHODfunction now accepts an additional parameter for generic type support. The newMETHODREFandMETHODSPECmethods have been added to support dumping of method references and specifications.
56-57: Improved const-correctness in VerifyStackOK methodThe
VerifyStackOKmethod now usesconstreferences and pointers for better type safety and const-correctness, which is an important improvement for a verification method.azure-pipelines.yml (5)
59-144: Enhanced build options detection for PR buildsAdded a new
Check_Build_Optionsjob that intelligently analyzes PR details to determine build actions. This improvement allows for conditional builds based on PR content and supports the new MDP-specific build option which aligns with the generics support objective.
129-134: Added support for building with specific MDP versionsThis addition allows developers to specify a particular MDP build to use via a PR description tag
[build with MDP buildId NNN]. This is a valuable feature for testing compatibility with specific MDP versions.
438-438: Updated version counters to 2.0.0Version counters for all build jobs (STM32, ESP32, NXP, TI SimpleLink, Azure RTOS, WIN32, nanoCLR CLI) have been updated to use 2.0.0 versioning, indicating a major release with significant changes like the generics support.
Also applies to: 553-553, 641-641, 720-720, 792-792, 844-844
1007-1015: Extended NuGet package publishing to include develop branchNuGet package push tasks now target both
mainanddevelopbranches, improving the development workflow by making packages available earlier in the development cycle.Also applies to: 1179-1188
1238-1238: Added MDP integration for mscorlib unit testsThe
Run_UnitTests_mscorlibjob now includes support for downloading and using specific MDP builds, and enables use of preview MSBuild components. This ensures compatibility with the generics support being added.Also applies to: 1264-1266
src/CLR/Diagnostics/Info.cpp (1)
409-413:❌ Incorrect review comment
Potential undefined identifier:
assemblyIndexinside new macro
LOOKUP_ELEMENT_IDXnow callss.Set(assemblyIndex, idx)but neither the macro norDumpToken()declares a variable namedassemblyIndex.
UnlessassemblyIndexis a class data-member or another visible identifier, this will not compile.- CLR_RT_##tblName##_Index s; \ - s.Set(assemblyIndex, idx) + CLR_RT_##tblName##_Index s; \ + s.Set(this->m_idx, idx) /* or whatever represents the assembly index */Please double-check that a valid symbol is in scope, or replace it with the correct assembly identifier.
To confirm, you can grep the codebase:
🏁 Script executed:
#!/bin/bash # Find all definitions of 'assemblyIndex' rg --type cpp --type c --type h --line-number --word-regexp '\bassemblyIndex\b'Length of output: 5348
#!/bin/bash set -e echo "=== Snippet around LOOKUP_ELEMENT_IDX macro definition ===" # Show lines around the macro definition in Info.cpp sed -n '390,430p' src/CLR/Diagnostics/Info.cpp echo echo "=== Occurrences of LOOKUP_ELEMENT_IDX in Info.cpp ===" rg --line-number "LOOKUP_ELEMENT_IDX" src/CLR/Diagnostics/Info.cpp || true echo echo "=== DumpToken function signature and context ===" rg --context 3 --line-number "DumpToken" src/CLR/Diagnostics/Info.cpp
#!/bin/bash set -e echo "=== DumpToken signature and nearby lines ===" rg --context 5 --line-number "DumpToken" src/CLR/Diagnostics/Info.cpp echo echo "=== LOOKUP_ELEMENT_IDX usage context ===" rg --context 5 --line-number "LOOKUP_ELEMENT_IDX" src/CLR/Diagnostics/Info.cpp echo echo "=== assemblyIndex occurrences around DumpToken ===" rg --context 5 --line-number "assemblyIndex" src/CLR/Diagnostics/Info.cpp
#!/bin/bash set -e echo "=== DumpToken definition and parameters ===" rg --context 3 --line-number "DumpToken" src/CLR/Diagnostics/Info.cpp echo echo "=== Macro context: lines 390–430 in Info.cpp ===" sed -n '390,430p' src/CLR/Diagnostics/Info.cpp
#!/bin/bash set -e echo "=== DumpToken definition and surrounding context ===" rg --context 5 --line-number "DumpToken" src/CLR/Diagnostics/Info.cpp
No change needed:
assemblyIndexis in scope
TheLOOKUP_ELEMENT_IDXmacro is invoked insideDumpToken, which declaresassemblyIndexas a parameter. The identifier is therefore valid and the code compiles as written.Likely an incorrect or invalid review comment.
src/CLR/Include/nanoCLR_Runtime__HeapBlock.h (1)
1038-1043: Dereferencing BYREF may raise null-pointer riskWhen
dstis aDATATYPE_BYREF,Dereference()can legally returnnullptr.
SetReference()forwards that pointer without a check, so later uses of the reference may fault.Consider guarding against a null dereference or propagating an error instead of silently storing
nullptr.src/CLR/Core/Interpreter.cpp (3)
2431-2440: Constructor path duplicates generic-instance allocation logic – looks solidThe extra branch that calls
NewGenericInstanceObject()whencalleeInst.genericType != nullptrcleanly separates classic and constructed types.
Nice job keeping the fallback toNewObjectuntouched.
427-453: Assert comments aid maintainability – keep themThe added
_ASSERTElines documenting invariants forfrom/tosimplify reasoning aboutFindEhBlockand will help future debugging.
123-131: Signature modernisation OKRenaming parameters from
CLR_DataTypetoNanoCLRDataTypemaintains type-safety. No functional issues spotted.src/CLR/Include/nanoCLR_Types.h (2)
109-118: Validate 16-bit index typedefs against enlarged tables
CLR_INDEX,CLR_TYPEORMETHODDEF,CLR_EncodedTypeDefOrRef, etc. remain 16-bit (uint16_t).
With the newGenericParamandMethodSpectables, the total number of rows per table can now exceed 65 535 in larger builds – which would silently overflow these indices.Please check the tool-chain (MDP, VS extension, runtime) for any scenarios where a single table can grow beyond 64 K entries. If that is possible, the typedefs should be widened to 32 bit (or made configurable) to prevent memory corruption.
38-44: Nice modernization of null-pointer checksSwitching from manual
NULLcomparisons to the modernptr == nullptridiom improves type-safety and helps static analyzers. 👍src/CLR/Include/nanoCLR_Runtime.h (1)
619-630: Shift-width changed to 24 bits – verify all metadata writersAll
Set()/Assembly()helpers now encode the assembly index in the top 8 bits (<< 24).
Please ensure:
- Metadata emitter tooling has been updated accordingly.
CLR_INDEXis at least 24 bits wide (it is currently anint16_tin some ports).- No existing images are loaded with the new runtime, otherwise they will mis-decode tokens.
If any of these assumptions fail, introduce compile-time/static assertions to catch it early.
| HRESULT CLR_RT_HeapBlock_GenericInstance::CreateInstance( | ||
| CLR_RT_HeapBlock &reference, | ||
| const CLR_RT_TypeSpec_Index &tsIndex) | ||
| { | ||
| NATIVE_PROFILE_CLR_CORE(); | ||
| NANOCLR_HEADER(); | ||
|
|
||
| (void)reference; | ||
| (void)tsIndex; | ||
|
|
There was a problem hiding this comment.
Skeleton implementation of CreateInstance method
The method signature is defined with proper parameters for creating generic instances, but currently ignores its parameters with void casts and lacks actual implementation.
This is a stub implementation that needs to be completed. The parameters are currently ignored with void casts, which is appropriate for work-in-progress code but must be properly implemented before release.
| void CLR_RT_HeapBlock_GenericInstance::Relocate() | ||
| { | ||
| NATIVE_PROFILE_CLR_CORE(); | ||
| // m_object.Relocate__HeapBlock(); | ||
| } |
There was a problem hiding this comment.
Stub implementation of Relocate method
The method is defined but contains only a commented-out call to relocate an internal object reference, indicating future support for heap block relocation.
This is a critical stub that needs implementation. Proper object relocation is essential for the garbage collector to work correctly with generic instances. Uncomment and complete the implementation before release.
***NO_CI***
***NO_CI***
***NO_CI***
…into develop ***NO_CI***
- Required to properly created array instance with generic type.
josesimoes
force-pushed
the
fix-create-generic-array
branch
from
4fb7ef7 to
f9a229d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/CLR/Include/nanoCLR_Types.h (1)
640-643: Implicituint8_t→NanoCLRDataTypecast risks truncation
CLR_UncompressElementTypereturns*(uint8_t*)which is fine for ECMA element-types (< 0x60).
But enumNanoCLRDataTypenow continues past 0x60, even jumping to 0x40/0x45 for modifiers. If a future encoder stores any value ≥ 0x80, the cast will wrap.Safer to make the return type the underlying 8-bit storage and convert only after range-checking:
-inline NanoCLRDataType CLR_UncompressElementType(const CLR_UINT8 *&p) +inline CLR_UINT8 CLR_UncompressElementType(const CLR_UINT8 *&p) { - return (NanoCLRDataType)*p++; + return *p++; }or assert
val < DATATYPE_FIRST_INVALID.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/CLR/CorLib/corlib_native_System_Guid.cpp(2 hunks)src/CLR/CorLib/corlib_native_System_Reflection_MethodBase.cpp(8 hunks)src/CLR/Core/CLR_RT_HeapBlock.cpp(36 hunks)src/CLR/Core/CLR_RT_HeapBlock_Array.cpp(11 hunks)src/CLR/Core/CLR_RT_Interop.cpp(5 hunks)src/CLR/Core/Execution.cpp(81 hunks)src/CLR/Core/Interpreter.cpp(55 hunks)src/CLR/Debugger/Debugger.cpp(119 hunks)src/CLR/Include/nanoCLR_Runtime.h(48 hunks)src/CLR/Include/nanoCLR_Runtime__HeapBlock.h(27 hunks)src/CLR/Include/nanoCLR_Types.h(23 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/CLR/CorLib/corlib_native_System_Guid.cpp
- src/CLR/Core/CLR_RT_Interop.cpp
🚧 Files skipped from review as they are similar to previous changes (6)
- src/CLR/CorLib/corlib_native_System_Reflection_MethodBase.cpp
- src/CLR/Core/CLR_RT_HeapBlock.cpp
- src/CLR/Core/CLR_RT_HeapBlock_Array.cpp
- src/CLR/Include/nanoCLR_Runtime__HeapBlock.h
- src/CLR/Core/Interpreter.cpp
- src/CLR/Include/nanoCLR_Runtime.h
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/CLR/Debugger/Debugger.cpp (10)
src/CLR/Include/nanoCLR_Runtime__HeapBlock.h (28)
dst(1034-1059)dst(1034-1034)dst(2554-2554)dst(2556-2556)dst(2559-2559)dst(2560-2560)dst(2561-2561)cmd(2212-2212)size(2777-2780)size(2777-2777)size(2854-2858)size(2854-2854)size(2897-2901)size(2897-2897)reference(1817-1817)reference(1818-1818)reference(1819-1819)reference(1820-1820)appDomain(1025-1025)fd(1352-1352)md(1353-1353)res(1632-1636)res(1637-1641)src(1967-1967)index(1854-1857)index(1854-1854)offset(1392-1392)szBuffer(2640-2640)src/CLR/Include/nanoCLR_Runtime.h (196)
dst(1140-1140)dst(1141-1141)dst(2949-2949)call(1562-1562)buffer(546-546)data(614-617)data(638-641)data(667-670)data(696-699)data(713-716)data(725-728)data(754-757)data(783-786)data(812-815)data(859-859)data(1393-1393)data(1399-1399)data(2800-2800)data(4077-4077)size(1143-1143)size(2368-2368)i(1465-1465)i(1482-1485)i(1482-1482)i(1486-1489)i(1486-1486)i(1490-1493)i(1490-1490)i(1494-1497)i(1494-1494)i(1498-1501)i(1498-1498)i(1502-1505)i(1502-1502)i(1506-1509)i(1506-1506)i(1510-1513)reference(4029-4029)reference(4030-4030)reference(4031-4031)reference(4033-4036)reference(4037-4040)reference(4042-4042)reference(4048-4048)reference(4049-4049)reference(4050-4050)reference(4051-4051)reference(4053-4053)reference(4054-4054)ptr(374-374)ptr(375-375)ptr(525-525)ptr(527-527)ptr(2386-2386)ptr(2822-2822)ptr(2846-2846)ptr(2864-2864)ptr(3070-3073)ptr(3070-3070)ptr(3074-3077)ptr(3074-3074)ptr(3078-3081)ptr(3078-3078)ptr(3083-3086)ptr(3083-3083)appDomain(1652-1655)appDomain(1658-1658)appDomain(3809-3809)appDomain(3812-3812)appDomain(3813-3813)appDomain(4118-4121)indexAssm(619-622)indexAssm(619-619)indexAssm(643-646)indexAssm(643-643)indexAssm(672-675)indexAssm(672-672)indexAssm(701-704)indexAssm(701-701)indexAssm(730-733)indexAssm(730-730)indexAssm(759-762)indexAssm(759-759)indexAssm(788-791)indexAssm(788-788)indexAssm(817-820)td(1587-1587)inst(2243-2243)inst(2244-2244)inst(2245-2245)inst(3998-4001)inst(4047-4047)assm(1203-1203)assm(1204-1204)assm(1206-1206)assm(1207-1207)assm(1208-1208)assm(1210-1210)assm(1211-1211)assm(1213-1213)assm(1215-1215)assm(1216-1216)assm(1388-1388)assm(1627-1627)assm(1639-1639)assm(1978-1978)assm(1979-1979)assm(1994-1994)fd(2015-2015)fd(2100-2100)fd(2338-2338)fd(3990-3990)md(1426-1430)md(1431-1434)md(2010-2014)md(2099-2099)md(2560-2560)ref(863-863)ref(864-864)ref(865-865)ref(866-866)ref(867-867)ref(868-868)ref(1638-1638)ref(2340-2340)ref(2347-2347)ref(2348-2348)ref(2799-2799)ref(2848-2848)ref(2955-2955)ref(2961-2961)ref(3021-3021)ref(3023-3023)pid(3310-3315)pid(3316-3316)th(2532-2532)th(2551-2555)th(3134-3134)th(3135-3135)th(4015-4020)th(4021-4021)th(4094-4094)th(4095-4095)th(4105-4105)target(466-466)target(471-471)stack(401-401)stack(1561-1561)stack(3339-3339)stack(3340-3345)stack(3356-3356)stack(3357-3357)stack(3376-3380)stack(3415-3419)stack(3415-3415)stack(4097-4097)stack(4098-4098)stack(4099-4099)stack(4100-4100)stack(4101-4101)stack(4103-4103)stack(4104-4104)res(1227-1227)res(2294-2294)_ASSERTE(2649-2653)_ASSERTE(2654-2658)src(1065-1068)src(1065-1065)src(1069-1072)src(1069-1069)src(1085-1085)src(1086-1086)src(1138-1138)src(1438-1443)src(1450-1455)src(1629-1629)index(1463-1463)index(1471-1471)index(1570-1570)index(1571-1571)index(1637-1637)index(2065-2065)index(2078-2078)index(2098-2098)index(2135-2135)index(2163-2163)index(2198-2198)index(2219-2219)assembly(2109-2112)assembly(2142-2145)assembly(2170-2173)assembly(2180-2183)assembly(2206-2209)assembly(2223-2226)assembly(4123-4123)szBuffer(2019-2019)src/CLR/Include/nanoCLR_Debugging.h (28)
cmd(899-899)cmd(924-924)msg(922-922)msg(926-926)msg(927-927)msg(933-933)msg(934-934)msg(935-935)msg(936-936)msg(938-938)msg(939-939)msg(940-940)msg(941-941)msg(942-942)msg(943-943)msg(944-944)msg(945-945)msg(946-946)indexAssm(917-917)td(918-918)fd(919-919)md(920-920)pid(913-913)pid(915-915)pid(931-931)cmdReply(914-914)index(1011-1011)index(1012-1012)src/CLR/WireProtocol/WireProtocol_Message.c (1)
WP_ReplyToCommand(117-165)targets/AzureRTOS/_common/platform_heap.c (1)
platform_malloc(12-24)src/CLR/Core/Execution.cpp (6)
CreateInstance(27-41)CreateInstance(27-27)CreateInstance(45-55)CreateInstance(45-45)AccessStaticField(1799-1822)AccessStaticField(1799-1799)src/CLR/Core/TypeSystem.cpp (4)
CreateInstance(2286-2543)CreateInstance(2286-2286)CreateInstance(2546-2566)CreateInstance(2546-2549)src/CLR/Core/Thread.cpp (6)
CreateInstance(10-38)CreateInstance(10-14)CreateInstance(202-278)CreateInstance(202-202)CreateInstance(280-298)CreateInstance(280-285)targets/ChibiOS/_nanoCLR/nanoCRT.cpp (2)
hal_strncpy_s(159-181)hal_strncpy_s(159-159)src/CLR/Diagnostics/Info_Safeprintf.cpp (2)
CLR_SafeSprintf(41-54)CLR_SafeSprintf(41-41)
src/CLR/Include/nanoCLR_Types.h (2)
src/CLR/Include/nanoCLR_Runtime.h (33)
val(2564-2564)val(2565-2565)val(2566-2566)val(2567-2567)val(2568-2568)val(2569-2569)val(2570-2570)val(2571-2571)val(2572-2572)val(2575-2575)val(2576-2576)val(2578-2578)val(2579-2579)val(2582-2582)val(2583-2583)tbl(1475-1478)tbl(1475-1475)data(614-617)data(638-641)data(667-670)data(696-699)data(713-716)data(725-728)data(754-757)data(783-786)data(812-815)data(859-859)data(1393-1393)data(1399-1399)data(2800-2800)data(4077-4077)p(1568-1568)p(1569-1569)src/CLR/CorLib/corlib_native.h (1)
val(476-482)
⏰ Context from checks skipped due to timeout of 90000ms (26)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_ETHERNET_KIT_1.2)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_S3_ALL)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_H2_THREAD)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C6_THREAD)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C3)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_BLE_REV0)
- GitHub Check: nf-interpreter (Build_WIN32_nanoCLR)
- GitHub Check: nf-interpreter (Build_STM32_targets ST_STM32F769I_DISCOVERY)
- GitHub Check: nf-interpreter (Build_TI_SimpleLink_targets TI_CC1352R1_LAUNCHXL_915)
- GitHub Check: nf-interpreter (Build_NXP_targets NXP_MIMXRT1060_EVK)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_PSRAM_REV0)
- GitHub Check: nf-interpreter (Build_Azure_RTOS_targets SL_STK3701A)
- GitHub Check: nf-interpreter (Build_STM32_targets ST_STM32F429I_DISCOVERY)
- GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALX)
- GitHub Check: nf-interpreter (Check_Code_Style)
- 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)
- GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
- GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
- GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
- GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
- GitHub Check: build-target (ESP32_C3, Debug, ESP32)
- GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
- GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
- GitHub Check: build-target (SL_STK3701A, Debug, All)
🔇 Additional comments (50)
src/CLR/Debugger/Debugger.cpp (38)
23-23: Modernized code withnullptrusageThe change from
NULLtonullptrimproves type safety and follows modern C++ practices.
132-132: Approved use of nullptrConsistent use of
nullptrinstead ofNULLprovides better type safety for pointer assignments.
171-171: Good cleanup in destructorProper cleanup with
nullptrin the destructor prevents dangling pointer issues.
216-216: Consistent nullptr return valuesConsistent use of
nullptrfor null return values aligns with modern C++ practices.Also applies to: 237-237
304-305: Member naming convention updateUpdated member naming from Hungarian notation
m_flagstoFlags, consistent with the broader code modernization effort seen in related files.Also applies to: 323-324
380-381: Improved type safety with auto pointerUsing
auto *for pointer casting improves readability and type safety. The nullability check is also properly updated to usenullptr.Also applies to: 383-383
431-432: Consistent modern pointer handlingSimilar improvements with
auto *andnullptrchecks for message payload handling.Also applies to: 434-434
459-463: Modernized pointer initializationInitialization of multiple pointers with
nullptrrather thanNULLimproves type safety.
475-475: Enhanced null checksUpdated null pointer checks for better readability and type safety.
Also applies to: 482-482, 505-505, 522-522
1051-1051: Simplified command responseUsing
nullptrinstead ofNULLfor empty replies keeps consistency and improves code clarity.Also applies to: 1078-1078
1460-1469: Updated struct member accessMember access has been updated to use direct member names rather than the
m_prefix, consistent with broader codebase modernization.
1513-1513: Fixing memset callCorrected memset usage to properly zero memory before filing in array elements.
1527-1533: Modernized field accessUpdated version field access pattern consistently with naming convention changes seen throughout the codebase.
1547-1553: Improved pointer initialization and usageConsistent use of
nullptrand modern C++ initialization patterns.
1750-1753: Proper resource cleanup conditionalImproved check to ensure data is not null before freeing it, preventing potential crashes.
1781-1781: Type inference with autoConsistent use of
auto *for type inference when casting from message payload.
1810-1811: Fixed conditional return logicImproved formatting and simplified conditional return logic to make the code more readable.
Also applies to: 1815-1816
1824-1825: Updated conditional null checkingImproved null checking with
nullptrand modernized conditional structure.Also applies to: 1829-1832
1867-1878: Enhanced string handling safetyImproved null checks for string text pointers before accessing them.
1890-1891: Simplified dereferencingSimplification of dereference calls with
nullptras a default parameter.
1898-1900: Added support for generic instancesNew case handler for
DATATYPE_GENERICINSTimproves support for generic types, which is critical for the PR's goal of fixing generic array creation.
1904-1908: Updated array handlingUpdated array handling with proper type casting and access to reflection data.
1959-1962: Fixed default case handlingImproved default case handling with proper return value to indicate failure.
1986-1986: Simplified error responseUsing
nullptrfor response parameters simplifies error handling paths.
2049-2060: Updated method signature for IsGoodAssemblyChanged the parameter type from
CLR_IDXtoCLR_INDEXand renamed the member fromm_idxtoassemblyIndexto match updated structures in related files.
2067-2070: Consistent member access patternsUpdated tablesSize member access pattern to be consistent throughout the validation methods.
Also applies to: 2080-2083, 2093-2096
2142-2142: Improved null handling in thread creationUsing
nullptrinstead ofNULLfor thread initialization and better type safety.
2158-2160: Simplified target accessDirectly accessing target members without the
m_prefix, consistent with the modernization approach.
2164-2169: Updated flag handlingUpdated flag access patterns to match modernized member naming conventions.
2171-2171: Improved null safetySetting object references with
nullptrfor better type safety.
2191-2194: Better type checkingImproved type checking for references in parameter validation.
2212-2212: Cleaning up on failureProperly setting thread to
nullptrwhen cleanup happens on failure paths.
2438-2459: Simplified return flow for GetValueThe code flow was simplified by directly returning the result of GetValue rather than using an intermediate WP_ReplyToCommand call. This removes duplicate code and makes the control flow clearer.
2644-2659: Modernized scratch pad helperUpdated helper function with better null handling and simplified return logic.
2805-2834: Critical fix for handling generic parametersThis change adds proper support for generic parameters in both type definitions and method definitions. The code now correctly identifies and handles
DATATYPE_VAR(generic parameter in a type) andDATATYPE_MVAR(generic parameter in a method) by retrieving the corresponding generic parameter information.This appears to be central to the PR's goal of fixing generic array creation, as it now properly supports resolution of the type parameters needed to create arrays of generic types.
2845-2856: Enhanced enum type handlingAdded specific handling for generic types that might be enums, ensuring proper type information is passed.
2870-2871: Streamlined return flowMultiple instances of return flow simplification by directly returning the GetValue result rather than unnecessary intermediate function calls.
Also applies to: 2983-2984, 3034-3035, 3047-3048, 3072-3073
3881-3882: Consistent nullptr usage in command repliesUsing
nullptrconsistently in all command reply function calls.Also applies to: 3890-3892, 3899-3901
src/CLR/Core/Execution.cpp (9)
94-98: Modernized pointer initialization to nullptrThe change from NULL to nullptr is a good practice, improving type safety and aligning with modern C++ standards.
165-166: Improved variable naming for better readabilityRemoving the 'm_' prefix from local variables makes the code more readable and consistent with modern C++ naming conventions.
1555-1580: Added critical heap block extraction method for generic instancesThis new method provides the necessary functionality for allocating heap blocks specifically for generic instances, which is essential for the PR's goal of supporting generic arrays.
2234-2252: Implemented first overload of NewGenericInstanceObjectThis method provides a clean interface for creating generic instance objects by accepting a type definition and generic type index, then delegating to the implementation method.
2254-2322: Implemented second overload of NewGenericInstanceObject with proper pointer handlingThe implementation correctly addresses the pointer arithmetic issue mentioned in previous reviews by using a separate field cursor (
fieldCursor) instead of incrementing/decrementing the generic instance pointer directly. This prevents potential memory corruption during GC, relocation or compaction.
1961-1997: Enhanced support for generic parameter handlingAdded proper handling for GENERICINST, VAR, and MVAR data types, which is crucial for correctly initializing locals with generic type parameters.
2062-2067: Added special handling for generic instances in locals initializationGeneric instance locals are now properly initialized as OBJECT type with appropriate data ID and clearing, ensuring correct behavior when used in methods.
932-934: Updated SpawnStaticConstructorHelper signature to improve type clarityChanged the signature to use CLR_RT_MethodDef_Index by const reference instead of plain value, providing better type safety and preventing unnecessary copies.
2206-2208: Added generic instance case to switch statementUpdated the switch statement in NewObject to handle DATATYPE_GENERICINST, ensuring that when creating objects, generic instances are properly recognized and processed.
src/CLR/Include/nanoCLR_Types.h (3)
541-574:CLR_UncompressMethodTokentag layout diverges from ECMA-335 – double-check encoder/decoder symmetryThe lookup table now maps the 2-bit tag to
{ MethodDef, MethodRef, TypeSpec, MethodSpec }.
The official MethodDefOrRef coded token has only two legal values (00 = MethodDef, 01 = MemberRef). IntroducingTypeSpec&MethodSpecrequires that the encoder in the metadata-processor emits exactly the same 2-bit pattern (10 / 11). Any mismatch will corrupt every compressed method token at runtime.Please confirm that:
nanoCLR_TypeSystem& MDP were updated to emit the same mapping.- No existing binaries (without the new mapping) will be loaded by the updated runtime – otherwise backward-compat must be handled.
292-317: Enum expansion looks good and fixes the off-by-one literal
TBL_GenericParam,TBL_MethodSpecand the corrected
TBL_EndOfAssembly = 0x00000011align with the new metadata layout.
Nice catch on the missing leading zero! 👍
109-118:❓ Verification inconclusive
16-bit encoded indices may overflow when large generic metadata is emitted
All new encoded-token typedefs (
CLR_Encoded*,CLR_TYPEORMETHODDEF, etc.) are fixed touint16_t.
With generics switched on, tables such asTBL_MethodSpec,TBL_GenericParam,TBL_TypeSpeccan easily grow past 65 535 rows (think of multi-assembly projects or heavily-instantiated templates). The current type silently wraps around, producing hard-to-diagnose runtime faults.Consider either
-typedef CLR_UINT16 CLR_EncodedMethodDefOrRef; +typedef CLR_UINT32 CLR_EncodedMethodDefOrRef;for all encoded handles, or at least add a build-time guard that fails the metadata‐processor when a table would exceed 0xFFFF rows.
Would you run the metadata-processor on a large solution (e.g. the core-library + several class-libs) to confirm the maximum row count stays below 65 535?
🏁 Script executed:
#!/bin/bash # naïve head-count per table in all *.nanoFramework metadata fd -e nfmeta | while read f; do printf "%s\t" "$f" ast-grep --pattern $'TBL_MethodSpec($_)' "$f" | wc -l doneLength of output: 23
Verify metadata table sizes to prevent 16-bit overflow
It looks like there are no
.nfmetafiles in the repo, so the naïve script didn’t return any counts. Please run the metadata-processor on a representative large solution (e.g. core-library + several class-libs) and confirm that none of the encoded tables (MethodSpec, GenericParam, TypeSpec, TypeRefOrSpec, etc.) exceed 65 535 entries. If any table can grow past 0xFFFF, you’ll need to update the encoded-handle typedefs or add a build-time check to the metadata-processor:• File: src/CLR/Include/nanoCLR_Types.h (lines 109–118)
• Suggestion for allCLR_Encoded*typedefs:- typedef CLR_UINT16 CLR_EncodedMethodDefOrRef; + typedef CLR_UINT32 CLR_EncodedMethodDefOrRef;…and similarly for
CLR_EncodedTypeDefOrRef,CLR_EncodedTypeRefOrSpec,CLR_TYPEORMETHODDEF, etc.Alternatively, implement a guard in the metadata-processor that fails if any table row count exceeds 0xFFFF.
Description
Motivation and Context
How Has This Been Tested?
[build with MDP buildId 55803]
Screenshots
Types of changes
Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
NULLwithnullptrfor safer pointer handling.Bug Fixes
Chores
Documentation
These changes enhance reliability, maintainability, and add new capabilities for advanced .NET nanoFramework scenarios.