Disable, by default, CLRConfig settings on DAC builds.
#114234
Conversation
The majority of the CLRConfig settings should have no impact on the code in the DAC. During investigation 4 where found but only 2 were still used. This PR removes the 2 unused and creates a new flag that that indicates the setting is valid for the DAC. All others fallback to their default value.
There was a problem hiding this comment.
Pull Request Overview
This PR disables most CLRConfig settings for DAC builds by removing unused settings and adding a new flag to indicate DAC validity. Key changes include updating configuration initialization to use boolean flags, adding conditional DAC checks in CLRConfig routines, and removing obsolete method sets.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/utilcode/util_nodependencies.cpp | Updates ConfigDWORD/ConfigString initialization using booleans and removes outdated assertions. |
| src/coreclr/utilcode/util.cpp | Removes unused ConfigMethodSet functions. |
| src/coreclr/utilcode/clrconfig.cpp | Adds DAC-specific checks for config settings via conditional compilation. |
| src/coreclr/inc/utilcode.h | Updates classes to use final keyword and bool for initialization state. |
| src/coreclr/inc/clrconfigvalues.h | Reworks config macros to conditionally include DAC-specific settings. |
| src/coreclr/inc/clrconfig.h | Introduces a new flag for DAC build validity. |
|
Tagging subscribers to this area: @tommcdon |
|
/cc @dotnet/dotnet-diag |
AaronRobinsonMSFT
added
the
NO-MERGE
|
@dotnet/dotnet-diag I'd like to get confirmation on the following sets of Keep Remove |
|
Making these fail at compile time is not useful nor going to result in something simple. I'm closing this PR. |
The majority of the
CLRConfigsettings should have no impact on the code in the DAC. During investigation 4 where found but only 2 were still used. This PR removes the 2 unused and creates a new flag that that indicates the setting is valid for the DAC. All others fallback to their default value.Addresses future issues like #106148.
I would have preferred to assert this state, but there felt like too many places where the querying of a
CLRConfigsetting is performed. I found three on a simple test case and after considering scenarios like mixed mode, EnC scenarios, et al, I decided to simply log it.