Clarify behavior of iterator/attribute on failed parse

include/boost/spirit/x4/core/attribute.hpp

@@ -34,10 +34,10 @@ concept X4NonUnusedAttribute =
     !X4UnusedAttribute<T> &&
     std::is_object_v<T> && // implies not reference
     !std::is_base_of_v<detail::parser_base, std::remove_const_t<T>> &&
-    std::move_constructible<T>;
+    std::move_constructible<std::remove_const_t<T>>;
     // TODO: `fusion::iterator_range` does not satisfy these due to `fusion::vector`'s iterator being a reference type
-    //std::default_initializable<T> &&
-    //std::assignable_from<T&, T>;
+    //std::default_initializable<std::remove_const_t<T>> &&
+    //std::assignable_from<std::remove_const_t<T>&, std::remove_const_t<T>>;
 
 template<class T>
 concept X4Attribute = X4UnusedAttribute<T> || X4NonUnusedAttribute<T>;

include/boost/spirit/x4/core/container_appender.hpp

@@ -0,0 +1,167 @@
+#ifndef BOOST_SPIRIT_X4_CORE_CONTAINER_APPENDER_HPP
+#define BOOST_SPIRIT_X4_CORE_CONTAINER_APPENDER_HPP
+
+#include <boost/spirit/core/type_traits.hpp>
+#include <boost/spirit/x4/traits/container_traits.hpp>
+#include <boost/spirit/x4/traits/transform_attribute.hpp>
+
+#include <iterator>
+#include <type_traits>
+#include <utility>
+
+namespace boost::spirit::x4 {
+
+namespace detail {
+
+template<class ContainerAttr>
+struct container_appender_base {};
+
+template<class ContainerAttr>
+    requires traits::is_associative_v<ContainerAttr>
+struct container_appender_base<ContainerAttr>
+{
+    using key_type = typename ContainerAttr::key_type;
+};
+
+} // detail
+
+template<class ContainerAttr>
+struct container_appender : detail::container_appender_base<ContainerAttr>
+{
+    static_assert(!is_ttp_specialization_of_v<std::remove_cvref_t<ContainerAttr>, container_appender>);
+    static_assert(traits::X4Container<ContainerAttr>);
+
+    using container_type = ContainerAttr;
+    using value_type = typename ContainerAttr::value_type;
+
+    ContainerAttr& container;  // NOLINT(cppcoreguidelines-avoid-const-or-ref-data-members)
+
+    constexpr explicit container_appender(ContainerAttr& container) noexcept
+        : container(container)
+    {}
+
+    constexpr auto begin() const
+        noexcept(noexcept(traits::begin(this->container)))
+    {
+        return traits::begin(this->container);
+    }
+
+    constexpr auto begin()
+        noexcept(noexcept(traits::begin(this->container)))
+    {
+        return traits::begin(this->container);
+    }
+
+    constexpr auto end() const
+        noexcept(noexcept(traits::end(this->container)))
+    {
+        return traits::end(this->container);
+    }
+
+    constexpr auto end()
+        noexcept(noexcept(traits::end(this->container)))
+    {
+        return traits::end(this->container);
+    }
+
+    [[nodiscard]] constexpr bool empty() const
+        noexcept(noexcept(traits::is_empty(this->container)))
+    {
+        return traits::is_empty(this->container);
+    }
+
+    constexpr void clear()
+        noexcept(noexcept(traits::clear(this->container)))
+    {
+        traits::clear(this->container);
+    }
+
+    template<class T>
+        requires requires(ContainerAttr& container) {
+            traits::push_back(container, std::declval<T>());
+        }
+    constexpr void push_back(T&& value)
+        noexcept(noexcept(traits::push_back(this->container, std::forward<T>(value))))
+    {
+        traits::push_back(this->container, std::forward<T>(value));
+    }
+};
+
+template<class T>
+struct unwrap_container_appender
+{
+    static_assert(!std::is_reference_v<T>);
+    static_assert(!std::is_const_v<T>);
+    using type = T;
+};
+
+template<class T>
+using unwrap_container_appender_t = typename unwrap_container_appender<T>::type;
+
+template<class ContainerAttr>
+struct unwrap_container_appender<container_appender<ContainerAttr>>
+{
+    using type = ContainerAttr;
+};
+
+
+template<class Attr>
+[[nodiscard]] constexpr Attr&
+make_container_appender(Attr& other_attr) noexcept
+{
+    return other_attr;  // NOLINT(bugprone-return-const-ref-from-parameter)
+}
+
+template<class Attr>
+constexpr void
+make_container_appender(Attr const&&) = delete; // dangling
+
+template<traits::X4Container ContainerAttr>
+[[nodiscard]] constexpr container_appender<ContainerAttr>
+make_container_appender(ContainerAttr& container_attr) noexcept
+{
+    return container_appender<ContainerAttr>{container_attr};
+}
+
+template<traits::X4Container ContainerAttr>
+    requires is_ttp_specialization_of_v<std::remove_cvref_t<ContainerAttr>, container_appender>
+[[nodiscard]] constexpr ContainerAttr&
+make_container_appender(ContainerAttr& appender) noexcept
+{
+    return appender;
+}
+
+namespace traits {
+
+template<class ContainerAttr>
+struct append_container<container_appender<ContainerAttr>>
+{
+    template<std::forward_iterator It, std::sentinel_for<It> Se>
+    static constexpr void call(container_appender<ContainerAttr>& appender, It first, Se last)
+        noexcept(noexcept(traits::append(appender.container, std::move(first), std::move(last))))
+    {
+        traits::append(appender.container, std::move(first), std::move(last));
+    }
+};
+
+template<class Transformed>
+struct transform_attribute<Transformed, container_appender<Transformed>>
+{
+    using type = Transformed&;
+
+    [[nodiscard]] static constexpr Transformed& pre(container_appender<Transformed>& appender)
+    {
+        return appender.container;
+    }
+
+    template<class TransformedT>
+    static constexpr void post(container_appender<Transformed>&, TransformedT&&) noexcept
+    {
+    }
+};
+
+} // traits
+
+} // boost::spirit::x4
+
+#endif

