Remove dead C++ wrappers left over from corehost C conversion - #131489
Open
elinor-fung wants to merge 2 commits into
Open
Remove dead C++ wrappers left over from corehost C conversion#131489elinor-fung wants to merge 2 commits into
elinor-fung wants to merge 2 commits into
Conversation
Following the apphost/nethost C conversions, several thin C++ wrappers in hostmisc are now unreferenced: - utils: get_dotnet_root_from_env (C callers use utils_get_dotnet_root_from_env directly), get_dotnet_root_env_var_for_arch, and to_upper - pal: is_directory (thin wrapper over pal_directory_exists, which is still used by fxr_resolver.c) and get_timestamp Remove the dead declarations and definitions. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8d8340e7-36da-4a97-9ce4-72da8beffa39
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes unused C++ wrapper APIs in src/native/corehost/hostmisc (utils/pal) that are no longer referenced after the corehost-side C conversions, reducing dead code without intended behavioral impact.
Changes:
- Remove unused
DOTNET_ROOTenvironment wrapper helpers fromhostmisc/utils.{h,cpp}. - Remove unused
pal::get_timestampandpal::is_directoryC++ wrappers fromhostmisc/pal.{h,unix.cpp,windows.cpp}. - Keep the remaining C and C++ interop surfaces (e.g.,
pal_*C APIs and otherpal::helpers) unchanged.
Show a summary per file
| File | Description |
|---|---|
| src/native/corehost/hostmisc/utils.h | Removes unused C++ wrapper declarations for DOTNET_ROOT env var helpers and to_upper. |
| src/native/corehost/hostmisc/utils.cpp | Removes unused wrapper implementations for DOTNET_ROOT env var helpers and to_upper. |
| src/native/corehost/hostmisc/pal.h | Removes unused C++ wrapper declarations for get_timestamp and is_directory. |
| src/native/corehost/hostmisc/pal.windows.cpp | Removes unused Windows implementation of pal::get_timestamp and pal::is_directory. |
| src/native/corehost/hostmisc/pal.unix.cpp | Removes unused Unix implementation of pal::get_timestamp and pal::is_directory. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 0
jkoritzinsky
approved these changes
Jul 28, 2026
am11
reviewed
Jul 28, 2026
| @@ -442,13 +421,6 @@ pal::string_t to_lower(const pal::char_t* in) { | |||
Member
There was a problem hiding this comment.
nit: we can use the minipal version that's generated by ICU raw data (covers the 16-bit range, or 216 values). It is much more reliable for international text than ASCII-only tolower:
Suggested change
| [](pal::char_t c) { return static_cast<pal::char_t>(minipal_tolower_invariant(static_cast<CHAR16_T>(c))); }); |
(remember to add #include <minipal/strings.h>; obj is already linked into corehost)
This was referenced Jul 29, 2026
Contributor
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/native/corehost/hostmisc/utils.cpp:423
to_lowernow lowercases eachpal::char_tby callingminipal_tolower_invarianton aCHAR16_T-cast value. On Unix,pal::char_tischarand strings are UTF-8 bytes; ifcharis unsigned on a given platform, bytes in the range 0x80-0xFF can be interpreted as U+00xx and get Unicode-lowercased, potentially corrupting UTF-8 data. This should only apply Unicode/UTF-16 lowercasing on Windows (wherepal::char_tis UTF-16), and use an ASCII-only invariant fold on Unix to avoid mutating non-ASCII bytes.
pal::string_t to_lower(const pal::char_t* in) {
pal::string_t ret = in;
std::transform(ret.begin(), ret.end(), ret.begin(),
[](pal::char_t c) { return static_cast<pal::char_t>(minipal_tolower_invariant(static_cast<CHAR16_T>(c))); });
return ret;
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following the apphost/nethost conversions to C, a handful of thin C++ wrappers in
hostmisc(utilsandpal) are no longer referenced by anything. This removes that dead code.Note
This PR description was generated with assistance from GitHub Copilot.