Skip to content

Commit

Permalink
sequential_or: Fixed random order execution of underlying parsers
Browse files Browse the repository at this point in the history
After two hours of fighting with the optimizer, I happily drew attention to
this little nifty bitwise operator in `any_if_ns` function.

Explanation of the bug: bitwise inclusive OR operator is not a sequence point
(per chapter §5.13 of C++14 standard), that's why at compiling the expression
`a() | b() | ... | z()` optimizer is allowed to rearrange the execution order
of the functions `a`, `b` ... `z`.

There is high chance that a lot of people were misguided by the bug and chose
not to use `sequential_or`.

I vaguely remember how about three years ago I thought that I am dumb and/or
the documentation is wrong when I tried to use the `sequential_or` but ended
with some workaround.

There are three possible fixes:
  - Add and use strict order version of `any_ns` and `any_if_ns` functions
  - This one. Make the original `any_ns` and `any_if_ns` strict ordered
    (could potentially make permutations operator slower)
  - Break the `any_ns` and `any_if_ns` API and somehow generalize the code
  • Loading branch information
Kojoley committed Dec 5, 2017
1 parent 1265ee1 commit b1c74d0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
6 changes: 4 additions & 2 deletions include/boost/spirit/home/support/algorithm/any_if_ns.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,16 @@ namespace boost { namespace spirit
any_if_ns(First1 const& first1, First2 const& first2
, Last1 const& last1, Last2 const& last2, F& f, mpl::false_)
{
return (0 != (f(*first1, spirit::detail::attribute_value<Pred, First1, Last2>(first2)) |
bool head = f(*first1, spirit::detail::attribute_value<Pred, First1, Last2>(first2));
bool tail =
detail::any_if_ns<Pred>(
fusion::next(first1)
, attribute_next<Pred, First1, Last2>(first2)
, last1, last2
, f
, fusion::result_of::equal_to<
typename fusion::result_of::next<First1>::type, Last1>())));
typename fusion::result_of::next<First1>::type, Last1>());
return head || tail;
}
}

Expand Down
12 changes: 8 additions & 4 deletions include/boost/spirit/home/support/algorithm/any_ns.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,16 @@ namespace boost { namespace spirit
inline bool
any_ns(First1 const& first1, First2 const& first2, Last const& last, F& f, mpl::false_)
{
return (0 != (f(*first1, *first2) |
bool head = f(*first1, *first2);
bool tail =
detail::any_ns(
fusion::next(first1)
, fusion::next(first2)
, last
, f
, fusion::result_of::equal_to<
typename fusion::result_of::next<First1>::type, Last>())));
typename fusion::result_of::next<First1>::type, Last>());
return head || tail;
}

template <typename First, typename Last, typename F>
Expand All @@ -59,13 +61,15 @@ namespace boost { namespace spirit
inline bool
any_ns(First const& first, Last const& last, F& f, mpl::false_)
{
return (0 != (f(*first) |
bool head = f(*first);
bool tail =
detail::any_ns(
fusion::next(first)
, last
, f
, fusion::result_of::equal_to<
typename fusion::result_of::next<First>::type, Last>())));
typename fusion::result_of::next<First>::type, Last>());
return head || tail;
}
}

Expand Down

0 comments on commit b1c74d0

Please sign in to comment.