[RFC][Discussion] Parser extension API

Alexander Lisachenko wrote on 17/02/2015 12:41:

2015-02-17 15:29 GMT+03:00 Benjamin Eberlei kontakt@beberlei.de:

Well not really, depending on the path towards a require a parser
extension is already registered or not. Or you have conditional
registration of extensions. So you could load a file with an extension
registered, but it still serves the old opcodes for that file.

Technically, this will be possible, however, Parser Extensions should be
registered ASAP during bootstrap process, near spl_autoload_register(),
stream_wrapper_register(), stream_filter_register(), etc..

I think "should" is OK for extensions, but not for userland hooks. There
needs to be a very firm definition of the right and wrong way to
implement these hooks, which can at the very least warn the user when
they are slipping into undefined behaviour.

Note that spl_autoload_register doesn't affect OpCache because it only
changes the mapping of a class to a parsed file (keyed on the file
system path to the source), not the compiled contents of that file.

If AST hooks can be registered on a global scope at any time, then
compiling the same file can lead to different op codes at different
times (e.g. page loads which take a different path through the code),
which would mean the OpCache would need extra information in its keys to
cache each possible version.

One more possible way for this RFC is to remove these register/unregister
methods from the Php\Parser\Engine class and add an option to the php.ini
with list of classes to load. This option can be adjusted then per
project/directory. Is this more suitable?

The other alternative is to go to the other extreme, and have the
extensions scoped to a particular file, more like Perl pragmas. Dare I
say we could use the declare() syntax that everyone seems to have an
irrational hatred of?

declare(pragma=Example\DbcParserExtension);

Regards,

Rowan Collins
[IMSoP]

Hi,

On Tue, Feb 17, 2015 at 2:46 PM, Alexander Lisachenko <
lisachenko.it@gmail.com> wrote:

Hello, internals!

I want to introduce a RFC for providing a userland API for accessing an
Abstract Syntax Tree of the source code and to provide userland parser
hooks for source code modification:
https://wiki.php.net/rfc/parser-extension-api

Thanks!

The first part, describing https://github.com/nikic/php-ast , looks
fine.
I see no problems including this extension into PHP-7.0 core
distribution.
May be even making it required (like ext/reflection).

Nikita, what do you think?

The second part looks very interesting, however it has some uncovered
questions.

• API for AST modification
• AST validation (someone may insert "break" node in "parameter-list").

If you have enough experience, I would suggest you to try writing an
external extension that would implement this idea.
If you'll need some modification in PHP core (e.g adding callbacks), we
may consider including them in PHP-7.0.

Thanks. Dmitry.

I agree with Dmitry.

Exporting the AST to userland in PHP 7.0 would be nice. With Dmitry's
work
on assertions we even have a pretty printer for it, which allows us to
convert the AST back to PHP code.

In this matter, how would it be? It'd be awesome if function expects an
AST tree object instead of a value:

function doQuery($table, *$where) {
   $where = convert_ast_toSqlWhere($where);
}

doQuery("foobar", $col1 = "something" AND $col2 == $col3);

Or at least ast(<expr>), although both would be nice to have.

The problem here is that in most cases we do not actually know what
function will be called at compile time. A very simple example would be to
replace doQuery() with $db->query() and already we don't know what it is
that we're actually calling and whether or not it needs an AST. However
this does not apply to just methods, it's an issue with nearly all calls.

As such PHP cannot know at compile time whether it should issue a normal
call with evaluated arguments or whether it needs to preserve the AST and
pass that. Just keeping the AST around for all calls would be too expensive
(it's a pretty big memory hog).

So rather than having this as a modifier in the function signature, it
would have to be a modifier in the call syntax. E.g. rust uses foo!()
syntax for macros.

Nikita

Wouldn't the trick be to have $col1 be an object instance that uses
operator overloading? So instead of solving the expression, operators
would return the AST.

Was possible with the operator extension

Some old experimental code from 2006

echo query($connection,
array($catalog->Person->id, $catalog->Titles->title. ' ' .
$catalog->Person->name),
new SqlJoin(SqlJoin::TYPE_INNER,
$catalog->Person,
$catalog->Titles,
$catalog->Person->titleId == $catalog->Titles->id),
$catalog->Person->id == 1);

outputs

SELECT Person.id, Titles.title || ' ' || Person.name FROM Person INNER
JOIN Titles ON Person.titleId = Titles.id WHERE Person.id = 1

Jared

I think the AST API shouldn't use "public" properties.
Using it, we will have to construct the whole tree of objects, duplicating
information from AST.
I would propose SimpleXML approach instead - construct object only for
node(s) we currently access.

So at first we will construct just single object referring to AST root.
Then traversing it we will create and destroy objects for necessary nodes.

To access children I would propose to implementing ArrayAccess interface.

$ast = \php\ast\parse($string);
foreach ($ast as $child) {
echo "\t" . $child->getKindName() . "\n";
foreach ($child as $grandchild) {
echo "\t" . $child->getKindName() . "\n";

}
}

Thanks. Dmitry.

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

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 think that getting name of kind is useless without value of kind.
According to the RFC, there is only one way from user land to get a kind of
node: ask a parser for an AST, that will contain nodes. Is there any reason
to do getKindName(1), where 1 - is just integer value. I can't see how it
can be useful, this information is fully related to the 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.

I think that PHP is ugly enough :) We should try to use namespaces and
classes with methods to provide a better experience for user land
developers. Classes allow for logical composition of common functionality,
provides autocompletion in IDE and can be considered as step to
standardization of core API. (Today I discussed this question with
colleagues that will be great to have all system classes in Php namespace
for PHP>=7.0)

I know, that on engine level, classes aren't native, but you should think
about how this feature will be used by developers. Because, creation of API
is one-time process, but imagine how many times it will be used by
developers. I think, it will be more polite to provide OO-code for modern
API.

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?

Yes, reason here is the same as for token_get_all(). I hear so many WTFs
when value can be either simple character or array. Much harder to work
with inconsistent data and many libraries performs additional steps to
provide better usability (see
https://github.com/Andrewsville/PHP-Token-Reflection/blob/1c5ff91ee6877d1fb0f60fab3e3fc562bdb02684/TokenReflection/Stream/StreamBase.php#L100-L108
or
https://github.com/lisachenko/go-aop-php/blob/3c381e243367096d47dc9676b3ad7445d7720ba9/src/Instrument/Transformer/MagicConstantTransformer.php#L117
)

So, each element should be represented as common node, this will simplify
usage of API.