Re: [boost] Formal Review: The Spirit Parser Framework.

From: Dan Gohman (dgohman_at_[hidden])
Date: 2002-10-12 07:16:01

On Sat, Oct 12, 2002 at 08:38:48AM +0800, Joel de Guzman wrote:
> First of all, thanks!

Thanks to you and everyone else that has developed Spirit. It
is a very impressive library.

> Iff spirit gets accepted, namespace and the macro prefix will
> boost::spirit and BOOST_SPIRIT. However, since detail
> is an implementation convention, I'd prefer to keep impl if
> that will not (as I assume) affect the review, as a matter of
> choice.

I'll leave up to others any decision about use of `impl' instead
of `detail'.

> Pre 1.5 Spirit did it that way. Yet, here's the rub: doing so
> would prevent rules from being storable in containers. Previous
> experiments with dynamic parsers necessitated rules that are
> copyable and storable in stacks, vectors etc. 

Perhaps Spirit should use operator|= for the EBNF syntax. This function
could then provide behavior whatever semantics desired without having
to be constrained by the otherwise-unrelated container requirements.

For example, operator|= could implement the 1.3 behavior:

  r |= a;
  r |= b;

This would be equivalant to `r = a | b' in 1.5 syntax.

When writing in the EBNF-syntax, I don't want to have to worry about
details like remembering to use .alias() for rules that need it.

> > Types like anychar_ look like they should be moved into an
> > implementation-detail namespace.
> 
> Why?

They're not mentioned in the documentation, and as far as I can see, library
users never need to use these by name; anychar_p and the others are the
way that library users use these.

> > How does one instantiate the four-parameter ast_parse function template?
> > I don't know if I just haven't done it right or if there's a bug in
> > Spirit. It isn't ever done in the examples, and there doesn't appear to
> > be tests for any of the tree components :-(.
> 
> I CC'd Dan Nuffer. He's the author of the AST/Parse trees.

Upon further investiagion, it appears to be a bug.

--- spirit.orig/boost/spirit/tree/ast.hpp
+++ spirit/boost/spirit/tree/ast.hpp
@@ -248,7 +248,7 @@
     scanner_policies_t policies(iter_policy);
     IteratorT first = first_;
     scanner_t scan(first, last_, policies);
-    tree_match<IteratorT> hit = parser.parse(scan);
+    tree_match<IteratorT> hit = parser.derived().parse(scan);
     return tree_parse_info<IteratorT>(
         first, hit, hit && (first == last_), hit.length(), hit.trees);
 }

-- 
Dan Gohman
dgohman_at_[hidden]