Materialize recursive_wrapper<T> instead of T when possible

Author: saki7

include/iris/x4/core/detail/parse_into_container.hpp

@@ -41,7 +41,7 @@ struct parse_into_container_base_impl
 {
     // Parser has attribute (synthesize; Attribute is a container)
     template<std::forward_iterator It, std::sentinel_for<It> Se, class Context, X4Attribute Attr>
-        requires (!parser_accepts_container_v<Parser, Attr>)
+        requires (!parser_accepts_container_v<Parser, unwrap_recursive_type<Attr>>)
     [[nodiscard]] static constexpr bool
     call_synthesize(
         Parser const& parser, It& first, Se const& last,
@@ -50,17 +50,18 @@ struct parse_into_container_base_impl
     {
         static_assert(!std::same_as<std::remove_const_t<Attr>, unused_container_type>);
 
-        using value_type = traits::container_value_t<Attr>;
+        using value_type = traits::container_value_t<unwrap_recursive_type<Attr>>;
         value_type val; // default-initialize
 
-        static_assert(Parsable<Parser, It, Se, Context, value_type>);
+        //static_assert(Parsable<Parser, It, Se, Context, value_type>);
         if (!parser.parse(first, last, ctx, val)) return false;
 
         // push the parsed value into our attribute
-        traits::push_back(attr, std::move(val));
+        traits::push_back(unwrap_recursive(attr), std::move(val));
         return true;
     }
 
+    // unused_container_type
     template<std::forward_iterator It, std::sentinel_for<It> Se, class Context>
         requires (!parser_accepts_container_v<Parser, unused_container_type>)
     [[nodiscard]] static constexpr bool
@@ -69,23 +70,25 @@ struct parse_into_container_base_impl
         Context const& ctx, unused_container_type const&
     ) noexcept(is_nothrow_parsable_v<Parser, It, Se, Context, unused_type>)
     {
-        static_assert(Parsable<Parser, It, Se, Context, unused_type>);
+        //static_assert(Parsable<Parser, It, Se, Context, unused_type>);
         return parser.parse(first, last, ctx, unused);
     }
 
     // Parser has attribute (synthesize; Attribute is a container)
     template<std::forward_iterator It, std::sentinel_for<It> Se, class Context, X4Attribute Attr>
-        requires parser_accepts_container_v<Parser, Attr>
+        requires parser_accepts_container_v<Parser, unwrap_recursive_type<Attr>>
     [[nodiscard]] static constexpr bool
     call_synthesize(
         Parser const& parser, It& first, Se const& last,
         Context const& ctx, Attr& attr
-    ) noexcept(is_nothrow_parsable_v<Parser, It, Se, Context, Attr>)
+    ) noexcept(is_nothrow_parsable_v<Parser, It, Se, Context, unwrap_recursive_type<Attr>>)
     {
-        static_assert(Parsable<Parser, It, Se, Context, Attr>);
+        //static_assert(Parsable<Parser, It, Se, Context, unwrap_recursive_type<Attr>>);
         return parser.parse(first, last, ctx, attr);
     }
 
+    // ------------------------------------------------------
+
     // Parser has attribute && it is NOT fusion sequence
     template<std::forward_iterator It, std::sentinel_for<It> Se, class Context, X4Attribute Attr>
         requires

include/iris/x4/core/move_to.hpp

@@ -46,20 +46,23 @@ template<traits::NonUnusedAttr T>
 constexpr void move_to(T const&& src, T& dest)
     noexcept(std::is_nothrow_assignable_v<T&, T const&&>)
 {
+    static_assert(std::is_assignable_v<T&, T const>);
     dest = std::move(src);
 }
 
 template<traits::NonUnusedAttr T>
 constexpr void move_to(T&& src, T& dest)
     noexcept(std::is_nothrow_assignable_v<T&, T&&>)
 {
+    static_assert(std::is_assignable_v<T&, T>);
     dest = std::forward<T>(src);
 }
 
 template<traits::NonUnusedAttr T>
 constexpr void move_to(T const& src, T& dest)
     noexcept(std::is_nothrow_copy_assignable_v<T>)
 {
+    static_assert(std::is_assignable_v<T&, T const&>);
     dest = src;
 }
 
@@ -120,6 +123,7 @@ move_to(Source&& src, Dest& dest)
     noexcept(noexcept(dest = std::forward_like<Source>(boost::fusion::front(std::forward<Source>(src)))))
 {
     static_assert(!std::same_as<std::remove_cvref_t<Source>, Dest>, "[BUG] This call should instead resolve to the overload handling identical types");
+    // TODO: preliminarily invoke static_assert to check if the assignment is valid
     dest = std::forward_like<Source>(boost::fusion::front(std::forward<Source>(src)));
 }
 
@@ -130,7 +134,7 @@ move_to(Source&& src, Dest& dest)
     noexcept(std::is_nothrow_assignable_v<Dest&, Source&&>)
 {
     static_assert(!std::same_as<std::remove_cvref_t<Source>, Dest>, "[BUG] This call should instead resolve to the overload handling identical types");
-    static_assert(std::is_assignable_v<Dest&, Source&&>);
+    static_assert(std::is_assignable_v<Dest&, Source>);
     dest = std::forward<Source>(src);
 }
 
