Skip to content

[libcu++] Partially implement P2591R5 Concatenation of strings and string views - #10706

Open
davebayer wants to merge 1 commit into
NVIDIA:mainfrom
davebayer:string_view_concat
Open

[libcu++] Partially implement P2591R5 Concatenation of strings and string views#10706
davebayer wants to merge 1 commit into
NVIDIA:mainfrom
davebayer:string_view_concat

Conversation

@davebayer

Copy link
Copy Markdown
Contributor

Implements cuda::std::string_view interoperability with std::string (#10255).

@davebayer
davebayer requested a review from a team as a code owner August 7, 2026 09:18
@github-project-automation github-project-automation Bot moved this to Todo in CCCL Aug 7, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Review in CCCL Aug 7, 2026
@davebayer davebayer linked an issue Aug 7, 2026 that may be closed by this pull request
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for concatenating host std::basic_string values with cuda::std::basic_string_view values.
    • Concatenation works in either operand order, including lvalue and rvalue strings.
    • Supports custom character traits and multiple character types.
    • Preserves efficient in-place behavior when concatenating with movable strings.
  • Tests

    • Added coverage for result types, ordering, empty strings, character types, custom traits, and host runtime behavior.

Walkthrough

Changes

String-view interoperability

Layer / File(s) Summary
Hosted concatenation overloads
libcudacxx/include/cuda/std/string_view
Adds eight hosted operator+ overloads for std::basic_string and cuda::std::basic_string_view. Lvalue strings create reserved results. Rvalue strings are modified in place and moved into the result.
Interoperability validation
libcudacxx/test/libcudacxx/cuda/strings/std_interop/string.view.operator_plus.pass.cpp
Tests operand order, result types, values, empty strings, character traits, character types, constexpr support, and host-only execution.

Suggested reviewers: bernhardmgruber


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c72301c7-c0e4-45d5-b7cd-e3be4cc97113

📥 Commits

Reviewing files that changed from the base of the PR and between c959fc9 and c7c87ea.

📒 Files selected for processing (2)
  • libcudacxx/include/cuda/std/string_view
  • libcudacxx/test/libcudacxx/cuda/strings/std_interop/string.view.operator_plus.pass.cpp

Comment thread libcudacxx/include/cuda/std/string_view
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⏱️ CCCL compile-time benchmark comparison: Public headers compile-time bench

Result: 0 regression row(s), 6 improvement row(s) above threshold.

Run Value
Config public-headers-gcc13
Baseline origin/main
Preset all-dev
Targets cub.headers.base, thrust.cpp.cuda.headers.base, libcudacxx.test.public_headers
GPU / launch args rtx2080 / --cuda 13.3 --host gcc13

Artifacts: reports and traces

Direct file processing

-f file-processing exclusive --sort total

🟢 Direct file processing — Improvements
Rank Improvement impact Selected Δ Baseline Current Event Matched traces
1 1.258729 -1.258729 9.579898 8.321169 Processing Header File: libcudacxx/include/cuda/std/__cccl/prologue.h 550
2 0.532617 -0.532617 7.874751 7.342134 Processing Header File: cub/cub/util_type.cuh 279
3 0.349889 -0.349889 3.418533 3.068644 Processing Header File: libcudacxx/include/cuda/std/__cccl/epilogue.h 550
4 0.282890 -0.282890 6.040036 5.757146 Processing Header File: libcudacxx/include/cuda/std/__tuple_dir/vector_types.h 238
5 0.238046 -0.238046 3.653431 3.415385 Processing Header File: libcudacxx/include/cuda/std/__tuple_dir/tuple.h 243
6 0.211616 -0.211616 4.066801 3.855185 Processing Header File: libcudacxx/include/cuda/std/__utility/pair.h 404

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

😬 CI Workflow Results

🟥 Finished in 2h 21m: Pass: 95%/115 | Total: 3d 22h | Max: 2h 15m | Hits: 48%/861037

See results here.

AI failure analysis

1. NVCC/MSVC rejects custom-traits string concatenation during constant evaluation · 5 jobs

Explanation: All five C++20 Windows matrices fail compiling the new string-view concatenation test at static_assert(test()). The NVCC constant evaluator reaches cuda::std::char_traits::move through std::basic_string::append and rejects a relational comparison between pointers to unrelated objects.

Evidence:

libcu++ nvcc MSVC / [CTK12.0 MSVC14.39 C++20] Build(amd64), step 4

2026-08-07T10:30:05.8421132Z FAIL: libcu++ :: cuda/strings/std_interop/string.view.operator_plus.pass.cpp (644 of 3787)
2026-08-07T10:30:08.2796857Z C:\cccl\libcudacxx\test\libcudacxx\cuda\strings\std_interop\string.view.operator_plus.pass.cpp(176): error: expression must have a constant value

libcu++ nvcc MSVC / [CTK13.3 MSVC14.44 C++20] Build(amd64), step 4

2026-08-07T10:49:55.0046998Z FAIL: libcu++ :: cuda/strings/std_interop/string.view.operator_plus.pass.cpp (651 of 3787)

Root cause: The new constexpr test instantiates std::basic_string with CustomCharTraits derived from cuda::std::char_traits. The added operator+ calls std::basic_string::append; MSVC then calls CustomCharTraits::move, whose constexpr memmove implementation tests __src < __dst even though the source literal and destination string storage are unrelated, which NVCC correctly refuses as a constant expression. Sources: libcudacxx/test/libcudacxx/cuda/strings/std_interop/string.view.operator_plus.pass.cpp:45, libcudacxx/test/libcudacxx/cuda/strings/std_interop/string.view.operator_plus.pass.cpp:148, libcudacxx/test/libcudacxx/cuda/strings/std_interop/string.view.operator_plus.pass.cpp:177, libcudacxx/include/cuda/std/string_view:880, libcudacxx/include/cuda/std/__string/constexpr_c_functions.h:409.

Suggested next steps: Reproduce only string.view.operator_plus.pass.cpp with an NVCC/MSVC C++20 configuration and determine whether __cccl_memmove_impl_constexpr can avoid ordering unrelated pointers. If that helper cannot be made conforming for this toolchain, exclude only the custom-traits portion from NVCC/MSVC constant evaluation while retaining its runtime coverage and the default-traits static assertion; then rerun the targeted libcudacxx.test.lit.precompile target.

Copy this prompt into a coding agent
Repository: https://github.com/NVIDIA/cccl
Workflow run: https://github.com/NVIDIA/cccl/actions/runs/31165326903
Failure group: NVCC/MSVC rejects custom-traits string concatenation during constant evaluation
Affected jobs:
- libcu++ nvcc MSVC / [CTK12.0 MSVC14.39 C++20] Build(amd64): https://github.com/NVIDIA/cccl/actions/runs/31165326903/job/92824784250
- libcu++ nvcc MSVC / [CTK13.3 MSVC14.44 C++20] Build(amd64): https://github.com/NVIDIA/cccl/actions/runs/31165326903/job/92824784321
- libcu++ nvcc MSVC / [CTK13.0 MSVC14.44 C++20] Build(amd64): https://github.com/NVIDIA/cccl/actions/runs/31165326903/job/92824784408
- libcu++ nvcc MSVC / [CTK13.3 MSVC14.50 C++20] Build(amd64): https://github.com/NVIDIA/cccl/actions/runs/31165326903/job/92824784525
- libcu++ nvcc MSVC / [CTK12.9 MSVC14.44 C++20] Build(amd64): https://github.com/NVIDIA/cccl/actions/runs/31165326903/job/92824784555

Reproduce the C++20 NVCC/MSVC failure in libcudacxx/test/libcudacxx/cuda/strings/std_interop/string.view.operator_plus.pass.cpp narrowly. Trace the custom-traits constexpr path from operator+ in libcudacxx/include/cuda/std/string_view through std::basic_string::append, cuda::std::char_traits::move, and __cccl_memmove_impl_constexpr at constexpr_c_functions.h:409. Implement the smallest conforming fix: prefer correcting the constexpr memmove overlap detection if it can avoid relational comparison of unrelated pointers across supported compilers; otherwise restructure the test so NVCC/MSVC still statically validates the default-traits overloads but runs CustomCharTraits cases only at runtime. Do not disable the complete test. Run the single lit test or libcudacxx.test.lit.precompile with an NVCC/MSVC C++20 configuration, plus focused constexpr memmove tests if the helper changes.

Jobs:

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

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

[FEA]: P2591R5 Concatenation of strings and string views

1 participant