[VOTE] Abstract Syntax Tree

11 years ago by Andi Gutmans — view source — reply

unread

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the AST.
Are these as a side-effect of how AST was implemented? Or are they unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time separating out the benefits of the AST (which I really like) and making other changes to semantics w/o really understanding if they are side-effects and we have no choice vs. we’re trying to solve for two separate items in one RFC.
Any BC breaks here we think could bite us?

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

11 years ago by Dmitry Stogov — view source — reply

unread

Hi Andi,

We already discussed most of semantic changes introduced in AST patch.
Most of them came from another approved RFC
https://wiki.php.net/rfc/uniform_variable_syntax

Thanks. Dmitry.

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

11 years ago by Laruence — view source — reply

unread

Hey:

I find a problem while I was trying play with the patch

Generating phar.php
Generating phar.phar

Fatal error: Out of memory (allocated 786432) at
/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h:69 (tried
to allocate 34359738400 bytes) in
/home/huixinchen/opensource/github/php-src/ext/phar/phar.php on line
163
make: *** [ext/phar/phar.phar] Error 255
make: *** Waiting for unfinished jobs....

is this a knew issue?

thanks

Hi Andi,

We already discussed most of semantic changes introduced in AST patch.
Most of them came from another approved RFC
https://wiki.php.net/rfc/uniform_variable_syntax

Thanks. Dmitry.

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

--

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Laruence — view source — reply

unread

Hey:

the problem came from zend_ast_size for 0 children node..

backtrace:

Program terminated with signal 6, Aborted.
#0 0x00000034dc030285 in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x00000034dc030285 in raise () from /lib64/libc.so.6
#1 0x00000034dc031d30 in abort () from /lib64/libc.so.6
#2 0x00000000009d271c in _zend_mm_alloc_int (heap=0x82fff40, size=34359738400,
__zend_filename=0xfc5c78
"/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h",
__zend_lineno=69,
__zend_orig_filename=0x0, __zend_orig_lineno=0)
at /home/huixinchen/opensource/github/php-src/Zend/zend_alloc.c:2003
#3 0x00000000009d4354 in _emalloc (size=34359738400,
__zend_filename=0xfc5c78
"/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h",
__zend_lineno=69,
__zend_orig_filename=0x0, __zend_orig_lineno=0)
at /home/huixinchen/opensource/github/php-src/Zend/zend_alloc.c:2442
#4 0x0000000000a42c05 in zend_arena_alloc (arena_ptr=0x135e698,
size=34359738376)
at /home/huixinchen/opensource/github/php-src/Zend/zend_arena.h:69
#5 0x0000000000a42b0e in zend_ast_alloc (size=34359738376)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:27
#6 0x0000000000a42dcb in zend_ast_create_from_va_list (kind=1,
attr=7, va=0x7fff2be64550)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:89
#7 0x0000000000a4303e in zend_ast_create_ex (kind=1, attr=7)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:116
#8 0x00000000009aa0a3 in zendparse () at
/home/huixinchen/opensource/github/php-src/Zend/zend_language_parser.y:551
#9 0x00000000009aed81 in compile_file (file_handle=0x7fff2be68850,
type=8) at Zend/zend_language_scanner.l:586
#10 0x0000000000784de6 in phar_compile_file (file_handle=0x7fff2be68850, type=8)
at /home/huixinchen/opensource/github/php-src/ext/phar/phar.c:3358
#11 0x0000000000a0bab2 in zend_execute_scripts (type=8, retval=0x0,
file_count=3)
at /home/huixinchen/opensource/github/php-src/Zend/zend.c:1301
#12 0x000000000097529b in php_execute_script (primary_file=0x7fff2be68850)
at /home/huixinchen/opensource/github/php-src/main/main.c:2564
#13 0x0000000000ac44b1 in do_cli (argc=31, argv=0x82ff680)
at /home/huixinchen/opensource/github/php-src/sapi/cli/php_cli.c:980
#14 0x0000000000ac55de in main (argc=31, argv=0x82ff680)
at /home/huixinchen/opensource/github/php-src/sapi/cli/php_cli.c:1358
(gdb) f 6
#6 0x0000000000a42dcb in zend_ast_create_from_va_list (kind=1,
attr=7, va=0x7fff2be64550)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:89
89 zend_ast *ast = zend_ast_alloc(zend_ast_size(children) TSRMLS_CC);
(gdb) p children
$1 = 0