include/boost/spirit/x4/core/detail/parse_into_container.hpp

@@ -12,6 +12,7 @@
 #include <boost/spirit/config.hpp>
 #include <boost/spirit/x4/core/move_to.hpp>
 #include <boost/spirit/x4/core/parser.hpp>
+#include <boost/spirit/x4/core/container_appender.hpp>
 
 #include <boost/spirit/x4/traits/container_traits.hpp>
 #include <boost/spirit/x4/traits/attribute.hpp>
@@ -195,22 +196,10 @@ struct parse_into_container_impl<Parser, Context>
     {
         static_assert(!std::same_as<std::remove_const_t<Attr>, unused_type>);
         static_assert(!std::same_as<std::remove_const_t<Attr>, unused_container_type>);
-
         static_assert(Parsable<Parser, It, Se, Context, Attr>);
 
-        if (traits::is_empty(attr)) {
-            return parser.parse(first, last, ctx, attr);
-        }
-
-        Attr rest; // default-initialize
-        if (!parser.parse(first, last, ctx, rest)) return false;
-
-        traits::append(
-            attr,
-            std::make_move_iterator(traits::begin(rest)),
-            std::make_move_iterator(traits::end(rest))
-        );
-        return true;
+        auto&& appender = x4::make_container_appender(attr);
+        return parser.parse(first, last, ctx, appender);
     }
 };
 

include/boost/spirit/x4/core/detail/parse_sequence.hpp

