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:
  - 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
Kojoley committed Dec 4, 2017
1 parent 1265ee1 commit 6a37fde
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 4 deletions.
9 changes: 5 additions & 4 deletions include/boost/spirit/home/qi/operator/sequential_or.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include <boost/spirit/home/qi/detail/pass_function.hpp>
#include <boost/spirit/home/qi/detail/attributes.hpp>
#include <boost/spirit/home/support/detail/what_function.hpp>
#include <boost/spirit/home/support/algorithm/any_if_ns.hpp>
#include <boost/spirit/home/support/algorithm/any_if_ns_so.hpp>
#include <boost/spirit/home/support/handles_container.hpp>
#include <boost/fusion/include/as_vector.hpp>
#include <boost/fusion/include/for_each.hpp>
Expand Down Expand Up @@ -75,9 +75,10 @@ namespace boost { namespace spirit { namespace qi
typename traits::wrap_if_not_tuple<Attribute>::type attr_local(attr_);

// return true if *any* of the parsers succeed
// (we use the non-short-circuiting version: any_if_ns
// to force all elements to be tested)
return spirit::any_if_ns(elements, attr_local, f, predicate());
// (we use the non-short-circuiting and strict order version:
// any_if_ns_so to force all the elements to be tested and
// in the defined order: first is first, last is last)
return spirit::any_if_ns_so(elements, attr_local, f, predicate());
}

template <typename Context>
Expand Down
92 changes: 92 additions & 0 deletions include/boost/spirit/home/support/algorithm/any_if_ns_so.hpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*=============================================================================
Copyright (c) 2001-2011 Hartmut Kaiser
Copyright (c) 2001-2011 Joel de Guzman
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)
==============================================================================*/
#if !defined(BOOST_SPIRIT_ANY_IF_NS_SO_DECEMBER_03_2017_0826PM)
#define BOOST_SPIRIT_ANY_IF_NS_SO_DECEMBER_03_2017_0826PM

#if defined(_MSC_VER)
#pragma once
#endif

#include <boost/spirit/home/support/algorithm/any_ns_so.hpp>

namespace boost { namespace spirit
{
///////////////////////////////////////////////////////////////////////////
// This is a special version for a binary fusion::any. The predicate
// is used to decide whether to advance the second iterator or not.
// This is needed for sequences containing components with unused
// attributes. The second iterator is advanced only if the attribute
// of the corresponding component iterator is not unused.
//
// This is a non-short circuiting (ns) strict order (so) version of the
// any_if algorithm.
///////////////////////////////////////////////////////////////////////////
namespace detail
{
template <
typename Pred, typename First1, typename Last1, typename First2
, typename Last2, typename F
>
inline bool
any_if_ns_so(First1 const&, First2 const&, Last1 const&, Last2 const&
, F const&, mpl::true_)
{
return false;
}

template <
typename Pred, typename First1, typename Last1, typename First2
, typename Last2, typename F
>
inline bool
any_if_ns_so(First1 const& first1, First2 const& first2
, Last1 const& last1, Last2 const& last2, F& f, mpl::false_)
{
bool head = f(*first1, spirit::detail::attribute_value<Pred, First1, Last2>(first2));
bool tail =
detail::any_if_ns_so<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>());
return head || tail;
}
}

template <typename Pred, typename Sequence1, typename Sequence2, typename F>
inline bool
any_if_ns_so(Sequence1 const& seq1, Sequence2& seq2, F f, Pred)
{
return detail::any_if_ns_so<Pred>(
fusion::begin(seq1), fusion::begin(seq2)
, fusion::end(seq1), fusion::end(seq2)
, f
, fusion::result_of::equal_to<
typename fusion::result_of::begin<Sequence1>::type
, typename fusion::result_of::end<Sequence1>::type>());
}

template <typename Pred, typename Sequence, typename F>
inline bool
any_if_ns_so(Sequence const& seq, unused_type const, F f, Pred)
{
return detail::any_ns_so(
fusion::begin(seq)
, fusion::end(seq)
, f
, fusion::result_of::equal_to<
typename fusion::result_of::begin<Sequence>::type
, typename fusion::result_of::end<Sequence>::type>());
}

}}

#endif

