Re: [RFC][Discussion] Parser extension API

On Wed, Feb 18, 2015 at 4:22 PM, Alexander Lisachenko < lisachenko.it@gmail.com> wrote: > > 2015-02-18 17:59 GMT+03:00 Nikita Popov <nikita.ppv@gmail.com>: > >> Alexander, I would recommend you to split this into two RFCs, one dealing >> only with AST export (and maybe pretty printing) and the second one with >> the compilation hooks. There's probably a few questions about the export >> API that should be discussed that will be forgotten if everyone focuses on >> the more complicated, and probably not yet relevant, question of >> compilation hooks. > > > > Hello, Nikita! Thanks for you thoughts! > > Sounds reasonable for me, because of short timeframe for PHP7. Let's go > with the first part of RFC, because second part is really cumbersome and > can be applied later, eg. in 7.1 I will split this RFC into two parts and > mark first part for 7.0. > Great! About first part, do you agree with proposed classes and namespace > Php\Parser? > Yeah, I'm okay with that namespace. I prefer php\ast as being more distinctive from existing projects, but that's not really important. > I want to propose to use classes for parser API instead of pure functions > because it can give more usable OO-API for future needs. > I'm okay with having stuff like ->getKindName() on the nodes, however I'd still keep around the free-standing functions, because they can be used without a node (i.e. you only need the kind AST_FOO and not an instantiated node). I also don't see why we need ParserEngine::parse() instead of just a (namespaced) parse() function. Not a fan of unnecessary wrapper classes. > There is one more notice regarding to the zval nodes, they should be also > exported as objects, not as values. I think we should export coherent nodes > for everything. > What's the advantage of this? Most leaf nodes will be zval nodes and I feel it would be rather inconvenient if they'd all have extra wrappers. Is there some specific problem with including the values directly? Nikita