thanks

Hey:
I find a problem while I was trying play with the patch
Generating phar.php
Generating phar.phar

Fatal error: Out of memory (allocated 786432) at
/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h:69 (tried
to allocate 34359738400 bytes) in
/home/huixinchen/opensource/github/php-src/ext/phar/phar.php on line
163
make: *** [ext/phar/phar.phar] Error 255
make: *** Waiting for unfinished jobs....

is this a knew issue?

thanks

Hi Andi,

We already discussed most of semantic changes introduced in AST patch.
Most of them came from another approved RFC
https://wiki.php.net/rfc/uniform_variable_syntax

Thanks. Dmitry.

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

--

--
Laruence Xinchen Hui
http://www.laruence.com/

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Laruence — view source — reply

unread

Hey:

a fix could be:
diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c
index eb35a51..f738e34 100644
--- a/Zend/zend_ast.c
+++ b/Zend/zend_ast.c
@@ -33,11 +33,11 @@ static inline void *zend_ast_realloc(void *old,
size_t old_size, size_t new_size
return new;
}

-size_t zend_ast_size(zend_uint children) {
+size_t zend_ast_size(int children) {
return sizeof(zend_ast) + sizeof(zend_ast *) * (children - 1);
}

-size_t zend_ast_list_size(zend_uint children) {
+size_t zend_ast_list_size(int children) {
return sizeof(zend_ast_list) + sizeof(zend_ast *) * (children - 1);
}

my compiler must take (children -1) as a unsigned

thanks

Hey:

the problem came from zend_ast_size for 0 children node..

backtrace:

Program terminated with signal 6, Aborted.
#0 0x00000034dc030285 in raise () from /lib64/libc.so.6
(gdb) bt
#0 0x00000034dc030285 in raise () from /lib64/libc.so.6
#1 0x00000034dc031d30 in abort () from /lib64/libc.so.6
#2 0x00000000009d271c in _zend_mm_alloc_int (heap=0x82fff40, size=34359738400,
__zend_filename=0xfc5c78
"/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h",
__zend_lineno=69,
__zend_orig_filename=0x0, __zend_orig_lineno=0)
at /home/huixinchen/opensource/github/php-src/Zend/zend_alloc.c:2003
#3 0x00000000009d4354 in _emalloc (size=34359738400,
__zend_filename=0xfc5c78
"/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h",
__zend_lineno=69,
__zend_orig_filename=0x0, __zend_orig_lineno=0)
at /home/huixinchen/opensource/github/php-src/Zend/zend_alloc.c:2442
#4 0x0000000000a42c05 in zend_arena_alloc (arena_ptr=0x135e698,
size=34359738376)
at /home/huixinchen/opensource/github/php-src/Zend/zend_arena.h:69
#5 0x0000000000a42b0e in zend_ast_alloc (size=34359738376)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:27
#6 0x0000000000a42dcb in zend_ast_create_from_va_list (kind=1,
attr=7, va=0x7fff2be64550)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:89
#7 0x0000000000a4303e in zend_ast_create_ex (kind=1, attr=7)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:116
#8 0x00000000009aa0a3 in zendparse () at
/home/huixinchen/opensource/github/php-src/Zend/zend_language_parser.y:551
#9 0x00000000009aed81 in compile_file (file_handle=0x7fff2be68850,
type=8) at Zend/zend_language_scanner.l:586
#10 0x0000000000784de6 in phar_compile_file (file_handle=0x7fff2be68850, type=8)
at /home/huixinchen/opensource/github/php-src/ext/phar/phar.c:3358
#11 0x0000000000a0bab2 in zend_execute_scripts (type=8, retval=0x0,
file_count=3)
at /home/huixinchen/opensource/github/php-src/Zend/zend.c:1301
#12 0x000000000097529b in php_execute_script (primary_file=0x7fff2be68850)
at /home/huixinchen/opensource/github/php-src/main/main.c:2564
#13 0x0000000000ac44b1 in do_cli (argc=31, argv=0x82ff680)
at /home/huixinchen/opensource/github/php-src/sapi/cli/php_cli.c:980
#14 0x0000000000ac55de in main (argc=31, argv=0x82ff680)
at /home/huixinchen/opensource/github/php-src/sapi/cli/php_cli.c:1358
(gdb) f 6
#6 0x0000000000a42dcb in zend_ast_create_from_va_list (kind=1,
attr=7, va=0x7fff2be64550)
at /home/huixinchen/opensource/github/php-src/Zend/zend_ast.c:89
89 zend_ast *ast = zend_ast_alloc(zend_ast_size(children) TSRMLS_CC);
(gdb) p children
$1 = 0