@@ -41,7 +41,7 @@ namespace boost::spirit::x4::detail {
 template<class Parser, class Context>
 struct sequence_size
 {
-    static constexpr int value = traits::has_attribute_v<Parser, Context>;
+    static constexpr std::size_t value = traits::has_attribute_v<Parser, Context>;
 };
 
 template<class Parser, class Context>
@@ -53,11 +53,15 @@ struct sequence_size<Parser, Context>
 template<class L, class R, class Context>
 struct sequence_size<sequence<L, R>, Context>
 {
-    static constexpr int value =
+    static constexpr std::size_t value =
         sequence_size<L, Context>::value +
         sequence_size<R, Context>::value;
 };
 
+template<class Parser, class Context>
+constexpr bool is_sequence_size_more_than_1 = sequence_size<Parser, Context>::value > 1;
+
+
 struct pass_sequence_attribute_unused
 {
     using type = unused_type;
@@ -127,11 +131,11 @@ struct partition_attribute
         "The parser expects tuple-like attribute type"
     );
 
-    static constexpr int l_size = sequence_size<L, Context>::value;
-    static constexpr int r_size = sequence_size<R, Context>::value;
+    static constexpr std::size_t l_size = sequence_size<L, Context>::value;
+    static constexpr std::size_t r_size = sequence_size<R, Context>::value;
 
-    static constexpr int actual_size = fusion::result_of::size<Attr>::value;
-    static constexpr int expected_size = l_size + r_size;
+    static constexpr std::size_t actual_size = static_cast<std::size_t>(fusion::result_of::size<Attr>::value);
+    static constexpr std::size_t expected_size = l_size + r_size;
 
     // If you got an error here, then you are trying to pass
     // a fusion sequence with the wrong number of elements
@@ -243,8 +247,10 @@ parse_sequence(
     It& first, Se const& last,
     Context const& ctx,
     Attr& attr
-)
+) // TODO: noexcept
 {
+    static_assert(X4Attribute<Attr>);
+
     using partition = partition_attribute<
         typename Parser::left_type,
         typename Parser::right_type,
@@ -258,29 +264,29 @@ parse_sequence(
     auto&& l_attr = l_pass::call(l_part);
     auto&& r_attr = r_pass::call(r_part);
 
-    It const first_saved = first;
+    auto&& l_attr_appender = x4::make_container_appender(l_attr);
+    auto&& r_attr_appender = x4::make_container_appender(r_attr);
 
-    if (parser.left.parse(first, last, ctx, l_attr) &&
-        parser.right.parse(first, last, ctx, r_attr)
+    It local_it = first;
+    if (parser.left.parse(local_it, last, ctx, l_attr_appender) &&
+        parser.right.parse(local_it, last, ctx, r_attr_appender)
     ) {
+        first = std::move(local_it);
         return true;
     }
-    first = first_saved;
+
     return false;
 }
 
-template<class Parser, class Context>
-constexpr bool pass_sequence_container_attribute = sequence_size<Parser, Context>::value > 1;
-
 template<
     class Parser,
     std::forward_iterator It, std::sentinel_for<It> Se,
     class Context,
     X4Attribute Attr
 >
-    requires pass_sequence_container_attribute<Parser, Context>
+    requires is_sequence_size_more_than_1<Parser, Context>
 [[nodiscard]] constexpr bool
-parse_sequence_container(
+parse_sequence_impl(
     Parser const& parser,
     It& first, Se const& last,
     Context const& ctx,
@@ -292,30 +298,21 @@ parse_sequence_container(
     return parser.parse(first, last, ctx, attr);
 }
 
-template<
-    class Parser,
-    std::forward_iterator It, std::sentinel_for<It> Se,
-    class Context,
-    class Attr // unconstrained
->
-    requires (!pass_sequence_container_attribute<Parser, Context>)
+template<class Parser, std::forward_iterator It, std::sentinel_for<It> Se, class Context>
+    requires (!is_sequence_size_more_than_1<Parser, Context>)
 [[nodiscard]] constexpr bool
-parse_sequence_container(
+parse_sequence_impl(
     Parser const& parser,
     It& first, Se const& last,
     Context const& ctx,
-    Attr& attr
+    X4Attribute auto& attr
 )
     noexcept(noexcept(detail::parse_into_container(parser, first, last, ctx, attr)))
 {
     return detail::parse_into_container(parser, first, last, ctx, attr);
 }
 
-template<
-    class Parser,
-    std::forward_iterator It, std::sentinel_for<It> Se,
-    class Context
->
+template<class Parser, std::forward_iterator It, std::sentinel_for<It> Se, class Context>
 [[nodiscard]] constexpr bool
 parse_sequence(
     Parser const& parser,
@@ -325,17 +322,17 @@ parse_sequence(
 )
     noexcept(
         std::is_nothrow_copy_assignable_v<It> &&
-        noexcept(detail::parse_sequence_container(parser.left, first, last, ctx, attr)) &&
-        noexcept(detail::parse_sequence_container(parser.right, first, last, ctx, attr))
+        noexcept(detail::parse_sequence_impl(parser.left, first, last, ctx, attr)) &&
+        noexcept(detail::parse_sequence_impl(parser.right, first, last, ctx, attr))
     )
 {
-    It const first_saved = first;
-    if (detail::parse_sequence_container(parser.left, first, last, ctx, attr) &&
-        detail::parse_sequence_container(parser.right, first, last, ctx, attr)
+    It local_it = first;
+    if (detail::parse_sequence_impl(parser.left, local_it, last, ctx, attr) &&
+        detail::parse_sequence_impl(parser.right, local_it, last, ctx, attr)
     ) {
+        first = std::move(local_it);
         return true;
     }
-    first = first_saved;
     return false;
 }
 
@@ -379,22 +376,14 @@ struct parse_into_container_impl<sequence<Left, Right>, Context>
         );
 
         if constexpr (
-            std::is_same_v<std::remove_const_t<Attr>, unused_type> ||
-            std::is_same_v<std::remove_const_t<Attr>, unused_container_type>
+            std::same_as<std::remove_const_t<Attr>, unused_type> ||
+            std::same_as<std::remove_const_t<Attr>, unused_container_type>
         ) {
             return detail::parse_sequence(parser, first, last, ctx, x4::assume_container(attr));
 
         } else {
-            Attr attr_;
-            if (!detail::parse_sequence(parser, first, last, ctx, attr_)) {
-                return false;
-            }
-            traits::append(
-                attr,
-                std::make_move_iterator(traits::begin(attr_)),
-                std::make_move_iterator(traits::end(attr_))
-            );
-            return true;
+            auto&& appender = x4::make_container_appender(x4::assume_container(attr));
+            return detail::parse_sequence(parser, first, last, ctx, appender);
         }
     }
 };