Skip to content

Fix compile errors from device_scalar constructor signature change. - #5042

Merged
mythrocks merged 1 commit into
NVIDIA:mainfrom
mythrocks:rmm-device-scalar-ctor
Aug 26, 2026
Merged

mythrocks merged 1 commit into
NVIDIA:mainfrom
mythrocks:rmm-device-scalar-ctor

Conversation

@mythrocks

@mythrocks mythrocks commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Fixes #5041.

rapidsai/rmm#2527 changes the signature of rmm::device_scalar's constructor, so as not to take r-value references.

decimal_utils.cu and charset_decode.cu were mistakenly passing r-values to rmm::device_scalar's constructor. This is a bug, because there is no guarantee that the r-value would be alive by the time its value is read and copied to device memory.

This commit changes the call sites to use l-value references.

Note that stream.sync()s aren't required here, immediately after the device_scalars are constructed. The source objects remain alive for the copy, and there are stream syncs downstream
(e.g. flag->value(stream)).

Fixes NVIDIA#5041.

rapidsai/rmm#2527 changes the signature of
`rmm::decimal_scalar`'s constructor, so as not to take r-value
references.

`decimal_utils.cu` and `charset_decode.cu` were mistakenly passing
r-values to `rmm::decimal_scalar`'s constructor.  This is a bug,
because there is no guarantee that the r-value would be alive by the
time its value is read and copied to device memory.

This commit changes the call sites to use l-value references.

Note that `stream.sync()`s aren't required here, immediately after the
device_scalars are constructed.  The source objects remain alive for
the copy, and there are stream syncs downstream
(e.g. flag->value(stream)).

Signed-off-by: MithunR <mithunr@nvidia.com>
@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR updates cuDF and RMM revisions to incorporate the revised rmm::device_scalar constructor contract, then replaces two temporary initializer arguments with function-scoped l-values.

  • Initializes the charset decoding error flag from a named int32_t value.
  • Initializes the decimal conversion failure-row sentinel from a named cudf::size_type value.
  • Advances the cuDF submodule and corresponding RMM dependency pin.

Confidence Score: 5/5

The PR appears safe to merge, with no concrete correctness, security, or build failure identified in the changed code.

The named scalar initializer values remain in scope until downstream synchronized reads, and the dependency and call-site updates consistently address the constructor signature change.

Important Files Changed

Filename Overview
src/main/cpp/src/charset_decode.cu Replaces the temporary report-flag initializer with a function-scoped l-value that remains alive through the synchronized flag read.
src/main/cpp/src/decimal_utils.cu Replaces the temporary failure-row sentinel with a function-scoped l-value that remains alive through result retrieval.
thirdparty/cudf Advances the cuDF submodule revision associated with the upstream RMM API change.
thirdparty/cudf-pins/versions.json Advances the pinned RMM revision used by the native dependency build.

Reviews (1): Last reviewed commit: "Fix compile errors from decimal_scalar c..." | Re-trigger Greptile

@mythrocks

Copy link
Copy Markdown
Collaborator Author

Build

@mythrocks mythrocks changed the title Fix compile errors from decimal_scalar constructor signature change. Fix compile errors from device_scalar constructor signature change. Aug 26, 2026
@mythrocks
mythrocks merged commit 238b356 into NVIDIA:main Aug 26, 2026
7 checks passed
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.

[BUG] [BUILD] Compile error in decimal_utils.cu, charset_decode.cu

3 participants