thanks
Hey:
I find a problem while I was trying play with the patch
Generating phar.php
Generating phar.phar

Fatal error: Out of memory (allocated 786432) at
/home/huixinchen/opensource/github/php-src/Zend/zend_arena.h:69 (tried
to allocate 34359738400 bytes) in
/home/huixinchen/opensource/github/php-src/ext/phar/phar.php on line
163
make: *** [ext/phar/phar.phar] Error 255
make: *** Waiting for unfinished jobs....

is this a knew issue?

thanks

Hi Andi,

We already discussed most of semantic changes introduced in AST patch.
Most of them came from another approved RFC
https://wiki.php.net/rfc/uniform_variable_syntax

Thanks. Dmitry.

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

--

--
Laruence Xinchen Hui
http://www.laruence.com/
--
Laruence Xinchen Hui
http://www.laruence.com/

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Pierre Joye — view source — reply

unread

Hey:

a fix could be:
diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c
index eb35a51..f738e34 100644
--- a/Zend/zend_ast.c
+++ b/Zend/zend_ast.c
@@ -33,11 +33,11 @@ static inline void *zend_ast_realloc(void *old,
size_t old_size, size_t new_size
return new;
}

-size_t zend_ast_size(zend_uint children) {
+size_t zend_ast_size(int children) {
return sizeof(zend_ast) + sizeof(zend_ast *) * (children - 1);
}

-size_t zend_ast_list_size(zend_uint children) {
+size_t zend_ast_list_size(int children) {
return sizeof(zend_ast_list) + sizeof(zend_ast *) * (children - 1);
}

my compiler must take (children -1) as a unsigned

It does, the result of the expression will be.

But the fix is imo wrong. A size cannot be negative, per se.

It would be cleaner to do:

size_t zend_ast_size(int children) {
if (children > 0) {
return sizeof(zend_ast) + sizeof(zend_ast *) * (children - 1);
} else {
// 0 or sizeof(zend_ast) + sizeof(zend_ast *) if at least one
elem is allocated (NULLed).
}
}

Cheers,

Pierre

@pierrejoye | http://www.libgd.org

11 years ago by Nikita Popov — view source — reply

unread

Hey:

the problem came from zend_ast_size for 0 children node..

Thanks for the report, this should be fixed now:
https://github.com/nikic/php-src/commit/69f0deb399eec76bc8afa97fa383b08a207582fb

Nikita

11 years ago by Andi Gutmans — view source — reply

unread

got it. thanks!

Hi Andi,

We already discussed most of semantic changes introduced in AST patch.
Most of them came from another approved RFC https://wiki.php.net/rfc/uniform_variable_syntax

Thanks. Dmitry.

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the AST.
Are these as a side-effect of how AST was implemented? Or are they unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time separating out the benefits of the AST (which I really like) and making other changes to semantics w/o really understanding if they are side-effects and we have no choice vs. we’re trying to solve for two separate items in one RFC.
Any BC breaks here we think could bite us?

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

11 years ago by Paul Dragoonis — view source — reply

unread

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Hi Nikita,

I second some of Andi's concerns about behavioural changes, some things
working from right-to-left and not left-to-right. Are these things that we
can adapt the AST part or tweak another part to make this more consistent
with previous versions of PHP. Improving consistency when changing
underlying parts is a good migration technique. Facebook done this when
they implemented HHMV, they didn't fix bugs but made a like-for-like stable
copy before tackling improvements.

Do you feel it's an important goal to keep consistency from PHP 5.x to PHP
7, with regards to the behaviour of syntax parsing/behaviour, or do you
feel that you can live with these non-BC changes?

Many thanks,
Paul

Any insight would be much appreciated.
Thanks!
Andi

Hi internals!

I've opened the vote on the Abstract Syntax Tree RFC:

https://wiki.php.net/rfc/abstract_syntax_tree#vote

Thanks,
Nikita

11 years ago by Nikita Popov — view source — reply

unread

Hi Nikita,

I second some of Andi's concerns about behavioural changes, some things
working from right-to-left and not left-to-right. Are these things that we
can adapt the AST part or tweak another part to make this more consistent
with previous versions of PHP. Improving consistency when changing
underlying parts is a good migration technique. Facebook done this when
they implemented HHMV, they didn't fix bugs but made a like-for-like stable
copy before tackling improvements.

Do you feel it's an important goal to keep consistency from PHP 5.x to PHP
7, with regards to the behaviour of syntax parsing/behaviour, or do you
feel that you can live with these non-BC changes?

Many thanks,
Paul

Is there some particular change that concerns you? This is something that
needs to be decided on a case-by-case basis. The RFC contains a list of
changes. All of them are minor adjustments verging on bug fixes, which imho
are all within the bounds of reasonable changes for a major version.
However if you can provide argument why one of the listed changes is
incorrect (as in, introduces less intuitive/consistent/... behavior) or has
larger BC impact than anticipated, then I'm open to discussion that.

Nikita

11 years ago by Paul Dragoonis — view source — reply

unread

On Wed, Aug 20, 2014 at 3:30 PM, Paul Dragoonis dragoonis@gmail.com
wrote:

Hi Nikita,

I second some of Andi's concerns about behavioural changes, some things
working from right-to-left and not left-to-right. Are these things that we
can adapt the AST part or tweak another part to make this more consistent
with previous versions of PHP. Improving consistency when changing
underlying parts is a good migration technique. Facebook done this when
they implemented HHMV, they didn't fix bugs but made a like-for-like stable
copy before tackling improvements.

Do you feel it's an important goal to keep consistency from PHP 5.x to
PHP 7, with regards to the behaviour of syntax parsing/behaviour, or do you
feel that you can live with these non-BC changes?

Many thanks,
Paul

Is there some particular change that concerns you? This is something
that needs to be decided on a case-by-case basis. The RFC contains a list
of changes. All of them are minor adjustments verging on bug fixes, which
imho are all within the bounds of reasonable changes for a major version.
However if you can provide argument why one of the listed changes is
incorrect (as in, introduces less intuitive/consistent/... behavior) or has
larger BC impact than anticipated, then I'm open to discussion that.

I will get back to you with some specific use cases that I see could be
potentially problematic and non-intuitive.

Nikita

11 years ago by Levi Morrison — view source — reply

unread

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Hi Nikita,

I second some of Andi's concerns about behavioural changes, some things
working from right-to-left and not left-to-right. Are these things that we
can adapt the AST part or tweak another part to make this more consistent
with previous versions of PHP.

I just had a chat with Paul about this, and there was a
misunderstanding about what this RFC actually does regarding
left-to-right evaluation on list() constructs. I also saw this
misunderstanding in a few other places, so it seems to be a common
misunderstanding.

Under the proposed changes as well as in PHP 5.x, the end result of
the following is the same:

list($a, $b) = array(1, 2);

$a will be 1 and $b will be 2. Again, this hasn't changed. The
difference is that in PHP 5.x the variable $b will be assigned 2
before $a is assigned 1; under these changes $a will be assigned 1
first and then $b will be assigned 2.

This really only affects strange situations, such as ones listed in the RFC.

11 years ago by Paul Dragoonis — view source — reply

unread

On Wed, Aug 20, 2014 at 7:30 AM, Paul Dragoonis dragoonis@gmail.com
wrote:

Hi Nikita,

I reviewed the AST RFC on my way to vote but there was something that
wasn’t clear to me.
This patch introduces some semantic/behavioral changes in addition to
the
AST.
Are these as a side-effect of how AST was implemented? Or are they
unrelated to the AST patch?
I think some of them make sense but I’m having a bit of a hard time
separating out the benefits of the AST (which I really like) and making
other changes to semantics w/o really understanding if they are
side-effects and we have no choice vs. we’re trying to solve for two
separate items in one RFC.
Any BC breaks here we think could bite us?

Hi Nikita,

I second some of Andi's concerns about behavioural changes, some things
working from right-to-left and not left-to-right. Are these things that
we
can adapt the AST part or tweak another part to make this more consistent
with previous versions of PHP.

I just had a chat with Paul about this, and there was a
misunderstanding about what this RFC actually does regarding
left-to-right evaluation on list() constructs. I also saw this
misunderstanding in a few other places, so it seems to be a common
misunderstanding.

Under the proposed changes as well as in PHP 5.x, the end result of
the following is the same:
list($a, $b) = array(1, 2);
$a will be 1 and $b will be 2. Again, this hasn't changed. The
difference is that in PHP 5.x the variable $b will be assigned 2
before $a is assigned 1; under these changes $a will be assigned 1
first and then $b will be assigned 2.

I'm happy with this behaviour and agree that the AST version is what I
would expect, my confusion was that I thought PHP 5.x was 1,2 and the AST
was 2,1 which it of course is not the case. Now you could see why I raised
concerns about this in the first place.

This really only affects strange situations, such as ones listed in the
RFC.

11 years ago by Pascal MARTIN — view source — reply

unread

>
> I've opened the vote on the Abstract Syntax Tree RFC:
>
> https://wiki.php.net/rfc/abstract_syntax_tree#vote
>

After speaking with other members of AFUP (French UG) about this RFC, we
pretty much all agree that a cleaner compilation process is a nice
improvement.

It should help us getting some syntax options that were impossible
before, and there is not too much impact for developers and their
existing applications.

So, basically: +1

--
Pascal MARTIN
http://blog.pascal-martin.fr/
@pascal_martin

11 years ago by Nikita Popov — view source — reply

unread

> Hi internals!
>
> I've opened the vote on the Abstract Syntax Tree RFC:
>
> https://wiki.php.net/rfc/abstract_syntax_tree#vote
>

The Abstract Syntax Tree RFC has been accepted unanimously, with 47 votes
in favor and 0 against.

I'll merge the patch as soon as the int64 naming issues are resolved.

Nikita

11 years ago by Sebastian Bergmann — view source — reply

unread

Am 25.08.2014 um 19:01 schrieb Nikita Popov:

The Abstract Syntax Tree RFC has been accepted unanimously, with 47
votes in favor and 0 against.

Yay! :-) Thank you for your work, Nikita.

I'll merge the patch as soon as the int64 naming issues are resolved.

Do you when you'll get around to implement the extension that provides
userland access to the AST?

Cheers,
Sebastian

11 years ago by Nikita Popov — view source — reply

unread

On Mon, Aug 25, 2014 at 7:07 PM, Sebastian Bergmann sebastian@php.net
wrote:

Am 25.08.2014 um 19:01 schrieb Nikita Popov:

The Abstract Syntax Tree RFC has been accepted unanimously, with 47
votes in favor and 0 against.

Yay! :-) Thank you for your work, Nikita.

