Partial elimination of TMP from xutils.hpp and patches for dependencies by spectre-ns · Pull Request #2923 · xtensor-stack/xtensor

spectre-ns · 2026-06-07T12:29:19Z

Checklist

The title and commit message(s) are descriptive.
Small commits made to fix your PR have been squashed to avoid history pollution.
Tests have been added for new features or bug fixes.
API of new functions and classes are documented.

Description

With concepts and constexpr the library can be more readable and less reliant on recursion. This PR is the first of hopefully many that will flatten the template structure in xtensor logic without changing public interfaces or functionality. The goal is to make additional optimizations and functionality easier to reason about and implement moving forward.

Additionally, eliminating recursion should help compile times!

codspeed-hq · 2026-06-07T12:32:48Z

Merging this PR will not alter performance

✅ 255 untouched benchmarks

_{Comparing spectre-ns:sfinae-to-constexpr (2beb8c8) with master (dd857e5)}

spectre-ns · 2026-06-07T13:11:53Z

@Alex-PLACET @JohanMabille please let me know if these types of changes are in the direction you'd like to see xtensor move towards!

JohanMabille · 2026-06-08T07:57:32Z

@spectre-ns thanks for tackling this!

let me know if these types of changes are in the direction you'd like to see xtensor move towards!

Yes and "maybe", depending on the primitives we're talking about:

