refactor(Storage): collapse at<T>/back<T>/data<T> into single template bodies

[yingjerkao]

Summary

Each of Storage_base::at<T>, back<T>, and data<T> was 11 hand-written explicit specializations with identical bodies, differing only in the dtype enum and the type spelling inside the error message. That copy-paste already let one dtype check drift out of sync (#965, fixed by #978). This collapses each family into a single template body:

• The expected dtype comes from the existing compile-time trait Type_class::cy_typeid_v<T>, so the dtype check now exists in exactly one place per function and cannot drift per-type again.
• A file-local type_spelling<T> trait supplies the C++ type spelling embedded in the error messages ("<float>", "< std::complex<double> >", ...), preserving the existing wording byte-for-byte.
• Explicit instantiations for the same 11 types keep the exported symbols and the header declaration (include/backend/Storage.hpp) unchanged.
• The GPU-only data<cuDoubleComplex> / data<cuFloatComplex> specializations keep their extra device check and remain hand-written.

Net: −413/+82 lines, no functional change.

Stacked on #978 (base: fix/storage-at-unconditional-dtype-check) since it touches the same lines; retarget to master after that merges.

Verification

• nm diff of the old vs new object file: all 33 mangled symbols identical (binding goes strong → weak, the normal consequence of explicit instantiation definitions; the body is only visible in this TU, so these remain the only definitions).
• Error-message check: a probe triggering all 11 dtype mismatches plus the out-of-bound, empty-back, and data mismatch errors, linked against both the old and new implementation — what() message lines are byte-identical in every case. Only the # Cytnx error occur at ... diagnostic line changes rendering (template pretty-function instead of the specialization's), which nothing asserts on.
• Storage test suite: 284 passed / 11 pre-existing skips / 0 failed (exit 0), including StorageTest.AtDtypeMismatchThrows* and StorageTest.AtOutOfBoundMessageReportsIndexAndSize run explicitly; Tensor suite 9/9.
• clang-format 14 (the CI-pinned version) reports no violations.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request refactors Storage_base.cpp by replacing repetitive explicit template specializations of data<T>, at<T>, and back<T> with unified template implementations and explicit instantiations, utilizing a type_spelling variable template for error messages. Feedback suggests enforcing compile-time safety for type_spelling using a helper struct with static_assert to prevent potential runtime undefined behavior if an unspecialized type is instantiated.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Currently, the primary template of type_spelling defaults to nullptr. If a new type is added to the explicit instantiations of data<T>, at<T>, or back<T> but is not specialized in type_spelling, it will compile successfully but result in undefined behavior at runtime when cytnx_error_msg attempts to format a nullptr with %s.

To prevent this, we can enforce compile-time safety by making the primary template trigger a compilation error if it is instantiated. Since always_false_v is already defined in Type.hpp, we can use a helper struct with a static_assert to block unspecialized types at compile time.

Suggested change

	template <typename T>
	constexpr const char *type_spelling = nullptr;
	template <typename T>
	struct type_spelling_holder {
	static_assert(always_false_v<T>, "type_spelling is not specialized for this type");
	static constexpr const char *value = nullptr;
	};
	template <typename T>
	constexpr const char *type_spelling = type_spelling_holder<T>::value;

[yingjerkao]

Done in 79fb7d0. Routed the primary type_spelling template through a type_spelling_holder<T> with static_assert(always_false_v<T>, ...), so any type that reaches the at/back/data dtype-mismatch message without an explicit spelling is now a hard compile error instead of a silent nullptr fed to %s. The 11 specialized types are unchanged, and the hand-written GPU data<cuDoubleComplex>/data<cuFloatComplex> specializations never reference type_spelling, so they're unaffected. Verified: the CPU TU compiles clean with the real build flags, and an isolated probe confirms an unspecialized type now fails with "type_spelling is not specialized for this type".

[ianmccul]

Add to include/Type.hpp:

template <typename T> concept CytnxType = variant_contains_v<T, Type_list>;

Then the functions can use

template <CytnxType T>
T* Storage_base::data() const { ... }

and attempt to convert to a type that has no corresponding dtype will not compile. Not sure how that works with the cuDoubleComplex / cuFloatComplex specializations, since I don't think you can add a specialization to a template where the primary declaration is a concept but the specialization is not part of the concept. But I think you can probably do

template <typename T>
T* Storage_base:::data() const = delete; // unsupported T: compile-time error

template <CytnxType T>
T* Storage_base::data() const { ... }

#ifdef UNI_GPU
template <>
cuDoubleComplex* Storage_base::data<cuDoubleComplex>() const { ... }
template <>
cuFloatComplex* Storage_base::data<cuFloatComplex>() const { ... }
#endif

[yingjerkao]

Add to include/Type.hpp:

template <typename T> concept CytnxType = variant_contains_v<T, Type_list>;

Then the functions can use

template <CytnxType T>
T* Storage_base::data() const { ... }

and attempt to convert to a type that has no corresponding dtype will not compile. Not sure how that works with the cuDoubleComplex / cuFloatComplex specializations, since I don't think you can add a specialization to a template where the primary declaration is a concept but the specialization is not part of the concept. But I think you can probably do

template <typename T>
T* Storage_base:::data() const = delete; // unsupported T: compile-time error

template <CytnxType T>
T* Storage_base::data() const { ... }

#ifdef UNI_GPU
template <>
cuDoubleComplex* Storage_base::data<cuDoubleComplex>() const { ... }
template <>
cuFloatComplex* Storage_base::data<cuFloatComplex>() const { ... }
#endif

Are these in #1004

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.67%. Comparing base (d6dcd16) to head (f346f7b).
⚠️ Report is 101 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/backend/Storage_base.cpp	66.66%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #979       +/-   ##
===========================================
+ Coverage   31.82%   55.67%   +23.84%     
===========================================
  Files         230      230               
  Lines       33124    32979      -145     
  Branches    13852       77    -13775     
===========================================
+ Hits        10543    18360     +7817     
+ Misses      15539    14594      -945     
+ Partials     7042       25     -7017     

Flag	Coverage Δ
cpp	55.59% <66.66%> (+24.13%)	⬆️
python	61.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
C++ backend	53.34% <66.66%> (+20.79%)	⬆️
Python bindings	70.35% <ø> (+46.33%)	⬆️
Python package	61.84% <ø> (ø)

Files with missing lines	Coverage Δ
include/Type.hpp	97.56% <ø> (+43.90%)	⬆️
include/backend/Storage.hpp	95.26% <ø> (+14.20%)	⬆️
src/backend/Storage_base.cpp	27.84% <66.66%> (+3.47%)	⬆️

... and 172 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6dcd16...f346f7b. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ianmccul]

