On 04/24/2016 05:02 PM, Thomas Punt wrote:
Hi!

From: dmi...@zend.com
On 04/22/2016 02:46 PM, Thomas Punt wrote:
Hi Dmitry!

Just a couple of comments on this:

1. I'd definitely reuse the php-ast extension for parsing the code into an
AST. It performs a number of transformations on PHP's underlying AST
that make it much nicer to use (namely better consistency). It is also
less fragile by having the abstraction between PHP's internal AST and
the AST that is exposed to userland (enabling for internal AST changes
without impacting the AST exposed to userland).
I'm not sure. Both approaches make sense.
Whilst fragility is a concern, a bigger problem with not reusing
ast\parse_code() is that the AST produced will be different in even the
simplest of cases. For example, take the following simple expression:

$a> 2

With your attributes branch, this will produce an AST of:

object(ast\Node)#2 (4) {
     ["kind"]=> int(521) // ZEND_AST_GREATER
     ["flags"]=> int(0)
     ["lineno"]=> int(1)
     ["children"]=> [...]
}

Whereas with ast\parse_code(), the following AST will be produced:

object(ast\Node)#2 (4) {
     ["kind"]=> int(520) // AST_BINARY_OP
     ["flags"]=> int(256) // BINARY_IS_GREATER
     ["lineno"]=> int(1)
     ["children"]=> [...]
}

The php-ast extension transforms the special node types for>,>=, <, <=
into AST_BINARY_OP nodes with different flags set. This is just one
difference of many between the internal AST and the AST produced by
the php-ast extension.

This is initial implementation details, and they are going to be fixed.
If we decide to use AST in attributes, getAttributes() outout are going to be 100% compatible with php-ast.


2. You mentioned about moving some of the php-ast extension into core.
I wonder if it would be better to just move the whole extension into the
core first, and then enable this functionality if the php-ast extension is
enabled.
Even if we move php-ast into core (I think we will do it), it's going to
be optional.
However attributes should always work.
If we're going to reuse ast\parse_code(), then we will need the whole php-ast
extension in the core. It could therefore be made similar to the pcre, date,
Reflection, SPL, etc extensions, where it cannot be disabled.

Right,  but this is out of scope of this RFC.
Nikita was going to propose moving php-ast into core soon.

Thanks. Dmitry.


Also, slightly tangential, but the RFC says attributes are supported on
class constants, yet doc comments (IIRC) are not. I wonder if support
for doc comments should be added for class constants?
it is already implemented.
Oh right, my mistake. It does not seem to be exposed through php-ast or
reflection then.

Thanks. Dmitry.
Thanks,
Tom
-Tom                                    


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to