From a56d7efe70ea4e3aa81dcaf3b85e08ff9c78e151 Mon Sep 17 00:00:00 2001 From: Nikita Kniazev Date: Fri, 18 Feb 2022 01:35:51 +0300 Subject: [PATCH 1/2] X3: Do not rely on iterator rollback on fail Preparation to relax iterator rollback policy, which is also currently violated by not rolling back skipper almost everywhere, so this should fix postskip disabling. --- .../spirit/home/x3/directive/matches.hpp | 2 ++ .../boost/spirit/home/x3/directive/repeat.hpp | 5 ++- include/boost/spirit/home/x3/numeric/bool.hpp | 3 +- .../spirit/home/x3/operator/alternative.hpp | 6 ++-- .../home/x3/operator/detail/alternative.hpp | 6 ++-- .../boost/spirit/home/x3/operator/kleene.hpp | 5 +-- .../spirit/home/x3/operator/optional.hpp | 23 +++++++++++-- .../boost/spirit/home/x3/operator/plus.hpp | 5 +-- test/x3/alternative.cpp | 3 ++ test/x3/bool.cpp | 23 +++++++++++++ test/x3/kleene.cpp | 6 ++++ test/x3/list.cpp | 5 +++ test/x3/matches.cpp | 5 +++ test/x3/optional.cpp | 4 +++ test/x3/plus.cpp | 5 +++ test/x3/repeat.cpp | 11 +++++++ test/x3/test.hpp | 11 +++++++ test/x3/utils.hpp | 33 +++++++++++++++++++ 18 files changed, 148 insertions(+), 13 deletions(-) diff --git a/include/boost/spirit/home/x3/directive/matches.hpp b/include/boost/spirit/home/x3/directive/matches.hpp index 46428465ac..c12c55e664 100644 --- a/include/boost/spirit/home/x3/directive/matches.hpp +++ b/include/boost/spirit/home/x3/directive/matches.hpp @@ -28,8 +28,10 @@ namespace boost { namespace spirit { namespace x3 bool parse(Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, Attribute& attr) const { + Iterator save = first; bool const result = this->subject.parse( first, last, context, rcontext, unused); + if (!result) first = save; traits::move_to(result, attr); return true; } diff --git a/include/boost/spirit/home/x3/directive/repeat.hpp b/include/boost/spirit/home/x3/directive/repeat.hpp index 6deb2db303..8291bca660 100644 --- a/include/boost/spirit/home/x3/directive/repeat.hpp +++ b/include/boost/spirit/home/x3/directive/repeat.hpp @@ -76,12 +76,15 @@ namespace boost { namespace spirit { namespace x3 } first = local_iterator; + + Iterator iter = first; // parse some more up to the maximum specified for (/**/; !repeat_limit.got_max(i); ++i) { if (!detail::parse_into_container( - this->subject, first, last, context, rcontext, attr)) + this->subject, iter, last, context, rcontext, attr)) break; + first = iter; } return true; } diff --git a/include/boost/spirit/home/x3/numeric/bool.hpp b/include/boost/spirit/home/x3/numeric/bool.hpp index b3541c775d..6e3c50ace5 100644 --- a/include/boost/spirit/home/x3/numeric/bool.hpp +++ b/include/boost/spirit/home/x3/numeric/bool.hpp @@ -32,8 +32,9 @@ namespace boost { namespace spirit { namespace x3 , Context const& context, unused_type, T& attr) const { x3::skip_over(first, last, context); + Iterator save = first; return policies.parse_true(first, last, attr, get_case_compare(context)) - || policies.parse_false(first, last, attr, get_case_compare(context)); + || policies.parse_false(first = save, last, attr, get_case_compare(context)); } template diff --git a/include/boost/spirit/home/x3/operator/alternative.hpp b/include/boost/spirit/home/x3/operator/alternative.hpp index aeb6998ba4..0a42199a22 100644 --- a/include/boost/spirit/home/x3/operator/alternative.hpp +++ b/include/boost/spirit/home/x3/operator/alternative.hpp @@ -28,8 +28,9 @@ namespace boost { namespace spirit { namespace x3 Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, unused_type) const { + Iterator save = first; return this->left.parse(first, last, context, rcontext, unused) - || this->right.parse(first, last, context, rcontext, unused); + || this->right.parse(first = save, last, context, rcontext, unused); } template left, first, last, context, rcontext, attr) - || detail::parse_alternative(this->right, first, last, context, rcontext, attr); + || detail::parse_alternative(this->right, first = save, last, context, rcontext, attr); } }; diff --git a/include/boost/spirit/home/x3/operator/detail/alternative.hpp b/include/boost/spirit/home/x3/operator/detail/alternative.hpp index e6f6bfb827..43587cc192 100644 --- a/include/boost/spirit/home/x3/operator/detail/alternative.hpp +++ b/include/boost/spirit/home/x3/operator/detail/alternative.hpp @@ -221,8 +221,9 @@ namespace boost { namespace spirit { namespace x3 { namespace detail , Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, Attribute& attribute, mpl::false_) { + Iterator save = first; return detail::parse_into_container(parser.left, first, last, context, rcontext, attribute) - || detail::parse_into_container(parser.right, first, last, context, rcontext, attribute); + || detail::parse_into_container(parser.right, first = save, last, context, rcontext, attribute); } template @@ -231,8 +232,9 @@ namespace boost { namespace spirit { namespace x3 { namespace detail , Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, Attribute& attribute, mpl::true_) { + Iterator save = first; return detail::parse_into_container(alternative_helper{parser.left}, first, last, context, rcontext, attribute) - || detail::parse_into_container(alternative_helper{parser.right}, first, last, context, rcontext, attribute); + || detail::parse_into_container(alternative_helper{parser.right}, first = save, last, context, rcontext, attribute); } template diff --git a/include/boost/spirit/home/x3/operator/kleene.hpp b/include/boost/spirit/home/x3/operator/kleene.hpp index aa2977001d..2d186ea2c2 100644 --- a/include/boost/spirit/home/x3/operator/kleene.hpp +++ b/include/boost/spirit/home/x3/operator/kleene.hpp @@ -29,9 +29,10 @@ namespace boost { namespace spirit { namespace x3 bool parse(Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, Attribute& attr) const { + Iterator iter = first; while (detail::parse_into_container( - this->subject, first, last, context, rcontext, attr)) - ; + this->subject, iter, last, context, rcontext, attr)) + first = iter; return true; } }; diff --git a/include/boost/spirit/home/x3/operator/optional.hpp b/include/boost/spirit/home/x3/operator/optional.hpp index 257fd78cfb..56b4013caa 100644 --- a/include/boost/spirit/home/x3/operator/optional.hpp +++ b/include/boost/spirit/home/x3/operator/optional.hpp @@ -36,8 +36,10 @@ namespace boost { namespace spirit { namespace x3 , Context const& context, RContext& rcontext, Attribute& attr , traits::container_attribute) const { - detail::parse_into_container( - this->subject, first, last, context, rcontext, attr); + Iterator iter = first; + if (detail::parse_into_container( + this->subject, iter, last, context, rcontext, attr)) + first = iter; return true; } @@ -55,13 +57,28 @@ namespace boost { namespace spirit { namespace x3 // create a local value value_type val{}; - if (this->subject.parse(first, last, context, rcontext, val)) + Iterator iter = first; + if (this->subject.parse(iter, last, context, rcontext, val)) { + first = iter; // assign the parsed value into our attribute x3::traits::move_to(val, attr); } return true; } + + // Attribute is of other type + template + bool parse_subject(Iterator& first, Iterator const& last + , Context const& context, RContext& rcontext, Attribute& attr + , Category) const + { + Iterator iter = first; + if (this->subject.parse(iter, last, context, rcontext, attr)) + first = iter; + return true; + } }; template diff --git a/include/boost/spirit/home/x3/operator/plus.hpp b/include/boost/spirit/home/x3/operator/plus.hpp index 6227d6a6fa..573d287325 100644 --- a/include/boost/spirit/home/x3/operator/plus.hpp +++ b/include/boost/spirit/home/x3/operator/plus.hpp @@ -33,9 +33,10 @@ namespace boost { namespace spirit { namespace x3 this->subject, first, last, context, rcontext, attr)) return false; + Iterator iter = first; while (detail::parse_into_container( - this->subject, first, last, context, rcontext, attr)) - ; + this->subject, iter, last, context, rcontext, attr)) + first = iter; return true; } }; diff --git a/test/x3/alternative.cpp b/test/x3/alternative.cpp index 6a1a63388d..632928b5ff 100644 --- a/test/x3/alternative.cpp +++ b/test/x3/alternative.cpp @@ -15,6 +15,7 @@ #include #include #include "test.hpp" +#include "utils.hpp" struct di_ignore { @@ -72,6 +73,8 @@ main() BOOST_TEST((test("roll", lit("rock") | lit("roll")))); BOOST_TEST((test("rock", lit("rock") | int_))); BOOST_TEST((test("12345", lit("rock") | int_))); + + BOOST_TEST(test("F", sf | lit('F'))); } { diff --git a/test/x3/bool.cpp b/test/x3/bool.cpp index 3734f488ca..df6ae97a3e 100644 --- a/test/x3/bool.cpp +++ b/test/x3/bool.cpp @@ -6,6 +6,26 @@ file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) ==============================================================================*/ #include "bool.hpp" +#include "utils.hpp" + +struct sf_bool_policies : boost::spirit::x3::bool_policies<> +{ + template + static bool parse_true(Iterator& first, Iterator const& last, Attribute& attr_, CaseCompare const&) + { + bool r = parse(first, last, sf); + if (r) boost::spirit::x3::traits::move_to(true, attr_); + return r; + } + + template + static bool parse_false(Iterator& first, Iterator const& last, Attribute& attr_, CaseCompare const&) + { + bool r = parse(first, last, boost::spirit::x3::lit('F')); + if (r) boost::spirit::x3::traits::move_to(false, attr_); + return r; + } +}; int main() { @@ -19,6 +39,9 @@ int main() BOOST_TEST(test("true", bool_)); BOOST_TEST(test("false", bool_)); BOOST_TEST(!test("fasle", bool_)); + + constexpr boost::spirit::x3::bool_parser sf_bool{}; + BOOST_TEST(test("F", sf_bool)); } { diff --git a/test/x3/kleene.cpp b/test/x3/kleene.cpp index 7ceb8bbe86..ea608d050f 100644 --- a/test/x3/kleene.cpp +++ b/test/x3/kleene.cpp @@ -42,6 +42,8 @@ main() { using spirit_test::test; using spirit_test::test_attr; + using spirit_test::test_failure; + using spirit_test::test_partial; using boost::spirit::x3::char_; using boost::spirit::x3::alpha; using boost::spirit::x3::upper; @@ -58,6 +60,10 @@ main() BOOST_TEST(test("", *char_)); BOOST_TEST(test("aaaaaaaa", *alpha)); BOOST_TEST(!test("aaaaaaaa", *upper)); + + BOOST_TEST(test_partial("F", *sf, 0)); + BOOST_TEST(test_partial("sF", +sf, 1)); + BOOST_TEST(test_partial("ssF", +sf, 2)); } { diff --git a/test/x3/list.cpp b/test/x3/list.cpp index dc7121e908..ee8d748a68 100644 --- a/test/x3/list.cpp +++ b/test/x3/list.cpp @@ -25,6 +25,11 @@ main() BOOST_SPIRIT_ASSERT_CONSTEXPR_CTORS(char_ % ','); + { + BOOST_TEST(test_partial("s,F", sf % ',', 1)); + BOOST_TEST(test_partial("s,s,F", sf % ',', 3)); + } + { BOOST_TEST(test("a,b,c,d,e,f,g,h", char_ % ',')); BOOST_TEST(test("a,b,c,d,e,f,g,h,", char_ % ',', false)); diff --git a/test/x3/matches.cpp b/test/x3/matches.cpp index 08a8e5426c..9db2ea37d2 100644 --- a/test/x3/matches.cpp +++ b/test/x3/matches.cpp @@ -8,12 +8,15 @@ #include #include #include "test.hpp" +#include "utils.hpp" int main() { using spirit_test::test; using spirit_test::test_attr; + using spirit_test::test_failure; + using spirit_test::test_partial; using boost::spirit::x3::matches; using boost::spirit::x3::char_; @@ -23,6 +26,8 @@ main() BOOST_TEST(test("x", matches[char_])); bool result = false; BOOST_TEST(test_attr("x", matches[char_], result) && result); + + BOOST_TEST(test_partial("F", matches[sf], 0)); } { diff --git a/test/x3/optional.cpp b/test/x3/optional.cpp index 6224ad61d2..d9ccc0d806 100644 --- a/test/x3/optional.cpp +++ b/test/x3/optional.cpp @@ -45,6 +45,8 @@ main() using spirit_test::test; using spirit_test::test_attr; + using spirit_test::test_failure; + using spirit_test::test_partial; using boost::spirit::x3::int_; using boost::spirit::x3::omit; @@ -56,6 +58,8 @@ main() BOOST_TEST((test("1234", -int_))); BOOST_TEST((test("abcd", -int_, false))); + BOOST_TEST(test_partial("F", -sf, 0)); + boost::optional n; BOOST_TEST(test_attr("", -int_, n)) && BOOST_TEST(!n); diff --git a/test/x3/plus.cpp b/test/x3/plus.cpp index b92523b4d7..ed03d168ce 100644 --- a/test/x3/plus.cpp +++ b/test/x3/plus.cpp @@ -43,6 +43,8 @@ main() { using spirit_test::test; using spirit_test::test_attr; + using spirit_test::test_failure; + using spirit_test::test_partial; using boost::spirit::x3::char_; using boost::spirit::x3::alpha; using boost::spirit::x3::upper; @@ -63,6 +65,9 @@ main() BOOST_TEST(!test("", +char_)); BOOST_TEST(test("aaaaaaaa", +alpha)); BOOST_TEST(!test("aaaaaaaa", +upper)); + + BOOST_TEST(test_partial("sF", +sf, 1)); + BOOST_TEST(test_partial("ssF", +sf, 2)); } { diff --git a/test/x3/repeat.cpp b/test/x3/repeat.cpp index 4468c2e665..6dfe9cd5bf 100644 --- a/test/x3/repeat.cpp +++ b/test/x3/repeat.cpp @@ -24,6 +24,8 @@ main() { using spirit_test::test_attr; using spirit_test::test; + using spirit_test::test_failure; + using spirit_test::test_partial; using namespace boost::spirit::x3::ascii; using boost::spirit::x3::repeat; @@ -51,6 +53,15 @@ main() BOOST_TEST(test("aaaaa", repeat(3, inf)[char_])); BOOST_TEST(test("aaaaaa", repeat(3, inf)[char_])); BOOST_TEST(!test("aa", repeat(3, inf)[char_])); + + BOOST_TEST(test_partial("sF", repeat(1)[sf], 1)); + BOOST_TEST(test_partial("sF", repeat(1, inf)[sf], 1)); + BOOST_TEST(test_partial("ssF", repeat(2)[sf], 2)); + BOOST_TEST(test_partial("ssF", repeat(1, inf)[sf], 2)); + BOOST_TEST(test_partial("ssF", repeat(2, inf)[sf], 2)); + BOOST_TEST(test_partial("sssF", repeat(3)[sf], 3)); + BOOST_TEST(test_partial("sssF", repeat(1, inf)[sf], 3)); + BOOST_TEST(test_partial("sssF", repeat(2, inf)[sf], 3)); } { std::string s; diff --git a/test/x3/test.hpp b/test/x3/test.hpp index 60078492d1..bb27c392fd 100644 --- a/test/x3/test.hpp +++ b/test/x3/test.hpp @@ -57,6 +57,17 @@ namespace spirit_test return !boost::spirit::x3::parse(in, last, p) && (in == start); } + template + bool test_partial(Char const* in, Parser const& p, int parsed_chars) + { + Char const * const start = in; + Char const* last = in; + while (*last) + last++; + + return boost::spirit::x3::parse(in, last, p) && (in == start + parsed_chars); + } + template bool test_failure(boost::basic_string_view> const in, Parser const& p) diff --git a/test/x3/utils.hpp b/test/x3/utils.hpp index 588f60dec3..45675d69fc 100644 --- a/test/x3/utils.hpp +++ b/test/x3/utils.hpp @@ -9,6 +9,7 @@ #define BOOST_SPIRIT_TEST_X3_UTILS_HPP #include +#include struct move_only { @@ -45,4 +46,36 @@ synth_parser synth{}; synth_parser const synth_move_only{}; + +struct sf_parser : boost::spirit::x3::parser +{ + typedef boost::spirit::x3::unused_type attribute_type; + + static bool const has_attribute = false; + static bool const handles_container = false; + + template + bool parse(char const*& iter, char const* last, Context const&, + RuleContext&, Attribute&) const + { + if (iter == last) + std::abort(); + + switch (*iter) { + case 's': + ++iter; + return true; + case 'F': + ++iter; // fail without rollback + return false; + case 'f': + return false; + } + + std::abort(); + } +}; + +constexpr sf_parser const sf{}; + #endif From 161c601d85b35cdb3b6203498b8aad776c75f5b5 Mon Sep 17 00:00:00 2001 From: Nikita Kniazev Date: Fri, 18 Feb 2022 02:27:42 +0300 Subject: [PATCH 2/2] X3: Remove unneeded iterator rollback Under the current iterator rollback policy rollbacks could be omitted in compound parser which don't advance iterator themselves because the called subparser is responsible for rollback on failure. In reality these rollbacks are undoing skipper work what renders as inconsistency in error handling position (`p` vs `p >> eps` or `repeat(1)[p]`). --- include/boost/spirit/home/x3/directive/repeat.hpp | 5 +---- include/boost/spirit/home/x3/operator/detail/sequence.hpp | 6 ------ include/boost/spirit/home/x3/operator/sequence.hpp | 2 -- test/x3/error_handler.cpp | 2 +- 4 files changed, 2 insertions(+), 13 deletions(-) diff --git a/include/boost/spirit/home/x3/directive/repeat.hpp b/include/boost/spirit/home/x3/directive/repeat.hpp index 8291bca660..903748b60e 100644 --- a/include/boost/spirit/home/x3/directive/repeat.hpp +++ b/include/boost/spirit/home/x3/directive/repeat.hpp @@ -66,17 +66,14 @@ namespace boost { namespace spirit { namespace x3 Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, Attribute& attr) const { - Iterator local_iterator = first; typename RepeatCountLimit::type i{}; for (/**/; !repeat_limit.got_min(i); ++i) { if (!detail::parse_into_container( - this->subject, local_iterator, last, context, rcontext, attr)) + this->subject, first, last, context, rcontext, attr)) return false; } - first = local_iterator; - Iterator iter = first; // parse some more up to the maximum specified for (/**/; !repeat_limit.got_max(i); ++i) diff --git a/include/boost/spirit/home/x3/operator/detail/sequence.hpp b/include/boost/spirit/home/x3/operator/detail/sequence.hpp index f5322a733a..f7405247dc 100644 --- a/include/boost/spirit/home/x3/operator/detail/sequence.hpp +++ b/include/boost/spirit/home/x3/operator/detail/sequence.hpp @@ -248,11 +248,9 @@ namespace boost { namespace spirit { namespace x3 { namespace detail typename l_pass::type l_attr = l_pass::call(l_part); typename r_pass::type r_attr = r_pass::call(r_part); - Iterator save = first; if (parser.left.parse(first, last, context, rcontext, l_attr) && parser.right.parse(first, last, context, rcontext, r_attr)) return true; - first = save; return false; } @@ -289,11 +287,9 @@ namespace boost { namespace spirit { namespace x3 { namespace detail , Context const& context, RContext& rcontext, Attribute& attr , traits::container_attribute) { - Iterator save = first; if (parse_sequence_container(parser.left, first, last, context, rcontext, attr) && parse_sequence_container(parser.right, first, last, context, rcontext, attr)) return true; - first = save; return false; } @@ -312,11 +308,9 @@ namespace boost { namespace spirit { namespace x3 { namespace detail Parser const& parser , Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, Attribute& attr, mpl::true_ /*should_split*/) { - Iterator save = first; if (parser.left.parse( first, last, context, rcontext, attr) && parser.right.parse(first, last, context, rcontext, attr)) return true; - first = save; return false; } diff --git a/include/boost/spirit/home/x3/operator/sequence.hpp b/include/boost/spirit/home/x3/operator/sequence.hpp index 9e6e1702fc..8cd47ee8a9 100644 --- a/include/boost/spirit/home/x3/operator/sequence.hpp +++ b/include/boost/spirit/home/x3/operator/sequence.hpp @@ -29,11 +29,9 @@ namespace boost { namespace spirit { namespace x3 Iterator& first, Iterator const& last , Context const& context, RContext& rcontext, unused_type) const { - Iterator save = first; if (this->left.parse(first, last, context, rcontext, unused) && this->right.parse(first, last, context, rcontext, unused)) return true; - first = save; return false; } diff --git a/test/x3/error_handler.cpp b/test/x3/error_handler.cpp index 00d152efb6..23c1959f67 100644 --- a/test/x3/error_handler.cpp +++ b/test/x3/error_handler.cpp @@ -33,7 +33,7 @@ struct test_rule_class : x3::annotate_on_success, error_handler_base {}; x3::rule const test_inner_rule = "\"bar\""; x3::rule const test_rule; -auto const test_inner_rule_def = x3::lit("bar"); +auto const test_inner_rule_def = x3::lit("bar") > x3::eps; auto const test_rule_def = x3::lit("foo") > test_inner_rule > x3::lit("git"); BOOST_SPIRIT_DEFINE(test_inner_rule, test_rule)