Add non-template ref-to-value converting constructor to __basic_any - #8320
Conversation
The __basic_any value specialization inherits converting constructors from templates that accept __basic_any<_OtherInterface>. When a proxy type wraps a __basic_any reference type (e.g. __basic_any<I&>) and provides operator __basic_any<I&>&(), these template constructors cannot match: template argument deduction deduces the proxy type, not the underlying __basic_any, and implicit user-defined conversions are not considered during deduction. Add a non-template constructor __basic_any(__basic_any<_Interface&> const&) for the ref-to-value conversion case. Non-template parameters participate in implicit conversion sequences, allowing proxy types to convert through their user-defined conversion operator. This fixes any_resource construction from proxy-wrapped resource_ref and any_synchronous_resource from proxy-wrapped synchronous_resource_ref without requiring per-type overloads. Fixes NVIDIA#8316
…d refs Add regression tests for NVIDIA#8316 verifying that any_resource and any_synchronous_resource can be constructed from a value_proxy<T> that wraps resource_ref or synchronous_resource_ref via operator T&(), mimicking Cython's __Pyx_FakeReference proxy pattern.
Co-authored-by: Eric Niebler <eniebler@boost.org>
This comment has been minimized.
This comment has been minimized.
Work around CCCL issue where any_resource cannot be constructed from Cython's __Pyx_FakeReference proxy type wrapping resource_ref. Declare any_resource and device_accessible in .pxd, add inline make_any_device_resource helper. Update all 12 constructor call sites in .pyx to use the helper. Ref: NVIDIA/cccl#8320
- Add enable_if<__copyable> to the non-template converting constructors from __basic_any<_Interface&>, since converting a ref to a value requires copying the referenced object. - Add the corresponding move constructor from __basic_any<_Interface&>&&. - Update the template move converting constructor's same_as guard to use reference comparison (same_as<T&, U&>) for consistency with the copy converting constructor. - In tests, use CHECK((expr)) to prevent Catch2 expression decomposition from triggering an nvcc auto NTTP SFINAE bug via ADL. - Guard proxy construction tests with _CCCL_CUDA_COMPILER(NVCC, <, 12, 9) since older nvcc treats auto NTTP deduction failures as hard errors. - Add rvalue proxy test sections.
| # if !_CCCL_CUDA_COMPILER(NVCC, <, 12, 9) | ||
| // nvcc before CTK 12.9 has a bug where auto NTTP deduction failures in | ||
| // __satisfies are hard errors instead of SFINAE during overload resolution. |
There was a problem hiding this comment.
Just raising a flag here. It seems like CUDA 12.0 didn't work with these changes. I wasn't able to come to a good workaround, but maybe someone else will have ideas. This is the MRE (sadly not very minimal): https://gist.github.com/bdice/0704486a9d32bceb782acd931c823123
I can't give a Godbolt reproducer link because it requires the changes from this PR...
There was a problem hiding this comment.
seems reasonable to exclude the tests that trigger a since-fixed compiler bug. particularly one that is hard to work around, like this one.
EDIT: i wonder if it is the same issue that i worked around here: https://github.com/NVIDIA/cccl/blob/main/libcudacxx/include/cuda/__utility/__basic_any/basic_any_value.h#L254-L255
🥳 CI Workflow Results🟩 Finished in 2h 08m: Pass: 100%/108 | Total: 2d 10h | Max: 2h 08m | Hits: 95%/281975See results here. |
| __convert_from(__other); | ||
| } | ||
|
|
||
| template <bool _Copyable = __copyable, ::cuda::std::enable_if_t<_Copyable, int> = 0> |
There was a problem hiding this comment.
Should this be _CCCL_REQUIRES?
There was a problem hiding this comment.
No, this was intentional. The __basic_any class uses raw enable_if_t instead of _CCCL_TEMPLATE/_CCCL_REQUIRES to maintain ABI compatibility across C++17 and C++20. The _CCCL_REQUIRES macro expands to a requires-clause in C++20 but to additional SFINAE template parameters in C++17, which produces different mangled names depending on the dialect. This caused runtime failures (vtable cast assertions) when any_resource was used across DSO boundaries compiled with different -std= flags. See #7397 for the original bug report and #7401 / #7405 for the fix.
There was a problem hiding this comment.
we only needed to use enable_if for the functions whose ptrs actually end up in the pseudo-vtable (the ones marked _CCCL_PUBLIC_API). for these constructors, _CCCL_REQUIRES would not cause the ABI issue described in #7397, and it would be consistent with the other constructors in this class.
Change all resource/adaptor constructors from device_async_resource_ref to cuda::mr::any_resource<cuda::mr::device_accessible> taken by value, with std::move into member storage. This follows the sink-parameter idiom (like shared_ptr), enabling move semantics for rvalue arguments and making ownership transfer explicit. Rename upstream_mr/upstream_resource to upstream for consistency. Cython bindings use a make_any_device_resource inline helper to work around CCCL template deduction issue with Cython's FakeReference proxy. Ref: NVIDIA/cccl#8320
…rs (#2354) ## Description Change all resource/adaptor constructors from `device_async_resource_ref` to `cuda::mr::any_resource<cuda::mr::device_accessible>` taken **by value**, with `std::move` into member storage. This follows the sink-parameter idiom (like `std::shared_ptr`), enabling move semantics for rvalue arguments and making ownership transfer explicit. The central motivation is that `device_async_resource_ref` is a non-owning reference that cannot bind to temporaries (rvalues/xvalues). Code like `pool_memory_resource(cuda_memory_resource(), size)` is impossible with `device_async_resource_ref` because the temporary `cuda_memory_resource` is destroyed before the adaptor can use it. `any_resource<device_accessible>` by value solves this: callers pass a resource that gets moved into type-erased owned storage, so the adaptor owns its upstream and no external lifetime management is needed. Also renames `upstream_mr` / `upstream_resource` to `upstream` for consistency across all adaptors. Cython bindings use a `make_any_device_resource` inline helper to work around a CCCL template deduction issue where `any_resource` cannot be constructed from Cython's `__Pyx_FakeReference` proxy type wrapping `resource_ref` ([NVIDIA/cccl#8320](NVIDIA/cccl#8320)). This workaround should be removed once CCCL merges the upstream fix. **Testing:** - C++ build: 178/178 targets - C++ tests: 103/103 passed - Python tests: 1785 passed, 1 skipped - All against stock (unpatched) CCCL ## Checklist - [x] I am familiar with the [Contributing Guidelines](https://github.com/rapidsai/rmm/blob/HEAD/CONTRIBUTING.md). - [x] New or existing tests cover these changes. - [x] The documentation is up to date with these changes.
…VIDIA#8320) * Add non-template ref-to-value converting constructor to __basic_any The __basic_any value specialization inherits converting constructors from templates that accept __basic_any<_OtherInterface>. When a proxy type wraps a __basic_any reference type (e.g. __basic_any<I&>) and provides operator __basic_any<I&>&(), these template constructors cannot match: template argument deduction deduces the proxy type, not the underlying __basic_any, and implicit user-defined conversions are not considered during deduction. Add a non-template constructor __basic_any(__basic_any<_Interface&> const&) for the ref-to-value conversion case. Non-template parameters participate in implicit conversion sequences, allowing proxy types to convert through their user-defined conversion operator. This fixes any_resource construction from proxy-wrapped resource_ref and any_synchronous_resource from proxy-wrapped synchronous_resource_ref without requiring per-type overloads. Fixes NVIDIA#8316 * Add tests for any_resource/any_synchronous_resource from proxy-wrapped refs Add regression tests for NVIDIA#8316 verifying that any_resource and any_synchronous_resource can be constructed from a value_proxy<T> that wraps resource_ref or synchronous_resource_ref via operator T&(), mimicking Cython's __Pyx_FakeReference proxy pattern. * Update libcudacxx/include/cuda/__utility/__basic_any/basic_any_value.h Co-authored-by: Eric Niebler <eniebler@boost.org> * Constrain ref-to-value constructors, add move overload, fix tests - Add enable_if<__copyable> to the non-template converting constructors from __basic_any<_Interface&>, since converting a ref to a value requires copying the referenced object. - Add the corresponding move constructor from __basic_any<_Interface&>&&. - Update the template move converting constructor's same_as guard to use reference comparison (same_as<T&, U&>) for consistency with the copy converting constructor. - In tests, use CHECK((expr)) to prevent Catch2 expression decomposition from triggering an nvcc auto NTTP SFINAE bug via ADL. - Guard proxy construction tests with _CCCL_CUDA_COMPILER(NVCC, <, 12, 9) since older nvcc treats auto NTTP deduction failures as hard errors. - Add rvalue proxy test sections. --------- Co-authored-by: Eric Niebler <eniebler@boost.org>
This PR updates CCCL to the latest 3.4.0 pre-release commit. Comparison of CCCL commits: NVIDIA/cccl@c5594eb...c936ad8 We specifically need these features/fixes for RAPIDS projects: - NVIDIA/cccl#8353 - NVIDIA/cccl#8272 - NVIDIA/cccl#8320 - NVIDIA/cccl#8486 Authors: - Niranda Perera (https://github.com/nirandaperera) - Bradley Dice (https://github.com/bdice) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1008
Description
closes #8316
Add a non-template converting constructor
__basic_any(__basic_any<_Interface&> const&)to the__basic_anyvalue specialization, enabling implicit conversion from proxy types that wrap a__basic_anyreference type.The
__basic_anyvalue specialization provides a forwarding constructor__basic_any(_Tp&&)and template converting constructors__basic_any(__basic_any<_OtherInterface> const&). When a proxy type wraps a__basic_any<I&>(e.g. aresource_ref) and providesoperator __basic_any<I&>&(), neither path works: the forwarding constructor deduces the proxy type which doesn't satisfy the interface, and the template converting constructors can't deduce_OtherInterfacefrom the proxy since implicit user-defined conversions are not considered during template argument deduction.This occurs concretely with Cython's
__Pyx_FakeReference<T>, which wraps intermediate C++ expression results and providesoperator T&(). Constructingany_resource<P...>from a Cython-wrappedresource_ref<P...>fails at compile time.The fix adds a non-template constructor for the same-interface ref-to-value conversion. Because the parameter is non-template, the compiler considers implicit conversion sequences through the proxy's
operator T&(). The existing template converting constructor gains an additional!same_as<_OtherInterface, _Interface&>constraint to avoid ambiguity.This single fix in
__basic_anycovers all downstream types:any_resource,any_synchronous_resource, and any future__basic_any-based value types.Testing: A new test was added based on the minimal reproducer in the issue. RMM Cython bindings build and pass tests using direct
any_resourceconstruction fromresource_refwithout workaround helpers.Checklist