fix: eliminate unnecessary string copies in EmitDeref and GetOrMaterialize#75
Closed
Copilot wants to merge 1 commit into
Closed
fix: eliminate unnecessary string copies in EmitDeref and GetOrMaterialize#75Copilot wants to merge 1 commit into
Copilot wants to merge 1 commit into
Conversation
…alize - Change EmitDeref's `inner` parameter from `std::string` to `std::string_view` to avoid copying a string that is only ever read via StrCat. Update the three call sites accordingly (two std::move calls become plain refs). - Change GetOrMaterialize's `materialize_fn` parameter from by-value to `const std::function<...>&` to avoid copying the callable object on each call. Agent-Logs-Url: https://github.com/Cpp2Rust/cpp2rust/sessions/a4a77a6f-20a8-49a8-98d1-507851373647 Co-authored-by: nunoplopes <2998477+nunoplopes@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
nunoplopes
May 11, 2026 17:02
View session
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.
Found and fixed two cases where unnecessary string copies were created in the C++ converter, identified via
performance-unnecessary-value-paramclang-tidy checks.Changes
Converter::EmitDeref—std::string→std::string_viewinnerwas passed by value, butStrCatonly reads it viaconst T&(its variadic template parameter). No ownership was needed, so every call site was paying for a heap allocation/copy that was immediately discarded. Changed tostd::string_viewand updated the three call sites (two of which had redundantstd::movecalls removed).TempMaterializationCtx::GetOrMaterialize—std::functionby value →const &The callable parameter
materialize_fnwas taken by value, causing the lambda (and thestd::functionwrapping it) to be copied on every call. Since the function only calls it once and never stores it, passing byconstreference avoids the copy entirely.