From 23686c5f315d428eecb3038f93ee005600efece2 Mon Sep 17 00:00:00 2001 From: Nana Sakisaka <1901813+saki7@users.noreply.github.com> Date: Sun, 8 Feb 2026 00:32:28 +0900 Subject: [PATCH] Materialize `recursive_wrapper` instead of `T` when possible --- .../x4/core/detail/parse_into_container.hpp | 19 +++++---- include/iris/x4/core/move_to.hpp | 12 +++++- include/iris/x4/traits/variant_traits.hpp | 14 +++---- modules/iris | 2 +- test/x4/CMakeLists.txt | 3 +- test/x4/recursive.cpp | 40 +++++++++++++++++++ 6 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 test/x4/recursive.cpp diff --git a/include/iris/x4/core/detail/parse_into_container.hpp b/include/iris/x4/core/detail/parse_into_container.hpp index 7a2c3b4d8..58ba564e8 100644 --- a/include/iris/x4/core/detail/parse_into_container.hpp +++ b/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 Se, class Context, X4Attribute Attr> - requires (!parser_accepts_container_v) + requires (!parser_accepts_container_v>) [[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, unused_container_type>); - using value_type = traits::container_value_t; + using value_type = traits::container_value_t>; value_type val; // default-initialize - static_assert(Parsable); + //static_assert(Parsable); 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 Se, class Context> requires (!parser_accepts_container_v) [[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) { - static_assert(Parsable); + //static_assert(Parsable); return parser.parse(first, last, ctx, unused); } // Parser has attribute (synthesize; Attribute is a container) template Se, class Context, X4Attribute Attr> - requires parser_accepts_container_v + requires parser_accepts_container_v> [[nodiscard]] static constexpr bool call_synthesize( Parser const& parser, It& first, Se const& last, Context const& ctx, Attr& attr - ) noexcept(is_nothrow_parsable_v) + ) noexcept(is_nothrow_parsable_v>) { - static_assert(Parsable); + //static_assert(Parsable>); return parser.parse(first, last, ctx, attr); } + // ------------------------------------------------------ + // Parser has attribute && it is NOT fusion sequence template Se, class Context, X4Attribute Attr> requires diff --git a/include/iris/x4/core/move_to.hpp b/include/iris/x4/core/move_to.hpp index ee30e797c..d355a7de5 100644 --- a/include/iris/x4/core/move_to.hpp +++ b/include/iris/x4/core/move_to.hpp @@ -46,6 +46,7 @@ template constexpr void move_to(T const&& src, T& dest) noexcept(std::is_nothrow_assignable_v) { + static_assert(std::is_assignable_v); dest = std::move(src); } @@ -53,6 +54,7 @@ template constexpr void move_to(T&& src, T& dest) noexcept(std::is_nothrow_assignable_v) { + static_assert(std::is_assignable_v); dest = std::forward(src); } @@ -60,6 +62,7 @@ template constexpr void move_to(T const& src, T& dest) noexcept(std::is_nothrow_copy_assignable_v) { + static_assert(std::is_assignable_v); dest = src; } @@ -120,6 +123,7 @@ move_to(Source&& src, Dest& dest) noexcept(noexcept(dest = std::forward_like(boost::fusion::front(std::forward(src))))) { static_assert(!std::same_as, 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(boost::fusion::front(std::forward(src))); } @@ -130,7 +134,7 @@ move_to(Source&& src, Dest& dest) noexcept(std::is_nothrow_assignable_v) { static_assert(!std::same_as, Dest>, "[BUG] This call should instead resolve to the overload handling identical types"); - static_assert(std::is_assignable_v); + static_assert(std::is_assignable_v); dest = std::forward(src); } @@ -148,6 +152,8 @@ move_to(Source&& src, Dest& dest) { static_assert(!std::same_as, 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) { 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 = std::forward(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(boost::fusion::front(std::forward(src))); } @@ -195,6 +203,7 @@ move_to(Source&& src, Dest& dest) noexcept(std::is_nothrow_assignable_v) { static_assert(!std::same_as, Dest>, "[BUG] This call should instead resolve to the overload handling identical types"); + static_assert(std::is_assignable_v); dest = std::forward(src); } @@ -204,6 +213,7 @@ move_to(Source&& src, Dest& dest) noexcept(std::is_nothrow_assignable_v) { static_assert(!std::same_as, Dest>, "[BUG] This call should instead resolve to the overload handling identical types"); + static_assert(std::is_assignable_v); dest = std::forward(src); } diff --git a/include/iris/x4/traits/variant_traits.hpp b/include/iris/x4/traits/variant_traits.hpp index 4c452e827..38615ee32 100644 --- a/include/iris/x4/traits/variant_traits.hpp +++ b/include/iris/x4/traits/variant_traits.hpp @@ -47,22 +47,18 @@ template struct variant_find_substitute_impl { using type = std::conditional_t< - is_substitute_v>, + is_substitute_v>, - // TODO // Given some type `T`, when both `T` and `recursive_wrapper` 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`, - // - Constructing `T` and then moving it to `recursive_wrapper` + // (i.e. exposed attribute type, possibly a rule attribute type) that + // is `recursive_wrapper`, and + // - constructing `T` and then moving it to `recursive_wrapper` // 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, // no need to unwrap due to the reason described above typename variant_find_substitute_impl::type >; diff --git a/modules/iris b/modules/iris index 4ad99df0d..602496847 160000 --- a/modules/iris +++ b/modules/iris @@ -1 +1 @@ -Subproject commit 4ad99df0db993d71e01de7dbcc06208b3485e7ff +Subproject commit 60249684723cc7ff573713fe71ac439045a6ddea diff --git a/test/x4/CMakeLists.txt b/test/x4/CMakeLists.txt index d2dec5bfe..d38ccd59b 100644 --- a/test/x4/CMakeLists.txt +++ b/test/x4/CMakeLists.txt @@ -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 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 diff --git a/test/x4/recursive.cpp b/test/x4/recursive.cpp new file mode 100644 index 000000000..49b05d123 --- /dev/null +++ b/test/x4/recursive.cpp @@ -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 +#include +#include +#include + +#include + +#include + +TEST_CASE("recursive") +{ + using x4::int_; + + { + iris::recursive_wrapper val; + REQUIRE(parse("1", int_, val)); + CHECK(*val == 1); + } + + { + iris::recursive_wrapper> ints; + REQUIRE(parse("0,1", int_ % ',', ints)); + CHECK(*ints == std::vector{0, 1}); + } + + { + iris::recursive_wrapper> ints; + REQUIRE(parse("0-1-2,3-4-5", (int_ % '-') % ',', ints)); + CHECK(*ints == std::vector{0, 1, 2, 3, 4, 5}); + } +}