Skip to content

Conversation

@chhwang
Copy link
Contributor

@chhwang chhwang commented Dec 17, 2025

Remove custom delcarations inside std:: of which behaviors are undefined by the standard

Copy link
Contributor

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 undefined behavior by eliminating custom declarations inside the std:: namespace, which violates the C++ standard. The changes refactor std::to_string overloads to operator<< overloads and consolidate hash combining functionality into a centralized utility.

Key changes:

  • Introduced mscclpp::detail::hashCombine utility function to replace duplicate std::hash_combine implementations
  • Replaced std::to_string function declarations with operator<< overloads in the mscclpp namespace
  • Removed std::hash<TransportFlags> forward declaration (no longer needed)

Reviewed changes

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

Show a summary per file
File Description
include/mscclpp/utils.hpp Added hashCombine function in mscclpp::detail namespace with proper documentation
include/mscclpp/core.hpp Replaced std::to_string declarations with operator<< overload declarations in mscclpp namespace, removed std::hash<TransportFlags> forward declaration
src/core.cc Implemented operator<< overloads for Transport, DeviceType, and Device types
src/context.cc Updated to use operator<< instead of std::to_string for Transport types
python/csrc/core_py.cpp Updated Python binding to use stringstream with operator<< instead of std::to_string
src/include/execution_plan.hpp Replaced inline hash_combine calls with mscclpp::detail::hashCombine in hash specializations
src/executor/executor.cc Removed local hash_combine implementation, replaced with mscclpp::detail::hashCombine
include/mscclpp/algorithm.hpp Removed local hash_combine implementation, replaced with mscclpp::detail::hashCombine

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

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