No, they would be an addition to this PR, they are not connected with #1004. Except with #1004 the overloads for the GPU types should be Storage_base::data<cuda::std::complex<T>>() -- actually you could have both the cuda::std::complex and cuComplex versions, and it probably makes sense to do this.

[yingjerkao]

Done in 867e5bf (rebased onto master first, so #1004's cuda::std::complex is in the base).

Added the concept to Type.hpp:

template <typename T>
concept CytnxType = variant_contains_v<T, Type_list> && !std::is_void_v<T>;

(excluding the Void placeholder, which is in Type_list but isn't a real element type).

data/at/back now use exactly the = delete primary + constrained-overload pattern you sketched:

template <typename T> T* data() const = delete;   // unsupported T -> hard error
template <CytnxType T> T* data() const;

so e.g. data<char>() fails at the call site with "use of deleted function T* Storage_base::data() const [with T = char]" rather than falling through to the cy_typeid_v/type_spelling diagnostics. It composes with the earlier type_spelling static_assert, which still catches a CytnxType added without a spelling.

Per your second comment, the GPU complex overloads are provided both ways (they're explicit specializations of the deleted primary, since neither is a cytnx dtype):

• data<cuDoubleComplex>() / data<cuFloatComplex>() — cuBLAS/cuSOLVER ABI pointers (kept)
• data<cuda::std::complex<double>>() / data<cuda::std::complex<float>>() — the representation Use cuda::std::complex for typed GPU complex dispatch #1004 uses inside GPU kernels (new)

at/back get no GPU specializations — they were never valid for the cu* types (the old unconstrained template would already have failed in variant_index).

Verified: full CPU library + test suite build; Storage/Type/Tensor suites 300 passed / 0 failed (11 pre-existing skips); data<char>() correctly rejected; and Storage_base.cpp compiles under UNI_GPU (with the CCCL include path) so all four GPU data<> specializations type-check. I couldn't run a full GPU link here because cuKron is currently broken under CUDA 13 (#999), unrelated to this change.

[ianmccul]

I thought #1004 fixed #999 ?

[yingjerkao]

Follow-up on the wrapper, @ianmccul — now fully wired through, in two commits on top of the concept change:

1. Constrained the Storage wrapper too (92f86060). The thin Storage::data<T>/at<T>/back<T> delegators were still template <class T>, so an unsupported T errored one level deep in the wrapper body. All three now carry the constraint, so storage.data<char>() fails right at the call site with constraints not satisfied … CytnxType<T>.

2. Made the GPU views actually reachable (a70638f8). While constraining the wrapper I found the GPU data<> specializations were only defined in Storage_base.cpp and never declared in Storage.hpp — so from any other TU _impl->data<cuDoubleComplex>() resolved to the deleted primary. They were unreachable outside that one file (true on master too; there were no callers). So I:

• declared all four specializations in Storage.hpp at namespace scope before the wrapper, and
• widened the data wrapper constraint to a StorageDataType concept = CytnxType ∪ the four GPU views.

Now storage.data<cuDoubleComplex>(), data<cuFloatComplex>(), and data<cuda::std::complex<double/float>>() compile and link end-to-end, while data<char>() is still rejected. at/back stay CytnxType-only (they have no GPU specializations — the old unconstrained template would already have failed in variant_index).

Verified:

• GPU (UNI_GPU + CCCL include): the wrapper compiles all four GPU views and data<char>/at<cuDoubleComplex> are rejected; nm confirms Storage_base.cpp emits the four specializations as strong (T) symbols (double2, float2, both cuda::std::complex) matching the header declarations.
• CPU: full library + test suite build; Storage suite 268 passed / 0 failed (11 pre-existing skips).

I couldn't run a full GPU link here because cuKron is currently broken under CUDA 13 (#999), unrelated to this change — but the per-TU compile + symbol check covers the declaration↔definition contract.

[pcchen]

Code Review

Collapses Storage_base::at<T>/back<T>/data<T> from 11 hand-written specializations each into single template bodies. The refactor itself is correct and well-executed — but it introduces a downstream-compat regression that must be fixed before merge.

The refactor is correct (verified in full)

• Single template bodies; the dtype check now comes from cy_typeid_v<T> — one source of truth, so the Storage::at<T>() and Tensor::item<T>() silently reinterpret mismatched dtype in normal builds #965-style per-type drift cannot recur. ✅
• type_spelling<T> (variable template with a static_assert default) reproduces the old inline spellings, and the new bodies are byte-faithful to the old ones (bounds check on at, empty check on back — even the "stoarge" typo preserved — cudaDeviceSynchronize, static_cast). The "...try to get %s type..." + type_spelling<T> renders byte-identical. ✅
• Concept-constrained declarations + an unconstrained = delete fallback → an unsupported T is now a clean compile-time error at the call site (was a link error). GPU data<cuDoubleComplex/cuFloatComplex> specializations are #ifdef UNI_GPU-guarded and header-declared (ODR-correct); StorageDataType is correctly used on the Storage wrapper's data<T> to admit them. ✅
• Explicit instantiations preserve all exported symbols (the nm diff in the description confirms). ✅

🔴 Blocker — breaks DownstreamFindPackage (CI-verified)

include/Type.hpp:194: error: 'concept' does not name a type; did you mean 'const'?
include/backend/Storage.hpp:55: error: 'CytnxType' has not been declared

This PR introduces the first C++20 concepts into a universally-included public header (Type.hpp), but cytnx sets C++20 only via the build-level CMAKE_CXX_STANDARD variable — it does not export the requirement to consumers. So find_package(cytnx) consumers compile the headers in their default (pre-C++20) mode and fail to parse concept. DownstreamFindPackage was green on the recent merges (#972/#977/#998), so this is a regression this PR surfaces.

Fix: have cytnx declare its language requirement to consumers on the installed/exported target, e.g.

target_compile_features(cytnx PUBLIC cxx_std_20)

rather than relying only on the build-level CMAKE_CXX_STANDARD. That's a CMake change not in this PR. Silver lining: it fixes a genuine latent gap — the public headers already require C++20 (std::format, std::source_location); the concept keyword just makes it undeniable, and exporting the requirement benefits every downstream consumer.

Notes

• Behind master but MERGEABLE; BuildAndTest is green on Linux + macOS (cytnx's own build is C++20). macOS-15-intel + wheels were still pending.

Recommendation: request changes — the dedup + API hardening is good and should land; it just needs the PUBLIC cxx_std_20 export (or equivalent) so DownstreamFindPackage passes.

Posted by Claude Code on behalf of @pcchen

[yingjerkao]

@ianmccul You're right, and my two "cuKron still broken" notes were stale — I was building a base that predated the fix. #1004 fixed it: commit 7bbd814 rewrote the kernel line to compute in the output dtype (out[i] = TO(Lin[x]) * TO(Rin[y])), and #1019 then deleted cucomplex_arithmetic outright, so the cuFloatComplex × real → cuFloatComplex overloads at the root of #994/#999 are gone. Confirmed cuKron compiles on current master — I've closed #999 and #994 (and superseded PR #995) accordingly.

[yingjerkao]

@pcchen Confirmed and fixed in this PR (f346f7b). DownstreamFindPackage was red for exactly the reason you diagnosed: this is the first concept in a universally-included public header (Type.hpp), which turns the already-latent C++20 requirement (std::format, std::source_location) into a hard parse error for consumers that compile our headers in their default standard. We only set CMAKE_CXX_STANDARD at build level and never exported it.

Applied your recommended fix on the cytnx target:

target_compile_features(cytnx PUBLIC cxx_std_20)

so find_package(cytnx) consumers inherit -std=c++20. As you noted, this also closes the pre-existing latent gap rather than just papering over the concept keyword. CI is re-running; expecting DownstreamFindPackage to go green now. Thanks for the thorough review.

[ianmccul]

LGTM