Skip to content

Conversation

AaronRobinsonMSFT
Copy link
Member

No description provided.

Copy link
Contributor

@Copilot 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.

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (8)

src/coreclr/vm/i386/excepx86.cpp:829

  • The removal of the debug-only assertion in the OOM handling path may reduce diagnostic feedback in debug builds; ensure that this change is intended and that alternative diagnostics are in place if needed.
#if defined(_DEBUG)

src/coreclr/vm/codeman.cpp:5146

  • [nitpick] Consider renaming 'pwzJitNameMaybe' to a clearer name such as 'tempJitName' or 'jitNameTemp' to enhance code clarity.
LPWSTR pwzJitNameMaybe = NULL;

src/coreclr/vm/appdomain.cpp:922

  • The removal of the debug pause loop controlled by 'INTERNAL_PauseOnLoad' should be verified to ensure that it does not impact developers who rely on this mechanism during debugging.
#ifdef _DEBUG

src/coreclr/md/compiler/regmeta.h:2059

  • Removal of 'm_bKeepKnownCa' is appropriate for cleaning up unused configurations; please verify that no dependent logic assumes its presence.
bool        m_bKeepKnownCa;             // Should all known CA's be kept?

src/coreclr/md/compiler/regmeta.cpp:66

  • The elimination of 'm_bKeepKnownCa' initialization aligns with the header changes; ensure that metadata attribute processing continues to function as expected.
m_bKeepKnownCa(false),

src/coreclr/md/compiler/custattr_emit.cpp:1027

  • Updating attribute handling to use only 'props->bKeepCa' should be verified to ensure that removing 'm_bKeepKnownCa' does not alter the intended behavior for known custom attributes.
*bKeep = props->bKeepCa || m_bKeepKnownCa;

src/coreclr/inc/clrconfigvalues.h:111

  • The removal of unused config values like 'INTERNAL_EnableFullDebug' should be cross-checked with all modules to ensure no inadvertent dependencies remain.
CONFIG_DWORD_INFO(INTERNAL_EnableFullDebug, W("EnableFullDebug"), 0, "Heavy-weight checking for AD boundary violations (AD leaks)")

src/coreclr/inc/clrconfigvalues.h:392

  • Ensure that the coordinated removal of config values such as 'INTERNAL_MD_ForceNoColDesSharing' and 'INTERNAL_MD_KeepKnownCA' does not affect any legacy code paths or fallback behaviors.
RETAIL_CONFIG_DWORD_INFO(INTERNAL_MD_ForceNoColDesSharing, W("MD_ForceNoColDesSharing"), 0, "Don't know - the only usage I could find is #if 0 (?)")

Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

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.

Thanks

@jkotas jkotas merged commit 054949b into dotnet:main Apr 5, 2025
98 checks passed
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the remove_settings branch April 5, 2025 15:28
@github-actions github-actions bot locked and limited conversation to collaborators May 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants