Skip to content

Remove dead C++ wrappers left over from corehost C conversion - #131489

Open
elinor-fung wants to merge 2 commits into
dotnet:mainfrom
elinor-fung:remove-dead-corehost-cpp-wrappers
Open

Remove dead C++ wrappers left over from corehost C conversion#131489
elinor-fung wants to merge 2 commits into
dotnet:mainfrom
elinor-fung:remove-dead-corehost-cpp-wrappers

Conversation

@elinor-fung

@elinor-fung elinor-fung commented Jul 28, 2026

Copy link
Copy Markdown
Member

Following the apphost/nethost conversions to C, a handful of thin C++ wrappers in hostmisc (utils and pal) are no longer referenced by anything. This removes that dead code.

Note

This PR description was generated with assistance from GitHub Copilot.

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

Copy link
Copy Markdown
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 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_ROOT environment wrapper helpers from hostmisc/utils.{h,cpp}.
  • Remove unused pal::get_timestamp and pal::is_directory C++ wrappers from hostmisc/pal.{h,unix.cpp,windows.cpp}.
  • Keep the remaining C and C++ interop surfaces (e.g., pal_* C APIs and other pal:: 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

Comment thread src/native/corehost/hostmisc/utils.cpp Outdated
@@ -442,13 +421,6 @@ pal::string_t to_lower(const pal::char_t* in) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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)

Copilot AI review requested due to automatic review settings July 29, 2026 02:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot's findings

Comments suppressed due to low confidence (1)

src/native/corehost/hostmisc/utils.cpp:423

  • to_lower now lowercases each pal::char_t by calling minipal_tolower_invariant on a CHAR16_T-cast value. On Unix, pal::char_t is char and strings are UTF-8 bytes; if char is 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 (where pal::char_t is 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

@am11 am11 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. GCC failure is unrelated #131520.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants