Code Vamping

Spirit X3 File Organization

C++ Boost

For larger Spirit-based project, organizing the source code well can lead to more efficient builds and increased maintainability.

Of course, this is true for any project. But the heavily templated nature of even a fully realized Spirit parser makes this doubly so. Figuring out how to take advantage of separate compilation while maintaining the ability for each of the pieces to see the needed type/template information is not trivial.

Fortunately, the library authors have given us a starting place. Please go read and ponder that and then come back here.

Obviously, I have some problems with the organization proposed in that article. So lets dive in.

Iterate this

employee.hpp is billed as the “Main parser API”. But there is something missing - the iterator_type. main needs to know that type in order to properly call parse_phrase. employee.cpp needs it to correctly instantiate the parser. And the two better agree. If they don’t, at best you get an ugly compile error. At worst, you get an ugly unhelpful link error.

But, notice that main doesn’t include config.hpp. Instead, it redefines iterator_type. Yikes. So, two places to update if (for instance) you want to use the boost::spirit::istream_iterator as your iterator type.

short aside Be sure to call phrase_parse without a namespace. Do not call it like x3::phrase_parse. Doing so turns off Argument Dependent Lookup. Evil ensues.

Spaced out

While you are looking at the phrase_parse call, look at the 4th argument space. It is not immediately obvious, but that needs to match the template argument to phrase_parse_context<> - helpfully found in config.hpp. Again, if for no other reason than documentation, this needs to be visible to main.cpp

My suggestion would be to get rid of config.hpp and just fold its contents into employee.hpp. The separate file adds no value. Anyone who needs employee.hpp will need to see the contents of config.hpp.

But, to my mind that doesn’t really help with the skip processor problem. Let’s say you write your own parser that needs to be used with your grammar - we’ll call it my_skipper. The grammar you produce is no doubt going to rely on the fact that my_skipper is doing the skipping for correctness. So, in order for main.cpp create the correct call to phrase_parse, it needs to see the definition of my_skipper - a type it doesn’t otherwise care about.

We’ll do a bit better after looking at two more gripe.

Header-mania

Having the actual grammar definition in a separate file is a great idea. But this probably makes more sense to be folded into employee.cpp. The only counter-argument is that it reduces reusability of the grammar since it cannot be embedded in something larger.

True, but unless the grammar is quite simple, you will also need to pick up all its assumptions (e.g. skipper, iterator_type, ast, etc).

Adopting adapters

The AST definitions are in their own file. This makes sense. But is there any reason that the BOOST_FUSION_ADAPT_STRUCT calls are in a different file? I couldn’t come up with any. Looking at the generated code, it doesn’t (obviously) define objects - just template classes.

A Modest Proposal

See github for the worked out example. This is based on the same grammar example as the orginal.

Many of the problems listed above is because the grammar must leak quite a few non-negotiable details to main.cpp in order to allow it to properly call phrase_parse. I propose always encapulating that call into the grammar. The grammar should instead expose a do_parse or similar function that actually does this call. The only thing the client has to provide is the iterators.

The exact details of the function is a matter of taste and project requirements. For instance, an error reporting stream (or object) can be provided. But for this example it is as simple as:

std::pair<bool, ast::employee>do_parse(parser::iterator_type &first, parser::iterator_type const &last) {
    ast::employee e;

    bool r = phrase_parse(first, last, parser::employee_type(), x3::ascii::space, e);

    return std::make_pair(r, e);
}

Lets go over each of files:

ast.hpp

The definition of the ast structures and the needed fusion adaptors.

employee.hpp

This is - as before - the “API”. It contains the include to the ast definitions. It also contains the declaration for the do_parse function. So, this is the only file that a client needs to include in order to work with this parser.

employee.cpp

Everything else.

Nice and tidy. The grammar can grow and change - you can even rename the upper-level grammar symbol - without affecting the code in the client. Want to use that new awesome my_skipper? No-one needs to know. That code might reside in a separate file if need be, but that is a negotiation between the library and the parser - the client isn’t involved.