I think all the has_xxx metafunctions could actually just be concepts since concepts can be used as compile-time boolean values. It might be worth renaming them for syntax "aesthetic" reason, because has_xxx sounds like a function call and I would expect () after it. So has_strides could become strided_expression for instance (I don't have a good name for has_data_interface); these sounds less like boolean functions only and are closer to the concept naming pattern in the STL.
Regarding metafunctions returnig types, like get_value_t, I am more hesitant. If we only use get_value_t, then get_value is an implementation detail, and I agree that having a constexpr function makes it more readable. I just find weird to write decltype(some_constexpr_function), but I'm definitely biased by years of pre C++17 metaprogramming here ;)
We could be more systematic with these get_xxx_t helpers to avoid a lot of typename type::xxx in the code base (for instance storage_type could have its dedicated getter that would make the code easier to read). Probably not all inner types should have such a getter, but those we access the most should.

Please do not take this as assertions about what we should do, this is more a global thinking to gather thoughts and open the discussion. Whatever we decide to do in the end, the goal should be to simplify and reduce the amount of code to maintain.

spectre-ns · 2026-06-13T19:22:21Z

@spectre-ns thanks for tackling this!

let me know if these types of changes are in the direction you'd like to see xtensor move towards!

Yes and "maybe", depending on the primitives we're talking about:

I think all the has_xxx metafunctions could actually just be concepts since concepts can be used as compile-time boolean values. It might be worth renaming them for syntax "aesthetic" reason, because has_xxx sounds like a function call and I would expect () after it. So has_strides could become strided_expression for instance (I don't have a good name for has_data_interface); these sounds less like boolean functions only and are closer to the concept naming pattern in the STL.

Regarding metafunctions returnig types, like get_value_t, I am more hesitant. If we only use get_value_t, then get_value is an implementation detail, and I agree that having a constexpr function makes it more readable. I just find weird to write decltype(some_constexpr_function), but I'm definitely biased by years of pre C++17 metaprogramming here ;)

We could be more systematic with these get_xxx_t helpers to avoid a lot of typename type::xxx in the code base (for instance storage_type could have its dedicated getter that would make the code easier to read). Probably not all inner types should have such a getter, but those we access the most should.

Please do not take this as assertions about what we should do, this is more a global thinking to gather thoughts and open the discussion. Whatever we decide to do in the end, the goal should be to simplify and reduce the amount of code to maintain.

@JohanMabille this all makes sense.

In general, I'm trying to improve compile times with these changes. Readability could be improved as well.

Re: Concepts - I agree. Concepts can be evaluated to a bool at both constexpr and runtime. So this make sense.

decltype(some_constexpr_function) was unintuitive to me too but I wanted to see how it integrated into the library and the ergonomics of the resulting interface.

I think in general, I will try to put up some changes to sweep and remove cases where the library uses recursion such as in the case of:

xtensor/include/xtensor/views/xview.hpp

Lines 155 to 160 in dd857e5

    
           struct is_strided_view 
        
               : std::integral_constant< 
        
                     bool, 
        
                     std::conjunction<has_data_interface<E>, is_strided_slice_impl<std::decay_t<S>>...>::value> 
        
           { 
        
           };

I expect replacing recursive template expansions with constexpr functions could improve compile times by replacing multiple function template definitions with one pack expansion. Additionally, some of the compiler error message might improve as well! :)

spectre-ns · 2026-06-14T00:09:36Z

@JohanMabille I have updated the PR to use constraint style names for concepts rather than predicate style as they were originally.

I think when adopting concepts we should expand the interface constraints. For instance, the current iterable_expression concept requires an expression have T::begin(). In reality, we require at least T::begin() and T::end(). This PR is a refactor so I may follow up with some tighter constraints later.

Also, there is a relationship between concepts and the CTRP interfaces. It would be nice to constrain the base classes.

Finally, std::enable_if should be eradicated from the code base as the compiler errors are extremely cryptic.

JohanMabille · 2026-06-26T09:12:30Z

    {
        // Added indirection for MSVC 2017 bug with the operator value_type()
-        using func_return_type = typename meta_identity<
+        using func_return_type = typename std::type_identity<


I wonder if we still need this indirection with latest MSVC. But we can check that in a dedicated PR.

JohanMabille · 2026-06-26T09:14:47Z

        template <class E, class FE = F>
        void assign_to(xexpression<E>& e) const noexcept
-            requires(has_assign_to_v<E, FE>);
+            requires(assignable_expression<E, FE>);


This concept should probably be named assignable_to_expression, since it checks the existence of the assign_to method. assignable_expression makes me think of checking the existence of operator=.

JohanMabille · 2026-06-26T09:19:27Z

+        std::declval<const E>() != std::declval<const E>();
+        ++(*std::declval<E*>());
+        (*std::declval<E*>())++;
    };


Could we use std::forward_iterator instead of defining our own iterator concept? I know it was already here before your refactoring, but that might be worth a try.

JohanMabille · 2026-06-26T09:20:07Z

-        void_t<
-            decltype(*std::declval<const E>(), std::declval<const E>() == std::declval<const E>(), std::declval<const E>() != std::declval<const E>(), ++(*std::declval<E*>()), (*std::declval<E*>())++, std::true_type())>>
-        : std::true_type
+    constexpr bool is_iterator()


This concept should be enough and used everywhere, what do you think?

JohanMabille · 2026-06-26T09:20:36Z


    template <class E1, class E2>
-    constexpr bool has_assign_to_v = has_assign_to<E1, E2>::value;
+    concept assignable_expression = requires { std::declval<const E2&>().assign_to(std::declval<E1&>()); };


As explained above, assignable_to_expression would sound less confusing to me.

JohanMabille · 2026-06-26T09:21:02Z


    template <typename T>
-    concept with_memory_address_concept = has_memory_address<std::decay_t<T>>::value;
+    concept with_memory_address_concept = addressable_expression<std::decay_t<T>>;


Should we remove this one and use addressable_expression everywhere?

JohanMabille · 2026-06-26T09:21:22Z

+    concept with_memory_address_concept = addressable_expression<std::decay_t<T>>;
    template <typename T>
-    concept without_memory_address_concept = !has_memory_address<std::decay_t<T>>::value;
+    concept without_memory_address_concept = !addressable_expression<std::decay_t<T>>;


Same remark here.

first stage of eliminating sfinae

d3daeec

spectre-ns added 3 commits June 7, 2026 12:35

Formatting

31f1f59

Elminated some sfinae in xviews

c66d4a4

formatting

7e6e3c3

spectre-ns changed the title ~~first stage of eliminating sfinae~~ Eliminating TMP from xutils.hpp and patches for dependencies Jun 7, 2026

spectre-ns changed the title ~~Eliminating TMP from xutils.hpp and patches for dependencies~~ Partial elimination of TMP from xutils.hpp and patches for dependencies Jun 7, 2026

spectre-ns marked this pull request as ready for review June 7, 2026 13:17

spectre-ns marked this pull request as draft June 7, 2026 13:19

Updated find strides

62d19f4

spectre-ns added 4 commits June 13, 2026 20:14

Attempting to use concepts rather than constexpr function wrappers

03afd44

Fixed assign to

7aed890

Fixing naming

88f611e

formatting

fb543b9

removed decltype wrappers for type declarations

2beb8c8

spectre-ns marked this pull request as ready for review June 14, 2026 00:34

JohanMabille reviewed Jun 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Partial elimination of TMP from xutils.hpp and patches for dependencies#2923

Partial elimination of TMP from xutils.hpp and patches for dependencies#2923
spectre-ns wants to merge 10 commits into
xtensor-stack:masterfrom
spectre-ns:sfinae-to-constexpr

spectre-ns commented Jun 7, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Jun 7, 2026 •

edited

Loading

Uh oh!

spectre-ns commented Jun 7, 2026

Uh oh!

JohanMabille commented Jun 8, 2026 •

edited

Loading

Uh oh!

spectre-ns commented Jun 13, 2026 •

edited

Loading

Uh oh!

spectre-ns commented Jun 14, 2026 •

edited

Loading

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

JohanMabille Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

spectre-ns commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Description

Uh oh!

codspeed-hq Bot commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

spectre-ns commented Jun 7, 2026

Uh oh!

JohanMabille commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spectre-ns commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spectre-ns commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

JohanMabille Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

spectre-ns commented Jun 7, 2026 •

edited

Loading

codspeed-hq Bot commented Jun 7, 2026 •

edited

Loading

JohanMabille commented Jun 8, 2026 •

edited

Loading

spectre-ns commented Jun 13, 2026 •

edited

Loading

spectre-ns commented Jun 14, 2026 •

edited

Loading