Skip to content

Move stgpool to md subsystem, extract memory streams, eliminate VM dependencies on MD internals#122723

Merged
jkotas merged 33 commits intomainfrom
copilot/remove-private-implementation-details
Dec 30, 2025
Merged

Move stgpool to md subsystem, extract memory streams, eliminate VM dependencies on MD internals#122723
jkotas merged 33 commits intomainfrom
copilot/remove-private-implementation-details

Conversation

Copy link
Contributor

Copilot AI commented Dec 24, 2025 •
edited by jkotas
Loading

Fixes #35624

  • Moved stgpool.h, stgpooli.h from src/coreclr/inc/ to src/coreclr/md/inc/
  • Moved stgpool.cpp, stgpooli.cpp, stgpoolreadonly.cpp from src/coreclr/utilcode/ to src/coreclr/md/runtime/
  • Moved CPackedLen struct from stgpooli.h to src/coreclr/inc/mdfileformat.h
  • Moved CInMemoryStream and CGrowableStream from stgpool.* to memorystreams.*
  • Replaced CDescPool with standalone implementation

Copilot AI changed the title [WIP] Clean up src/coreclr/src/inc by removing private details Move stgpool to md subsystem and eliminate CDescPool dependency on StgPool Dec 24, 2025
Copilot AI requested a review from jkotas December 24, 2025 06:55
jkotas
jkotas reviewed Dec 24, 2025
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address feedback

Copilot AI requested a review from jkotas December 24, 2025 07:32
@build-analysis build-analysis bot mentioned this pull request Dec 24, 2025
Open
jkotas
jkotas reviewed Dec 24, 2025
View reviewed changes
jkotas
jkotas reviewed Dec 24, 2025
View reviewed changes
Copilot AI changed the title Move stgpool to md subsystem and eliminate CDescPool dependency on StgPool Move stgpool to md subsystem, extract memory streams, and eliminate CDescPool dependency on StgPool Dec 24, 2025
Copilot AI requested a review from jkotas December 24, 2025 13:07
@build-analysis build-analysis bot mentioned this pull request Dec 24, 2025
Open
jkotas
jkotas reviewed Dec 24, 2025
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address feedback

@jkotas
Copy link
Member

jkotas commented Dec 24, 2025

@copilot I see a lot of build breaks in the new memorystreams.h/.cpp:

2025-12-24T13:33:24.6276312Z   /__w/1/s/src/coreclr/inc/memoryrange.h:40:9: error: use of undeclared identifier 'SUPPORTS_DAC'
2025-12-24T13:33:24.6276951Z      40 |         SUPPORTS_DAC;
2025-12-24T13:33:24.6279217Z         |         ^~~~~~~~~~~~
2025-12-24T13:33:24.6281515Z   /__w/1/s/src/coreclr/inc/memoryrange.h:47:9: error: use of undeclared identifier 'SUPPORTS_DAC'
2025-12-24T13:33:24.6283726Z      47 |         SUPPORTS_DAC;
2025-12-24T13:33:24.6285911Z         |         ^~~~~~~~~~~~
2025-12-24T13:33:24.6336935Z   /__w/1/s/src/coreclr/inc/memoryrange.h:57:9: error: use of undeclared identifier 'LIMITED_METHOD_DAC_CONTRACT'
2025-12-24T13:33:24.6339567Z      57 |         LIMITED_METHOD_DAC_CONTRACT;
2025-12-24T13:33:24.6341777Z         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-12-24T13:33:24.6404244Z   /__w/1/s/src/coreclr/inc/memoryrange.h:65:9: error: use of undeclared identifier 'LIMITED_METHOD_DAC_CONTRACT'
2025-12-24T13:33:24.6430500Z      65 |         LIMITED_METHOD_DAC_CONTRACT;
2025-12-24T13:33:24.6431167Z         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-12-24T13:33:24.6431911Z   /__w/1/s/src/coreclr/inc/memoryrange.h:73:9: error: use of undeclared identifier 'SUPPORTS_DAC'
2025-12-24T13:33:24.6447962Z      73 |         SUPPORTS_DAC;
2025-12-24T13:33:24.6453606Z         |         ^~~~~~~~~~~~
2025-12-24T13:33:24.6461318Z   /__w/1/s/src/coreclr/inc/memoryrange.h:80:9: error: use of undeclared identifier 'SUPPORTS_DAC'
2025-12-24T13:33:24.6461976Z      80 |         SUPPORTS_DAC;
2025-12-24T13:33:24.6462530Z         |         ^~~~~~~~~~~~
2025-12-24T13:33:24.6463090Z   In file included from /__w/1/s/src/coreclr/utilcode/memorystreams.cpp:11:
2025-12-24T13:33:24.6463772Z   /__w/1/s/src/coreclr/inc/memorystreams.h:22:32: error: base class has incomplete type
2025-12-24T13:33:24.6464119Z      22 | class CInMemoryStream : public IStream
2025-12-24T13:33:24.6464405Z         |                         ~~~~~~~^~~~~~~
2025-12-24T13:33:24.6464729Z   /__w/1/s/src/coreclr/pal/inc/rt/palrt.h:313:11: note: forward declaration of 'IStream'
2025-12-24T13:33:24.6465064Z     313 | interface IStream;
2025-12-24T13:33:24.6465312Z         |           ^
2025-12-24T13:33:24.6493301Z   In file included from /__w/1/s/src/coreclr/utilcode/memorystreams.cpp:11:
2025-12-24T13:33:24.6520610Z   /__w/1/s/src/coreclr/inc/memorystreams.h:127:9: error: unknown type name 'STATSTG'
2025-12-24T13:33:24.6521297Z     127 |         STATSTG     *pstatstg,
2025-12-24T13:33:24.6521909Z         |         ^
2025-12-24T13:33:24.6582947Z   /__w/1/s/src/coreclr/inc/memorystreams.h:31:7: error: use of undeclared identifier 'LIMITED_METHOD_CONTRACT'
2025-12-24T13:33:24.6587706Z      31 |     { LIMITED_METHOD_CONTRACT; }
2025-12-24T13:33:24.6589547Z         |       ^~~~~~~~~~~~~~~~~~~~~~~
2025-12-24T13:33:24.6664254Z   /__w/1/s/src/coreclr/inc/memorystreams.h:39:9: error: use of undeclared identifier 'LIMITED_METHOD_CONTRACT'
2025-12-24T13:33:24.6666332Z      39 |         LIMITED_METHOD_CONTRACT;

