Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

Replace BaseString in util.h with std::string and std::basic_string

This PR replaces the custom BaseString template class with standard STL string types as suggested in the PR review comment from PR #5594.

Plan:

  • Explore repository structure and understand BaseString usage
  • Replace BaseString class template definition with std::string/std::basic_string
  • Update typedef declarations (String -> std::string, WString -> std::basic_string)
  • Update function signatures using WString to use std::basic_string
  • Update function implementations to use std::string methods (c_str(), length(), etc.)
  • Update string concatenation and operations to work with std::string
  • Update the Format class conversion operators to work with std::string/std::basic_string
  • Build the code to verify compilation
  • Address code review comments
  • Run security checks
  • Request code review

Changes made:

  • Removed the custom BaseString template class from util.h (150+ lines of code)
  • Changed String typedef from BaseString<char, strlen, strcpy_s> to std::string
  • Changed WString typedef from BaseString<WCHAR, _wcslen, wcscpy_s> to std::basic_string<WCHAR>
    • Note: WCHAR is char16_t on Linux/PAL platforms, so WString is std::basic_string<char16_t> on Linux
  • Updated GetLength() calls to length() (std::string standard method)
  • Updated implicit conversions to use explicit .c_str() calls where needed
  • Updated string concatenation with Format objects to use explicit WString() constructor wrapping
    • Required: Unlike BaseString which had custom operator+ overloads, std::basic_string's operator+ doesn't trigger implicit conversions for user-defined types. The Format class has operator WString(), but it's not invoked when used with std::string's operator+. Therefore, explicit wrapping with WString() is necessary.
  • Updated TableOutput::WriteColumn methods to call .c_str() on string parameters
  • Used implicit conversion from ArrayHolder to const WCHAR* instead of explicit cast
  • When passing String objects to output functions (ExtOut, DMLOut, etc.), explicitly call .c_str() to convert to PCSTR
    • Applied consistently across all platform-specific sections (AMD64, X86, ARM, ARM64, RISCV64, LOONGARCH64)
  • Updated comments in Format class conversion operators to reflect the requirement for explicit String()/WString() wrapping when concatenating with Format objects

Testing:

  • Clean rebuild successful on Linux x64 Debug configuration
  • No security vulnerabilities detected by CodeQL
  • All code compiles without warnings or errors
Original prompt

Based on this comment: #5594 (comment), can you open a PR to replace BaseString in util.h with usages of stl::string


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits January 14, 2026 02:57
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI changed the title [WIP] Replace BaseString with usages of stl::string in util.h Replace custom BaseString with std::string and std::basic_string<WCHAR> Jan 14, 2026
Copilot AI requested a review from max-charlamb January 14, 2026 03:06
Copilot AI and others added 2 commits January 14, 2026 03:58
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI changed the title Replace custom BaseString with std::string and std::basic_string<WCHAR> Replace custom BaseString with std::string and std::basic_string Jan 14, 2026
Copilot AI and others added 2 commits January 14, 2026 04:09
…erloads

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
…ions

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Copilot AI requested a review from max-charlamb January 14, 2026 18:04
@max-charlamb max-charlamb marked this pull request as ready for review January 15, 2026 19:34
@max-charlamb max-charlamb requested a review from a team as a code owner January 15, 2026 19:34
Copy link

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 removes the custom BaseString template class (~150 lines) from util.h and replaces it with standard library string types (std::string and std::basic_string<WCHAR>). The change modernizes the codebase by leveraging STL instead of maintaining custom string implementations.

Changes:

  • Removed custom BaseString template class and replaced type definitions with std::string and std::basic_string<WCHAR>
  • Updated method calls from GetLength() to standard length() and added explicit c_str() calls where implicit conversions previously occurred
  • Added explicit WString() constructor wrapping for Format objects in string concatenation expressions to work with std::basic_string's operator+

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/SOS/Strike/util.h Removed BaseString template class, updated typedefs to use STL types, and added .c_str() calls in Format conversion operator and TableOutput methods
src/SOS/Strike/util.cpp Updated string concatenations with Format objects to use explicit WString() wrapping, changed GetLength() to length(), and added intermediate pointer variable for ArrayHolder
src/SOS/Strike/strike.cpp Added explicit WString() wrapping for Format object concatenations and updated all ExtOut calls to use .c_str() on String objects across all platform-specific sections

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…uirements

Co-authored-by: max-charlamb <44248479+max-charlamb@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants