Misc cleanup (fixing comments, deleting dead code) (#117908)

jkotas · web-flow · commit 085732c314c7 · 2025-07-22T00:03:07.000-07:00
* Left-over feedback from #114246 * Delete unused macros * Delete dead ifdef * Delete obsolete comment * Delete dead code * Fix formatting * Delete dead code * Delete mentions of Windows Arm32 * Delete unrecognized attribute * Fix comment and log message
diff --git a/docs/workflow/requirements/windows-requirements.md b/docs/workflow/requirements/windows-requirements.md
@@ -46,9 +46,9 @@ It is highly recommended to use the *Workloads* approach, as that installs the f
 - .NET desktop development
 - Desktop development with C++
 
-To build the tests and do ARM32/ARM64 development, you'll need some additional components. You can find them by clicking on the *Individual components* tab in the *Visual Studio Installer*:
+To build the tests and do ARM64 development, you'll need some additional components. You can find them by clicking on the *Individual components* tab in the *Visual Studio Installer*:
 
-- For ARM stuff: *MSVC v143 - VS 2022 C++ ARM64/ARM64EC build tools (Latest)* for Arm64, and *MSVC v143 - VS 2022 C++ ARM build tools (Latest)* for Arm32.
+- For Arm64: *MSVC v143 - VS 2022 C++ ARM64/ARM64EC build tools (Latest)*
 - For building tests: *C++/CLI support for v143 build tools (Latest)*
 
 Alternatively, there is also a `.vsconfig` file included at the root of the runtime repo. It includes all the necessary components required, outlined in a JSON format that Visual Studio can read and parse. You can boot up Visual Studio directly and [import this `.vsconfig` file](https://learn.microsoft.com/visualstudio/install/import-export-installation-configurations?view=vs-2022#import-a-configuration) instead of installing the workloads yourself. It is worth mentioning however, that while we are very careful in maintaining this file up-to-date, sometimes it might get a tad obsolete and miss important components. So, it is always a good idea to double check that the full workloads are installed.
diff --git a/src/coreclr/inc/loaderheap.h b/src/coreclr/inc/loaderheap.h
@@ -466,7 +466,7 @@ struct InterleavedLoaderHeapConfig
 void InitializeLoaderHeapConfig(InterleavedLoaderHeapConfig *pConfig, size_t stubSize, void* templateInImage, void (*codePageGenerator)(uint8_t* pageBase, uint8_t* pageBaseRX, size_t size), void (*dataPageGenerator)(uint8_t* pageBase, size_t size));
 
 //===============================================================================
-// This is the base class for InterleavedLoaderHeap It's used as a simple
+// This is the base class for InterleavedLoaderHeap. It's used as a simple
 // allocator for stubs in a scheme where each stub is a small fixed size, and is paired
 // with memory which is GetStubCodePageSize() bytes away. In addition there is an
 // ability to free is via a "backout" mechanism that is not considered to have good performance.
@@ -574,7 +574,7 @@ class UnlockedInterleavedLoaderHeap : public UnlockedLoaderHeapBase
                                  );
 
 protected:
-    // This frees memory allocated by UnlockAllocMem. It's given this horrible name to emphasize
+    // This frees memory allocated by UnlockedAllocStub. It's given this horrible name to emphasize
     // that it's purpose is for error path leak prevention purposes. You shouldn't
     // use LoaderHeap's as general-purpose alloc-free heaps.
     void UnlockedBackoutStub(void *pMem
@@ -702,8 +702,7 @@ inline CRITSEC_COOKIE CreateLoaderHeapLock()
 }
 
 //===============================================================================
-// The LoaderHeap is the black-box heap and has a Backout() method but none
-// of the advanced features that let you control address ranges.
+// Thread-safe variant of UnlockedLoaderHeap.
 //===============================================================================
 typedef DPTR(class LoaderHeap) PTR_LoaderHeap;
 class LoaderHeap : public UnlockedLoaderHeap
@@ -972,7 +971,7 @@ class LoaderHeap : public UnlockedLoaderHeap
 
 
 public:
-    // This frees memory allocated by AllocMem. It's given this horrible name to emphasize
+    // This frees memory allocated by RealAllocMem. It's given this horrible name to emphasize
     // that it's purpose is for error path leak prevention purposes. You shouldn't
     // use LoaderHeap's as general-purpose alloc-free heaps.
     void RealBackoutMem(void *pMem
@@ -1033,8 +1032,7 @@ class LoaderHeap : public UnlockedLoaderHeap
 #endif
 
 //===============================================================================
-// The LoaderHeap is the black-box heap and has a Backout() method but none
-// of the advanced features that let you control address ranges.
+// Thread-safe variant of UnlockedInterleavedLoaderHeap.
 //===============================================================================
 typedef DPTR(class InterleavedLoaderHeap) PTR_InterleavedLoaderHeap;
 class InterleavedLoaderHeap : public UnlockedInterleavedLoaderHeap
@@ -1107,7 +1105,7 @@ class InterleavedLoaderHeap : public UnlockedInterleavedLoaderHeap
 
 
 public:
-    // This frees memory allocated by AllocMem. It's given this horrible name to emphasize
+    // This frees memory allocated by RealAllocStub. It's given this horrible name to emphasize
     // that it's purpose is for error path leak prevention purposes. You shouldn't
     // use LoaderHeap's as general-purpose alloc-free heaps.
     void RealBackoutMem(void *pMem
diff --git a/src/coreclr/inc/palclr.h b/src/coreclr/inc/palclr.h
@@ -362,30 +362,6 @@
     {                                                                           \
         PAL_TRY_HANDLER_DBG_BEGIN
 
-// PAL_TRY implementation that abstracts usage of COMPILER_INSTANCE*, which is used by
-// JIT64. On Windows, we dont need to do anything special as we dont have nested classes/methods
-// as on PAL.
-#define PAL_TRY_CI(__ParamType, __paramDef, __paramRef)                         \
-{                                                                               \
-    struct __HandlerData {                                                      \
-        __ParamType __param;                                                    \
-        COMPILER_INSTANCE *__ciPtr;                                             \
-    };                                                                          \
-    __HandlerData handlerData;                                                  \
-    handlerData.__param = __paramRef;                                           \
-    handlerData.__ciPtr = ciPtr;                                                \
-     __HandlerData* __param = &handlerData;                                     \
-    __ParamType __paramToPassToFilter = __paramRef;                             \
-    class __Body                                                                \
-    {                                                                           \
-    public:                                                                     \
-    static void Run(__HandlerData* __pHandlerData)                              \
-    {                                                                           \
-    PAL_TRY_HANDLER_DBG_BEGIN                                                   \
-        COMPILER_INSTANCE *ciPtr = __pHandlerData->__ciPtr;                     \
-        __ParamType __paramDef = __pHandlerData->__param;
-
-
 #define PAL_TRY_FOR_DLLMAIN(__ParamType, __paramDef, __paramRef, __reason)      \
 {                                                                               \
     __ParamType __param = __paramRef;                                           \
@@ -434,11 +410,6 @@
     PAL_TRY_NAKED                                                               \
     PAL_TRY_HANDLER_DBG_BEGIN
 
-// PAL_TRY implementation that abstracts usage of COMPILER_INSTANCE*, which is used by
-// JIT64. On Windows, we dont need to do anything special as we dont have nested classes/methods
-// as on PAL.
-#define PAL_TRY_CI(__ParamType, __paramDef, __paramRef) PAL_TRY(__ParamType, __paramDef, __paramRef)
-
 #define PAL_TRY_FOR_DLLMAIN(__ParamType, __paramDef, __paramRef, __reason)      \
 {                                                                               \
     __ParamType __param = __paramRef;                                           \
@@ -464,10 +435,6 @@
 
 #endif // _DEBUG
 
-// Executes the handler if the specified exception code matches
-// the one in the exception. Otherwise, returns EXCEPTION_CONTINUE_SEARCH.
-#define PAL_EXCEPT_IF_EXCEPTION_CODE(dwExceptionCode) PAL_EXCEPT((GetExceptionCode() == (dwExceptionCode))?EXCEPTION_EXECUTE_HANDLER:EXCEPTION_CONTINUE_SEARCH)
-
 #define PAL_CPP_TRY try
 #define PAL_CPP_ENDTRY
 #define PAL_CPP_THROW(type, obj) do { throw obj; } while (false)
diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp
@@ -6808,7 +6808,7 @@ void Compiler::impCheckForPInvokeCall(
         }
     }
 
-    JITLOG((LL_INFO1000000, "\nInline a CALLI PINVOKE call from method %s\n", info.compFullName));
+    JITLOG((LL_INFO1000000, "\nInline a PINVOKE call from method %s\n", info.compFullName));
 
     call->gtFlags |= GTF_CALL_UNMANAGED;
     call->unmgdCallConv = unmanagedCallConv;
@@ -6817,7 +6817,6 @@ void Compiler::impCheckForPInvokeCall(
         info.compUnmanagedCallCountWithGCTransition++;
     }
 
-    // AMD64 convention is same for native and managed
     if (unmanagedCallConv == CorInfoCallConvExtension::C ||
         unmanagedCallConv == CorInfoCallConvExtension::CMemberFunction)
     {
diff --git a/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs b/src/coreclr/tools/Common/Internal/Metadata/NativeFormat/NativeMetadataReader.cs
@@ -234,9 +234,6 @@ internal ReadOnlySpan<byte> ReadStringAsBytes(ConstantStringValueHandle handle)
 
     internal sealed partial class MetadataHeader
     {
-        /// <todo>
-        /// Signature should be updated every time the metadata schema changes.
-        /// </todo>
         public const uint Signature = 0xDEADDFFD;
 
         /// <summary>
diff --git a/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs b/src/coreclr/tools/Common/Internal/NativeFormat/NativeFormatReader.String.cs
@@ -73,15 +73,7 @@ public unsafe uint DecodeString(uint offset, out string value)
             if (endOffset < numBytes || endOffset > _size)
                 ThrowBadImageFormatException();
 
-#if NETFX_45
-            byte[] bytes = new byte[numBytes];
-            for (int i = 0; i < bytes.Length; i++)
-                bytes[i] = *(_base + offset + i);
-
-            value = Encoding.UTF8.GetString(bytes, 0, bytes.Length);
-#else
             value = Encoding.UTF8.GetString(_base + offset, (int)numBytes);
-#endif
 
             return endOffset;
         }
diff --git a/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler.MetadataTransform.csproj b/src/coreclr/tools/aot/ILCompiler.MetadataTransform/ILCompiler.MetadataTransform.csproj
@@ -4,7 +4,7 @@
     <AssemblyName>ILCompiler.MetadataTransform</AssemblyName>
     <TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <DefineConstants>$(DefineConstants);NATIVEFORMAT_PUBLICWRITER;NETFX_45</DefineConstants>
+    <DefineConstants>$(DefineConstants);NATIVEFORMAT_PUBLICWRITER</DefineConstants>
     <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
     <Platforms>x64;x86</Platforms>
     <PlatformTarget>AnyCPU</PlatformTarget>
diff --git a/src/coreclr/utilcode/CMakeLists.txt b/src/coreclr/utilcode/CMakeLists.txt
@@ -30,7 +30,6 @@ set(UTILCODE_COMMON_SOURCES
   interleavedloaderheap.cpp
   loaderheap_shared.cpp
   explicitcontrolloaderheap.cpp
-  allocmemtracker.cpp
   rangelist.cpp
   outstring.cpp
   ilformatter.cpp
@@ -69,6 +68,7 @@ endif(CLR_CMAKE_TARGET_WIN32)
 
 set(UTILCODE_SOURCES
   ${UTILCODE_COMMON_SOURCES}
+  allocmemtracker.cpp
   executableallocator.cpp
 )
 
diff --git a/src/coreclr/utilcode/allocmemtracker.cpp b/src/coreclr/utilcode/allocmemtracker.cpp
@@ -8,8 +8,6 @@
 #define DONOT_DEFINE_ETW_CALLBACK
 #include "eventtracebase.h"
 
-#ifndef DACCESS_COMPILE
-
 AllocMemTracker::AllocMemTracker()
 {
     CONTRACTL
@@ -169,5 +167,3 @@ void AllocMemTracker::SuppressRelease()
 
     m_fReleased = TRUE;
 }
-
-#endif //#ifndef DACCESS_COMPILE
diff --git a/src/coreclr/utilcode/explicitcontrolloaderheap.cpp b/src/coreclr/utilcode/explicitcontrolloaderheap.cpp
@@ -72,7 +72,6 @@ ExplicitControlLoaderHeap::ExplicitControlLoaderHeap(bool fMakeExecutable) :
 #endif
 }
 
-// ~LoaderHeap is not synchronised (obviously)
 ExplicitControlLoaderHeap::~ExplicitControlLoaderHeap()
 {
     CONTRACTL
diff --git a/src/coreclr/utilcode/interleavedloaderheap.cpp b/src/coreclr/utilcode/interleavedloaderheap.cpp
@@ -52,7 +52,6 @@ UnlockedInterleavedLoaderHeap::UnlockedInterleavedLoaderHeap(
     _ASSERTE((GetStubCodePageSize() % GetOsPageSize()) == 0); // Stub code page size MUST be in increments of the page size. (Really it must be a power of 2 as well, but this is good enough)
 }
 
-// ~LoaderHeap is not synchronised (obviously)
 UnlockedInterleavedLoaderHeap::~UnlockedInterleavedLoaderHeap()
 {
     CONTRACTL
@@ -186,7 +185,7 @@ BOOL UnlockedInterleavedLoaderHeap::UnlockedReservePages(size_t dwSizeToCommit)
 
     size_t dwSizeToCommitPart = dwSizeToCommit;
 
-    // For interleaved heaps, we perform two commits, each being half of the requested size
+    // We perform two commits, each being half of the requested size
     dwSizeToCommitPart /= 2;
 
     if (!CommitPages(pData, dwSizeToCommitPart))
@@ -314,28 +313,28 @@ BOOL UnlockedInterleavedLoaderHeap::GetMoreCommittedPages(size_t dwMinSize)
     // This mode interleaves data and code pages 1:1. So the code size is required to be smaller than
     // or equal to the page size to ensure that the code range is consecutive.
     _ASSERTE(dwMinSize <= GetStubCodePageSize());
-    // For interleaved heap, we always get two memory pages - one for code and one for data
+    // We always get two memory pages - one for code and one for data
     dwMinSize = 2 * GetStubCodePageSize();
 
     // Does this fit in the reserved region?
     if (dwMinSize <= (size_t)(m_pEndReservedRegion - m_pAllocPtr))
     {
         SIZE_T dwSizeToCommit;
 
-        // For interleaved heaps, the allocation cannot cross page boundary since there are data and executable
+        // The allocation cannot cross page boundary since there are data and executable
         // pages interleaved in a 1:1 fashion.
         dwSizeToCommit = dwMinSize;
 
         size_t unusedRemainder = (size_t)((BYTE*)m_pPtrToEndOfCommittedRegion - m_pAllocPtr);
 
         PTR_BYTE pCommitBaseAddress = m_pPtrToEndOfCommittedRegion;
 
-        // The end of committed region for interleaved heaps points to the end of the executable
+        // The end of committed region points to the end of the executable
         // page and the data pages goes right after that. So we skip the data page here.
         pCommitBaseAddress += GetStubCodePageSize();
 
         size_t dwSizeToCommitPart = dwSizeToCommit;
-        // For interleaved heaps, we perform two commits, each being half of the requested size
+        // We perform two commits, each being half of the requested size
         dwSizeToCommitPart /= 2;
 
         if (!CommitPages(pCommitBaseAddress, dwSizeToCommitPart))
@@ -345,7 +344,7 @@ BOOL UnlockedInterleavedLoaderHeap::GetMoreCommittedPages(size_t dwMinSize)
 
         INDEBUG(m_dwDebugWastedBytes += unusedRemainder;)
 
-        // For interleaved heaps, further allocations will start from the newly committed page as they cannot
+        // Further allocations will start from the newly committed page as they cannot
         // cross page boundary.
         m_pAllocPtr = (BYTE*)pCommitBaseAddress;
 
diff --git a/src/coreclr/utilcode/loaderheap.cpp b/src/coreclr/utilcode/loaderheap.cpp
@@ -70,7 +70,6 @@ UnlockedLoaderHeap::UnlockedLoaderHeap(DWORD dwReserveBlockSize,
     }
 }
 
-// ~LoaderHeap is not synchronised (obviously)
 UnlockedLoaderHeap::~UnlockedLoaderHeap()
 {
     CONTRACTL
@@ -1053,7 +1052,6 @@ size_t UnlockedLoaderHeap::AllocMem_TotalSize(size_t dwRequestedSize)
 
     size_t dwSize = dwRequestedSize;
 
-    // Interleaved heap cannot ad any extra to the requested size
 #ifdef _DEBUG
     dwSize += LOADER_HEAP_DEBUG_BOUNDARY;
     dwSize = ((dwSize + ALLOC_ALIGN_CONSTANT) & (~ALLOC_ALIGN_CONSTANT));
diff --git a/src/coreclr/vm/eedbginterface.h b/src/coreclr/vm/eedbginterface.h
@@ -36,9 +36,6 @@ class Frame;
 // between the EE and the Debugger.
 //
 //
-typedef BOOL (*HashMapEnumCallback)(HashMap* h,
-                                    void* pData,
-                                    ULONG value);
 
 typedef enum AttachAppDomainEventsEnum
 {
diff --git a/src/coreclr/vm/runtimehandles.h b/src/coreclr/vm/runtimehandles.h
@@ -113,8 +113,6 @@ class RuntimeTypeHandle
 
     static FCDECL1(FC_BOOL_RET, IsUnmanagedFunctionPointer, ReflectClassBaseObject *pTypeUNSAFE);
 
-    static FCDECL2(TypeHandle::CastResult, CanCastToInternal, ReflectClassBaseObject *pType, ReflectClassBaseObject *pTarget);
-
     static
     FCDECL1(FC_BOOL_RET, IsGenericVariable, PTR_ReflectClassBaseObject pType);
 
diff --git a/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatter.cs b/src/libraries/Microsoft.Extensions.Logging.Console/src/JsonConsoleFormatter.cs
@@ -89,12 +89,12 @@ private void WriteInternal(IExternalScopeProvider? scopeProvider, TextWriter tex
                             writer.WriteString("Message", stateMessage);
                         }
                         if (stateProperties != null)
+                        {
+                            foreach (KeyValuePair<string, object?> item in stateProperties)
                             {
-                                foreach (KeyValuePair<string, object?> item in stateProperties)
-                                {
-                                    WriteItem(writer, item);
-                                }
+                                WriteItem(writer, item);
                             }
+                        }
                         writer.WriteEndObject();
                     }
                     WriteScopeInformation(writer, scopeProvider);
diff --git a/src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs b/src/tests/nativeaot/SmokeTests/PInvoke/PInvoke.cs
@@ -12,13 +12,6 @@
 using System.Runtime.InteropServices;
 using System.Text;
 
-// Make sure the interop data are present even without reflection
-namespace System.Runtime.CompilerServices
-{
-    [AttributeUsage(AttributeTargets.All)]
-    internal class __BlockAllReflectionAttribute : Attribute { }
-}
-
 // Name of namespace matches the name of the assembly on purpose to
 // ensure that we can handle this (mostly an issue for C++ code generation).
 namespace PInvokeTests

Original file line number	Diff line number	Diff line change
`@@ -6808,7 +6808,7 @@ void Compiler::impCheckForPInvokeCall(`
`6808`	`6808`	`}`
`6809`	`6809`	`}`
`6810`	`6810`
`6811`		`- JITLOG((LL_INFO1000000, "\nInline a CALLI PINVOKE call from method %s\n", info.compFullName));`
	`6811`	`+ JITLOG((LL_INFO1000000, "\nInline a PINVOKE call from method %s\n", info.compFullName));`
`6812`	`6812`
`6813`	`6813`	`call->gtFlags \|= GTF_CALL_UNMANAGED;`
`6814`	`6814`	`call->unmgdCallConv = unmanagedCallConv;`
`@@ -6817,7 +6817,6 @@ void Compiler::impCheckForPInvokeCall(`
`6817`	`6817`	`info.compUnmanagedCallCountWithGCTransition++;`
`6818`	`6818`	`}`
`6819`	`6819`
`6820`		`- // AMD64 convention is same for native and managed`
`6821`	`6820`	`if (unmanagedCallConv == CorInfoCallConvExtension::C \|\|`
`6822`	`6821`	`unmanagedCallConv == CorInfoCallConvExtension::CMemberFunction)`
`6823`	`6822`	`{`
Original file line number	Diff line number	Diff line change
`@@ -234,9 +234,6 @@ internal ReadOnlySpan<byte> ReadStringAsBytes(ConstantStringValueHandle handle)`
`234`	`234`
`235`	`235`	`internal sealed partial class MetadataHeader`
`236`	`236`	`{`
`237`		`- /// <todo>`
`238`		`- /// Signature should be updated every time the metadata schema changes.`
`239`		`- /// </todo>`
`240`	`237`	`public const uint Signature = 0xDEADDFFD;`
`241`	`238`
`242`	`239`	`/// <summary>`
Original file line number	Diff line number	Diff line change
`@@ -8,8 +8,6 @@`
`8`	`8`	`#define DONOT_DEFINE_ETW_CALLBACK`
`9`	`9`	`#include "eventtracebase.h"`
`10`	`10`
`11`		`-#ifndef DACCESS_COMPILE`
`12`		`-`
`13`	`11`	`AllocMemTracker::AllocMemTracker()`
`14`	`12`	`{`
`15`	`13`	`CONTRACTL`
`@@ -169,5 +167,3 @@ void AllocMemTracker::SuppressRelease()`
`169`	`167`
`170`	`168`	`m_fReleased = TRUE;`
`171`	`169`	`}`
`172`		`-`
`173`		`-#endif //#ifndef DACCESS_COMPILE`
Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,6 @@ ExplicitControlLoaderHeap::ExplicitControlLoaderHeap(bool fMakeExecutable) :`
`72`	`72`	`#endif`
`73`	`73`	`}`
`74`	`74`
`75`		`-// ~LoaderHeap is not synchronised (obviously)`
`76`	`75`	`ExplicitControlLoaderHeap::~ExplicitControlLoaderHeap()`
`77`	`76`	`{`
`78`	`77`	`CONTRACTL`