Boost spirit core dump on parsing bracketed expression

Viewed 75

Having some simplified grammar that should parse sequence of terminal literals: id, '<', '>' and ":action". I need to allow brackets '(' ')' that do nothing but improve reading. (Full example is there http://coliru.stacked-crooked.com/a/dca93f5c8f37a889 ) Snip of my grammar:

    start  = expression % eol;
    expression   = (simple_def >> -expression)
    | (qi::lit('(') > expression > ')');

    simple_def = qi::lit('<') [qi::_val = Command::left] 
    | qi::lit('>') [qi::_val = Command::right] 
    | key [qi::_val = Command::id] 
    | qi::lit(":action") [qi::_val = Command::action] 
    ;
    
    key = +qi::char_("a-zA-Z_0-9");

When I try to parse: const std::string s = "(a1 > :action)"; Everything works like a charm. But when I little bit bring more complexity with brackets "(a1 (>) :action)" I've gotten coredump. Just for information - coredump happens on coliru, while msvc compiled example just demonstrate fail parsing.

So my questions: (1) what's wrong with brackets, (2) how exactly brackets can be introduced to expression.

p.s. It is simplified grammar, in real I have more complicated case, but this is a minimal reproduceable code.

1 Answers

You should just handle the expectation failure:

terminate called after throwing an instance of 'boost::wrapexcept<boost::spir
it::qi::expectation_failure<__gnu_cxx::__normal_iterator<char const*, std::__
cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >
>'
  what():  boost::spirit::qi::expectation_failure
Aborted (core dumped)

If you handle the expectation failure, the program will not have to terminate.

Fixing The Grammar

Your 'nested expression' rule only accepts a single expression. I think that

expression = (simple_def >> -expression)

is intended to match "1 or more `simple_def". However, the alternative branch:

     | ('(' > expression > ')');

doesn't accept the same: it just stops after parsing `)'. This means that your input is simply invalid according to the grammar.

I suggest a simplification by expressing intent. You were on the right path with semantic typedefs. Let's avoid the "weasely" Line Of Lines (what even is that?):

using Id     = std::string;
using Line   = std::vector<Command>;
using Script = std::vector<Line>;

And use these typedefs consistently. Now, we can express the grammar as we "think" about it:

    start  = skip(blank)[script];
    script = line % eol;

    line   = +simple;
    simple = group | command;
    group  = '(' > line > ')';

See, by simplifying our mental model and sticking to it, we avoided the entire problem you had a hard time spotting.

Here's a quick demo that includes error handling, optional debug output, both test cases and encapsulating the skipper as it is part of the grammar: Live On Compiler Explorer

#include <fmt/ranges.h>
#include <fmt/ostream.h>
#include <boost/spirit/include/qi.hpp>
#include <boost/spirit/include/phoenix.hpp>

namespace qi  = boost::spirit::qi;
namespace phx = boost::phoenix;

enum class Command { id, left, right, action };

static inline std::ostream& operator<<(std::ostream& os, Command cmd) {
    switch (cmd) {
        case Command::id: return os << "[ID]";
        case Command::left: return os << "[LEFT]";
        case Command::right: return os << "[RIGHT]";
        case Command::action: return os << "[ACTION]";
    }
    return os << "[???]";
}

using Id     = std::string;
using Line   = std::vector<Command>;
using Script = std::vector<Line>;

template <typename It>
struct ExprGrammar : qi::grammar<It, Script()> {
    ExprGrammar() : ExprGrammar::base_type(start) {
        using namespace qi;

        start  = skip(blank)[script];
        script = line % eol;

        line   = +simple;
        simple = group | command;
        group  = '(' > line > ')';

        command = 
            lit('<')       [ _val = Command::left   ] |
            lit('>')       [ _val = Command::right  ] |
            key            [ _val = Command::id     ] |
            lit(":action") [ _val = Command::action ] ;

        key = +char_("a-zA-Z_0-9");

        BOOST_SPIRIT_DEBUG_NODES((command)(line)(simple)(group)(script)(key));
    }

private:
    qi::rule<It, Script()>                 start;
    qi::rule<It, Line(), qi::blank_type>   line, simple, group;
    qi::rule<It, Script(), qi::blank_type> script;

    qi::rule<It, Command(), qi::blank_type> command;

    // lexemes
    qi::rule<It, Id()> key;
};

int main() {
    using It = std::string::const_iterator;
    ExprGrammar<It> const p;

    for (const std::string s : {
            "a1 > :action\na1 (>) :action",
            "(a1 > :action)\n(a1 (>) :action)",
            "a1 (> :action)",
        }) {

        It f(begin(s)), l(end(s));

        try {
            Script parsed;
            bool ok = qi::parse(f, l, p, parsed);

            if (ok) {
                fmt::print("Parsed {}\n", parsed);
            } else {
                fmt::print("Parsed failed\n");
            }

            if (f != l) {
                fmt::print("Remaining unparsed: '{}'\n", std::string(f, l));
            }
        } catch (qi::expectation_failure<It> const& ef) {
            fmt::print("{}\n", ef.what()); // TODO add more details :)
        }
    }
}

Prints

Parsed {{[ID], [RIGHT], [ACTION]}, {[ID], [RIGHT], [ACTION]}}
Parsed {{[ID], [RIGHT], [ACTION]}, {[ID], [RIGHT], [ACTION]}}
Parsed {{[ID], [RIGHT], [ACTION]}}

BONUS

However, I think this can all be greatly simplified using qi::symbols for the commands. In fact it looks like you're only tokenizing (you confirm this when you say that the parentheses are not important).

    line   = +simple;
    simple = group | command | (omit[key] >> attr(Command::id));
    group  = '(' > line > ')';
    key    = +char_("a-zA-Z_0-9");

Now you don't need Phoenix at all: Live On Compiler Explorer, printing

ok? true {{[ID], [RIGHT], [ACTION]}, {[ID], [RIGHT], [ACTION]}}
ok? true {{[ID], [RIGHT], [ACTION]}, {[ID], [RIGHT], [ACTION]}}
ok? true {{[ID], [RIGHT], [ACTION]}}

Even Simpler?

Since I observe that you're basically tokenizing line-wise, why not simply skip the parentheses, and simplify all the way down to:

    script = line % eol;
    line   = *(command | omit[key] >> attr(Command::id));

That's all. See it Live On Compiler Explorer again:

#include <boost/spirit/include/qi.hpp>
#include <fmt/ostream.h>
#include <fmt/ranges.h>
namespace qi = boost::spirit::qi;

enum class Command { id, left, right, action };
using Id     = std::string;
using Line   = std::vector<Command>;
using Script = std::vector<Line>;

static inline std::ostream& operator<<(std::ostream& os, Command cmd) {
    return os << (std::array{"ID", "LEFT", "RIGHT", "ACTION"}.at(int(cmd)));
}

template <typename It>
struct ExprGrammar : qi::grammar<It, Script()> {
    ExprGrammar() : ExprGrammar::base_type(start) {
        using namespace qi;
        start = skip(skipper.alias())[line % eol];
        line  = *(command | omit[key] >> attr(Command::id));
        key   = +char_("a-zA-Z_0-9");

        BOOST_SPIRIT_DEBUG_NODES((line)(key));
    }
private:
    using Skipper = qi::rule<It>;
    qi::rule<It, Script()>        start;
    qi::rule<It, Line(), Skipper> line;

    Skipper                 skipper = qi::char_(" \t\b\f()");
    qi::rule<It /*, Id()*/> key; // omit attribute for efficiency
    struct cmdsym : qi::symbols<char, Command> {
        cmdsym() { this->add("<", Command::left)
            (">", Command::right)
            (":action", Command::action);
        }
    } command;
};

int main() {
    using It = std::string::const_iterator;
    ExprGrammar<It> const p;

    for (const std::string s : {
            "a1 > :action\na1 (>) :action",
            "(a1 > :action)\n(a1 (>) :action)",
            "a1 (> :action)",
        })
    try {
        It f(begin(s)), l(end(s));

        Script parsed;
        bool ok = qi::parse(f, l, p, parsed);

        fmt::print("ok? {} {}\n", ok, parsed);
        if (f != l)
            fmt::print(" -- Remaining '{}'\n", std::string(f, l));
    } catch (qi::expectation_failure<It> const& ef) {
        fmt::print("{}\n", ef.what()); // TODO add more details :)
    }
}

Prints

ok? true {{ID, RIGHT, ACTION}, {ID, RIGHT, ACTION}}
ok? true {{ID, RIGHT, ACTION}, {ID, RIGHT, ACTION}}
ok? true {{ID, RIGHT, ACTION}}

Note I very subtly changed +() to *() so it would accept empty lines as well. This may or may not be what you want

Related