Fix correctness defects in the std::pmr allocator conversion patches - #19
Open
pete-tempo wants to merge 1 commit into
Open
Fix correctness defects in the std::pmr allocator conversion patches#19pete-tempo wants to merge 1 commit into
pete-tempo wants to merge 1 commit into
Conversation
Four fixes surfaced by an audit of the allocator/RTTI patches: 1. rclcpp.patch (context.hpp): make Context::sub_contexts_ keyed by std::string unconditionally instead of std::type_index under RTTI / std::string under _LIBCPP_HAS_NO_RTTI. The library is built with RTTI while Unreal consumers define _LIBCPP_HAS_NO_RTTI, so the RTTI-dependent member type gave the same class two different layouts (sizeof/offsets) across the boundary -- an ODR/ABI mismatch. get_sub_context now derives a string key in both branches (class_name under no-RTTI, typeid(...).name() otherwise), so the only RTTI dependence is key derivation, never layout. 2. rmw_cyclonedds.patch (TypeSupport2.hpp): ROSIDLCPP_U16StringValueType::type was left as std::u16string while its sibling string type was converted to std::pmr::string. sizeof_type() drives array/sequence striding in Serialization.cpp, so wstring arrays/sequences were strided by sizeof(std::u16string) over std::pmr::u16string data -- wrong offsets. Now std::pmr::u16string. 3. Fast-CDR.patch (Cdr.h): the bounded char-string-array deserializeArray overload was declared template<class T, class Traits, class Allocator> with T and Traits absent from the signature, making it non-deducible and uninstantiable. Reduced to template<class Allocator> to match its siblings. 4. image_common.patch (publisher.cpp): removed leftover std::cout debug output (and its <iostream> include) that was shipping in image_transport. Each patch was regenerated from source and verified to apply cleanly to a pristine submodule checkout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Four fixes surfaced by an audit of the allocator/RTTI patches:
rclcpp.patch (context.hpp): make Context::sub_contexts_ keyed by std::string unconditionally instead of std::type_index under RTTI / std::string under _LIBCPP_HAS_NO_RTTI. The library is built with RTTI while Unreal consumers define _LIBCPP_HAS_NO_RTTI, so the RTTI-dependent member type gave the same class two different layouts (sizeof/offsets) across the boundary -- an ODR/ABI mismatch. get_sub_context now derives a string key in both branches (class_name under no-RTTI, typeid(...).name() otherwise), so the only RTTI dependence is key derivation, never layout.
rmw_cyclonedds.patch (TypeSupport2.hpp): ROSIDLCPP_U16StringValueType::type was left as std::u16string while its sibling string type was converted to std::pmr::string. sizeof_type() drives array/sequence striding in Serialization.cpp, so wstring arrays/sequences were strided by sizeof(std::u16string) over std::pmr::u16string data -- wrong offsets. Now std::pmr::u16string.
Fast-CDR.patch (Cdr.h): the bounded char-string-array deserializeArray overload was declared template<class T, class Traits, class Allocator> with T and Traits absent from the signature, making it non-deducible and uninstantiable. Reduced to template to match its siblings.
image_common.patch (publisher.cpp): removed leftover std::cout debug output (and its include) that was shipping in image_transport.
Each patch was regenerated from source and verified to apply cleanly to a pristine submodule checkout.