I'll merge the patch as soon as the int64 naming issues are resolved.

Do you when you'll get around to implement the extension that provides
userland access to the AST?

I don't know if I will be implementing that ext myself. In any case, before
that can happen I will have to create another RFC for converting parse
errors into exceptions and making sure we don't leak memory on failed parse

I think thats a prerequisite for exposing the parser to userland.

Nikita

11 years ago by Stas Malyshev — view source — reply

unread

Hi!

I don't know if I will be implementing that ext myself. In any case, before
that can happen I will have to create another RFC for converting parse
errors into exceptions and making sure we don't leak memory on failed parse

If we're talking the plunge of putting exceptions into the core engine,
wouldn't it make sense to convert most of the fatal errors (especially
the "catchable" ones) to exceptions too? Most of them (except for out of
memory, timeouts and such) don't really require the engine to shut down,
as it seems... So if we're already saying people will have to deal with
exceptions in PHP engine anyway, maybe we could make it so that we could
have unified handling of them?

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

11 years ago by Trevor Suarez — view source — reply

unread

If we're talking the plunge of putting exceptions into the core engine,
wouldn't it make sense to convert most of the fatal errors (especially
the "catchable" ones) to exceptions too? Most of them (except for out of
memory, timeouts and such) don't really require the engine to shut down,
as it seems... So if we're already saying people will have to deal with
exceptions in PHP engine anyway, maybe we could make it so that we could
have unified handling of them?