@@ -148,6 +152,8 @@ move_to(Source&& src, Dest& dest)
 {
     static_assert(!std::same_as<std::remove_cvref_t<Source>, Dest>, "[BUG] This call should instead resolve to the overload handling identical types");
 
+    // TODO: preliminarily invoke static_assert to check if the assignment is valid
+
     if constexpr (std::is_rvalue_reference_v<Source&&>) {
         boost::fusion::move(std::move(src), dest);
     } else {
@@ -165,6 +171,7 @@ move_to(Source&& src, Dest& dest)
 
     // dest is a variant, src is a single element fusion sequence that the variant
     // *can* directly hold.
+    static_assert(std::is_assignable_v<Dest&, Source>);
     dest = std::forward<Source>(src);
 }
 
@@ -185,6 +192,7 @@ move_to(Source&& src, Dest& dest)
         "Error! The destination variant (Dest) cannot hold the source type (Source)"
     );
 
+    // TODO: preliminarily invoke static_assert to check if the assignment is valid
     dest = std::forward_like<Source>(boost::fusion::front(std::forward<Source>(src)));
 }
 
@@ -195,6 +203,7 @@ move_to(Source&& src, Dest& dest)
     noexcept(std::is_nothrow_assignable_v<Dest&, Source&&>)
 {
     static_assert(!std::same_as<std::remove_cvref_t<Source>, Dest>, "[BUG] This call should instead resolve to the overload handling identical types");
+    static_assert(std::is_assignable_v<Dest&, Source>);
     dest = std::forward<Source>(src);
 }
 
@@ -204,6 +213,7 @@ move_to(Source&& src, Dest& dest)
     noexcept(std::is_nothrow_assignable_v<Dest&, Source&&>)
 {
     static_assert(!std::same_as<std::remove_cvref_t<Source>, Dest>, "[BUG] This call should instead resolve to the overload handling identical types");
+    static_assert(std::is_assignable_v<Dest&, Source>);
     dest = std::forward<Source>(src);
 }
 

include/iris/x4/traits/variant_traits.hpp

@@ -47,22 +47,18 @@ template<class Attr, class First, class... Rest>
 struct variant_find_substitute_impl<Attr, First, Rest...>
 {
     using type = std::conditional_t<
-        is_substitute_v<Attr, iris::unwrap_recursive_t<First>>,
+        is_substitute_v<Attr, iris::unwrap_recursive_type<First>>,
 
-        // TODO
         // Given some type `T`, when both `T` and `recursive_wrapper<T>` is seen
         // during attribute resolution, X4 should ideally materialize the latter
         // because:
         //   - It means that the user has supplied at least one explicit type
-        //     (possibly a rule attribute type) that is `recursive_wrapper<T>`,
-        //   - Constructing `T` and then moving it to `recursive_wrapper<T>`
+        //     (i.e. exposed attribute type, possibly a rule attribute type) that
+        //     is `recursive_wrapper<T>`, and
+        //   - constructing `T` and then moving it to `recursive_wrapper<T>`
         //     involves copying from stack to heap.
         //
-        // This is to-do because the above optimization is currently not
-        // implementable in a straightforward way. We need to add
-        // `unwrap_recursive(attr)` to every places where any parser attempts
-        // to modify the content.
-        iris::unwrap_recursive_t<First>,
+        First, // no need to unwrap due to the reason described above
 
         typename variant_find_substitute_impl<Attr, Rest...>::type
     >;

modules/iris

@@ -12,6 +12,7 @@ function(x4_define_test test_name)
     iris_define_test(x4_${test_name} ${ARGN})
     iris_define_test_headers(x4_${test_name} iris_x4_test.hpp)
     target_link_libraries(x4_${test_name}_test PRIVATE Iris::X4)
+    set_target_properties(x4_${test_name}_test PROPERTIES FOLDER "test/x4")
 
     if(MSVC)
         # Prevent "Warning: Conflicting <Type> entries detected" error
@@ -28,7 +29,6 @@ endfunction()
 function(x4_define_tests)
     foreach(test_name IN LISTS ARGV)
         x4_define_test(${test_name} ${test_name}.cpp)
-        set_target_properties(x4_${test_name}_test PROPERTIES FOLDER "test/x4")
     endforeach()
 endfunction()
 
@@ -71,6 +71,7 @@ x4_define_tests(
     real1
     real2
     real3
+    recursive
     repeat
     rule1
     rule2

test/x4/CMakeLists.txt

@@ -0,0 +1,40 @@
+/*=============================================================================
+    Copyright (c) 2026 The Iris Project Contributors
+
+    Distributed under the Boost Software License, Version 1.0. (See accompanying
+    file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
+=============================================================================*/
+
+#include "iris_x4_test.hpp"
+
+#include <iris/x4/char/char.hpp>
+#include <iris/x4/string/string.hpp>
+#include <iris/x4/numeric/int.hpp>
+#include <iris/x4/operator/list.hpp>
+
+#include <iris/rvariant/recursive_wrapper.hpp>
+
+#include <vector>
+
+TEST_CASE("recursive")
+{
+    using x4::int_;
+
+    {
+        iris::recursive_wrapper<int> val;
+        REQUIRE(parse("1", int_, val));
+        CHECK(*val == 1);
+    }
+
+    {
+        iris::recursive_wrapper<std::vector<int>> ints;
+        REQUIRE(parse("0,1", int_ % ',', ints));
+        CHECK(*ints == std::vector{0, 1});
+    }
+
+    {
+        iris::recursive_wrapper<std::vector<int>> ints;
+        REQUIRE(parse("0-1-2,3-4-5", (int_ % '-') % ',', ints));
+        CHECK(*ints == std::vector{0, 1, 2, 3, 4, 5});
+    }
+}