Please address them and make sure that the build succeeds.

jkotas
jkotas reviewed Dec 27, 2025
View reviewed changes
Copilot AI requested a review from jkotas December 27, 2025 07:56
jkotas
jkotas reviewed Dec 27, 2025
View reviewed changes
jkotas
jkotas reviewed Dec 27, 2025
View reviewed changes
Copilot AI requested a review from jkotas December 27, 2025 08:12
@build-analysis build-analysis bot mentioned this pull request Dec 28, 2025
Open
@jkotas jkotas marked this pull request as ready for review December 28, 2025 13:57
Copilot AI review requested due to automatic review settings December 28, 2025 13:57
Copilot AI reviewed Dec 28, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR successfully refactors the metadata (MD) subsystem to eliminate VM dependencies on MD internal implementation details by relocating stgpool files, extracting memory stream classes, and reimplementing CDescPool.

Key changes:

  • Moved stgpool files from shared inc/utilcode to MD-specific directories (md/inc and md/runtime)
  • Extracted CInMemoryStream and CGrowableStream to new memorystreams.h/cpp files in inc/utilcode
  • Refactored CDescPool from StgPool inheritance to standalone allocator with flexible array members
  • Moved CPackedLen struct from stgpooli.h to mdfileformat.h for broader reusability

Reviewed changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/coreclr/vm/commtmemberinfomap.h Reimplemented CDescPool as standalone allocator with segmented memory management
src/coreclr/vm/commtmemberinfomap.cpp Updated CDescPool call sites to use size_t types and removed unnecessary null checks
src/coreclr/vm/common.h Changed include from stgpool.h to memorystreams.h
src/coreclr/vm/mlinfo.cpp Added mdfileformat.h include for CPackedLen
src/coreclr/utilcode/memorystreams.cpp New file containing CInMemoryStream and CGrowableStream implementations
src/coreclr/utilcode/CMakeLists.txt Removed stgpool files, added memorystreams.cpp
src/coreclr/md/runtime/stgpool.cpp Removed memory stream classes (moved to memorystreams.cpp)
src/coreclr/md/runtime/stgpooli.cpp New location for stgpooli implementation
src/coreclr/md/runtime/stgpoolreadonly.cpp New location for read-only pool implementation
src/coreclr/md/runtime/stdafx.h Added using declarations for std::min/max
src/coreclr/md/runtime/CMakeLists.txt Added stgpool files to MD runtime build
src/coreclr/md/inc/stgpool.h New location, removed memory stream class definitions
src/coreclr/md/inc/stgpooli.h New location with forward declaration for CPackedLen
src/coreclr/md/inc/streamutil.h Removed unused NullStream class
src/coreclr/md/compiler/regmeta_emit.cpp Added memorystreams.h include
src/coreclr/md/ceefilegen/cceegen.cpp Changed include from stgpool.h to memorystreams.h
src/coreclr/inc/stgpooli.h Deleted (moved to md/inc)
src/coreclr/inc/memorystreams.h New file with CInMemoryStream and CGrowableStream declarations
src/coreclr/inc/memoryrange.h Added contract.h include, removed blank lines
src/coreclr/inc/mdfileformat.h Added CPackedLen struct definition
src/coreclr/inc/formattype.cpp Removed Debug_ReportError call
src/coreclr/inc/caparser.h Changed include from stgpooli.h to mdfileformat.h
src/coreclr/ilasm/ilasmpch.h Removed stgpooli.h include (now in mdfileformat.h)
src/coreclr/debug/di/module.cpp Changed include from stgpool.h to memorystreams.h

@jkotas jkotas requested a review from jkoritzinsky December 28, 2025 14:06
@jkotas jkotas merged commit d21d7f7 into main Dec 30, 2025
116 checks passed
@jkotas jkotas deleted the copilot/remove-private-implementation-details branch December 30, 2025 14:03
This was referenced Jan 9, 2026
Closed
Merged
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@jkoritzinsky jkoritzinsky jkoritzinsky approved these changes

@jkotas jkotas Awaiting requested review from jkotas

Assignees

@jkotas jkotas

Copilot code review Copilot

Labels

area-VM-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Clean up src/coreclr/src/inc : remove private implementation details

5 participants

@jkotas @jkoritzinsky @am11