This is something that Nikita had originally proposed in an RFC (
https://wiki.php.net/rfc/engine_exceptions) but postponed until after 5.6.

I have a feeling we'll be hearing about its resurrection soon enough.

Trevor

11 years ago by Stas Malyshev — view source — reply

unread

Hi!

This is something that Nikita had originally proposed in an RFC
(https://wiki.php.net/rfc/engine_exceptions) but postponed until after 5.6.

I have a feeling we'll be hearing about its resurrection soon enough.

5.6 was not a good place for it, but PHP 7 very well may be. Some
cleanup to error handling is long due, and since we're talking about
fatal errors (which now stop the script from working) changes in this
area shouldn't break too much code that wasn't already broken.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

11 years ago by Benjamin Eberlei — view source — reply

unread

On Tue, Aug 26, 2014 at 1:12 AM, Stas Malyshev smalyshev@sugarcrm.com
wrote:

Hi!

This is something that Nikita had originally proposed in an RFC
(https://wiki.php.net/rfc/engine_exceptions) but postponed until after
5.6.

I have a feeling we'll be hearing about its resurrection soon enough.

5.6 was not a good place for it, but PHP 7 very well may be. Some
cleanup to error handling is long due, and since we're talking about
fatal errors (which now stop the script from working) changes in this
area shouldn't break too much code that wasn't already broken.

Depends, registering for shutdown handlers and catching fatals is a pretty
common thing. If EngineException does not get caught, it should produce an
E_FATAL again with the correct error code.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

11 years ago by Andrea Faulds — view source — reply

unread

Depends, registering for shutdown handlers and catching fatals is a pretty
common thing. If EngineException does not get caught, it should produce an
E_FATAL again with the correct error code.

Exceptions already do this, do they not? I don’t see why those exceptions would be any different.

Andrea Faulds
http://ajf.me/

11 years ago by Ferenc Kovacs — view source — reply

unread

Depends, registering for shutdown handlers and catching fatals is a
pretty
common thing. If EngineException does not get caught, it should produce
an
E_FATAL again with the correct error code.

Exceptions already do this, do they not? I don’t see why those exceptions
would be any different.

people have code out there like this:
try {
//something stupid
} catch (Exception $e) {
//do nothing or maybe log it
}

if we turn fatal errors into Exceptions extending the base Exception class,
these Pokémon constructs ("Gotta Catch 'Em All") will will swallow those
errors and contrinue the execution, maybe causing more dangerous
results(data corruption, etc.) than a simple fatal error could.
But these issues was discussed before, so I'm fairly sure that Nikita is
aware of those concerns, and let's just wait for him to update his proposal
and we can discuss it after it's ready.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

11 years ago by Matt Ficken — view source — reply

unread

I get no new test failures on master after AST merged.

Before and after the AST merge, I still get ~200 tests failing on Windows.

My test runs turned up a few crashes, which I am reproducing to get BTs to
share. I'm not sure if the crashes are AST related.

Depends, registering for shutdown handlers and catching fatals is a
pretty
common thing. If EngineException does not get caught, it should produce
an
E_FATAL again with the correct error code.

Exceptions already do this, do they not? I don’t see why those exceptions
would be any different.

people have code out there like this:
try {
//something stupid
} catch (Exception $e) {
//do nothing or maybe log it
}

if we turn fatal errors into Exceptions extending the base Exception class,
these Pokémon constructs ("Gotta Catch 'Em All") will will swallow those
errors and contrinue the execution, maybe causing more dangerous
results(data corruption, etc.) than a simple fatal error could.
But these issues was discussed before, so I'm fairly sure that Nikita is
aware of those concerns, and let's just wait for him to update his proposal
and we can discuss it after it's ready.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

11 years ago by Nikita Popov — view source — reply

unread

On Mon, Aug 25, 2014 at 11:39 PM, Stas Malyshev smalyshev@sugarcrm.com
wrote:

Hi!

I don't know if I will be implementing that ext myself. In any case,
before
that can happen I will have to create another RFC for converting parse
errors into exceptions and making sure we don't leak memory on failed
parse

If we're talking the plunge of putting exceptions into the core engine,
wouldn't it make sense to convert most of the fatal errors (especially
the "catchable" ones) to exceptions too? Most of them (except for out of
memory, timeouts and such) don't really require the engine to shut down,
as it seems... So if we're already saying people will have to deal with
exceptions in PHP engine anyway, maybe we could make it so that we could
have unified handling of them?

I was planning to deal with parse exceptions separately because they
require special handling and larger changes to the parser. However I could
also apply those changes independently, as they are also useful for
avoiding leaks on failed eval()s. The actual exception can then be
introduced together with other exceptions for fatal errors.

Yeah, I think that's what I'm going to do, thanks for pointing this out :)

Nikita

PS: The AST branch is now merged into master.

10 years ago by Sebastian Bergmann — view source — reply

unread

Am 25.08.2014 um 19:39 schrieb Nikita Popov:

I don't know if I will be implementing that ext myself. In any case, before
that can happen I will have to create another RFC for converting parse
errors into exceptions and making sure we don't leak memory on failed parse

I think thats a prerequisite for exposing the parser to userland.

Do you have an update on this? Just making sure this will not be
forgotten. IMHO it would be a huge boost for the PHP tools ecosystem
if PHP 7 would expose the AST to userland.

10 years ago by Ferenc Kovacs — view source — reply

unread

2015.01.11. 7:55 ezt írta ("Sebastian Bergmann" sebastian@php.net):

Am 25.08.2014 um 19:39 schrieb Nikita Popov:

I don't know if I will be implementing that ext myself. In any case,
before
that can happen I will have to create another RFC for converting parse
errors into exceptions and making sure we don't leak memory on failed
parse

I think thats a prerequisite for exposing the parser to userland.

Do you have an update on this? Just making sure this will not be
forgotten. IMHO it would be a huge boost for the PHP tools ecosystem
if PHP 7 would expose the AST to userland.

--

There is https://github.com/nikic/php-ast

10 years ago by Sebastian Bergmann — view source — reply

unread

Am 11.01.2015 um 09:03 schrieb Ferenc Kovacs:

There is https://github.com/nikic/php-ast

Aha! Thanks for the pointer, Ferenc. Can we get this bundled with
PHP 7 (and enabled by default)?