106 changes: 106 additions & 0 deletions include/boost/spirit/home/support/algorithm/any_ns_so.hpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*=============================================================================
Copyright (c) 2001-2011 Joel de Guzman
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)
==============================================================================*/
#if !defined(BOOST_SPIRIT_ANY_NS_SO_DECEMBER_03_2017_0826PM)
#define BOOST_SPIRIT_ANY_NS_SO_DECEMBER_03_2017_0826PM

#if defined(_MSC_VER)
#pragma once
#endif

#include <boost/mpl/bool.hpp>
#include <boost/fusion/include/equal_to.hpp>
#include <boost/fusion/include/next.hpp>
#include <boost/fusion/include/deref.hpp>
#include <boost/fusion/include/begin.hpp>
#include <boost/fusion/include/end.hpp>
#include <boost/fusion/include/any.hpp>
#include <boost/spirit/home/support/unused.hpp>

namespace boost { namespace spirit
{
// A non-short circuiting (ns) strict order (so) version of the any
// algorithm

namespace detail
{
template <typename First1, typename Last, typename First2, typename F>
inline bool
any_ns_so(First1 const&, First2 const&, Last const&, F const&, mpl::true_)
{
return false;
}

template <typename First1, typename Last, typename First2, typename F>
inline bool
any_ns_so(First1 const& first1, First2 const& first2, Last const& last, F& f, mpl::false_)
{
bool head = f(*first1, *first2);
bool tail =
detail::any_ns_so(
fusion::next(first1)
, fusion::next(first2)
, last
, f
, fusion::result_of::equal_to<
typename fusion::result_of::next<First1>::type, Last>());
return head || tail;
}

template <typename First, typename Last, typename F>
inline bool
any_ns_so(First const&, Last const&, F const&, mpl::true_)
{
return false;
}

template <typename First, typename Last, typename F>
inline bool
any_ns_so(First const& first, Last const& last, F& f, mpl::false_)
{
bool head = f(*first);
bool tail =
detail::any_ns_so(
fusion::next(first)
, last
, f
, fusion::result_of::equal_to<
typename fusion::result_of::next<First>::type, Last>());
return head || tail;
}
}

template <typename Sequence1, typename Sequence2, typename F>
inline bool
any_ns_so(Sequence1 const& seq1, Sequence2& seq2, F f)
{
return detail::any_ns_so(
fusion::begin(seq1)
, fusion::begin(seq2)
, fusion::end(seq1)
, f
, fusion::result_of::equal_to<
typename fusion::result_of::begin<Sequence1>::type
, typename fusion::result_of::end<Sequence1>::type>());
}

template <typename Sequence, typename F>
inline bool
any_ns_so(Sequence const& seq, unused_type, F f)
{
return detail::any_ns_so(
fusion::begin(seq)
, fusion::end(seq)
, f
, fusion::result_of::equal_to<
typename fusion::result_of::begin<Sequence>::type
, typename fusion::result_of::end<Sequence>::type>());
}

}}

#endif

18 changes: 18 additions & 0 deletions include/boost/spirit/include/support_any_if_ns_so.hpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*=============================================================================
Copyright (c) 2001-2011 Joel de Guzman
Copyright (c) 2001-2011 Hartmut Kaiser
http://spirit.sourceforge.net/
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)
=============================================================================*/
#ifndef BOOST_SPIRIT_INCLUDE_SUPPORT_ANY_IF_NS_SO
#define BOOST_SPIRIT_INCLUDE_SUPPORT_ANY_IF_NS_SO

#if defined(_MSC_VER)
#pragma once
#endif

#include <boost/spirit/home/support/algorithm/any_if_ns_so.hpp>

#endif
18 changes: 18 additions & 0 deletions include/boost/spirit/include/support_any_ns_so.hpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*=============================================================================
Copyright (c) 2001-2011 Joel de Guzman
Copyright (c) 2001-2011 Hartmut Kaiser
http://spirit.sourceforge.net/
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)
=============================================================================*/
#ifndef BOOST_SPIRIT_INCLUDE_SUPPORT_ANY_NS_SO
#define BOOST_SPIRIT_INCLUDE_SUPPORT_ANY_NS_SO

#if defined(_MSC_VER)
#pragma once
#endif

#include <boost/spirit/home/support/algorithm/any_ns_so.hpp>

#endif

0 comments on commit 6a37fde

Please sign in to comment.