Everything

php

[VOTE] Abstract Syntax Tree

php.internals

Nikita Popov

12 years ago

Hi internals!

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

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

Thanks,
Nikita

Andi Gutmans

12 years ago

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

On Aug 18, 2014, at 9:41 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:

Dmitry Stogov

12 years ago

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.


On Tue, Aug 19, 2014 at 6:32 AM, Andi Gutmans <andi@zend.com> wrote:

Laruence

12 years ago

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

On Tue, Aug 19, 2014 at 2:36 PM, Dmitry Stogov <dmitry@zend.com> wrote:

> 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.
>
>
> On Tue, Aug 19, 2014 at 6:32 AM, Andi Gutmans <andi@zend.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?
>>
>> Any insight would be much appreciated.
>> Thanks!
>> Andi
>>
>> On Aug 18, 2014, at 9:41 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
>>
>> > Hi internals!
>> >
>> > I've opened the vote on the Abstract Syntax Tree RFC:
>> >
>> >    https://wiki.php.net/rfc/abstract_syntax_tree#vote
>> >
>> > Thanks,
>> > Nikita
>>
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>>

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

Laruence

12 years ago

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

On Tue, Aug 19, 2014 at 5:12 PM, Laruence <laruence@php.net> wrote:

> 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
>
> On Tue, Aug 19, 2014 at 2:36 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>> 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.
>>
>>
>> On Tue, Aug 19, 2014 at 6:32 AM, Andi Gutmans <andi@zend.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?
>>>
>>> Any insight would be much appreciated.
>>> Thanks!
>>> Andi
>>>
>>> On Aug 18, 2014, at 9:41 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
>>>
>>> > Hi internals!
>>> >
>>> > I've opened the vote on the Abstract Syntax Tree RFC:
>>> >
>>> >    https://wiki.php.net/rfc/abstract_syntax_tree#vote
>>> >
>>> > Thanks,
>>> > Nikita
>>>
>>>
>>> --
>>> PHP Internals - PHP Runtime Development Mailing List
>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>
>>>
>
>
>
> --
> Laruence  Xinchen Hui
> http://www.laruence.com/

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

Laruence

12 years ago

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

On Tue, Aug 19, 2014 at 5:21 PM, Laruence <laruence@php.net> wrote:

> 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
>
> On Tue, Aug 19, 2014 at 5:12 PM, Laruence <laruence@php.net> wrote:
>> 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
>>
>> On Tue, Aug 19, 2014 at 2:36 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>>> 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.
>>>
>>>
>>> On Tue, Aug 19, 2014 at 6:32 AM, Andi Gutmans <andi@zend.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?
>>>>
>>>> Any insight would be much appreciated.
>>>> Thanks!
>>>> Andi
>>>>
>>>> On Aug 18, 2014, at 9:41 AM, Nikita Popov <nikita.ppv@gmail.com> wrote:
>>>>
>>>> > Hi internals!
>>>> >
>>>> > I've opened the vote on the Abstract Syntax Tree RFC:
>>>> >
>>>> >    https://wiki.php.net/rfc/abstract_syntax_tree#vote
>>>> >
>>>> > Thanks,
>>>> > Nikita
>>>>
>>>>
>>>> --
>>>> PHP Internals - PHP Runtime Development Mailing List
>>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>>>
>>>>
>>
>>
>>
>> --
>> Laruence  Xinchen Hui
>> http://www.laruence.com/
>
>
>
> --
> Laruence  Xinchen Hui
> http://www.laruence.com/

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

Pierre Joye

12 years ago

On Tue, Aug 19, 2014 at 11:30 AM, Laruence <laruence@php.net> wrote:

> 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

Nikita Popov

12 years ago

On Tue, Aug 19, 2014 at 11:21 AM, Laruence <laruence@php.net> wrote:

> 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

Andi Gutmans

12 years ago

got it. thanks!

On Aug 18, 2014, at 11:36 PM, Dmitry Stogov <dmitry@zend.com> wrote:

Paul Dragoonis

12 years ago

On Tue, Aug 19, 2014 at 3:32 AM, Andi Gutmans <andi@zend.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. 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

Nikita Popov

12 years ago

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.

Nikita

Paul Dragoonis

12 years ago

On Wed, Aug 20, 2014 at 2:57 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> 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.

Levi Morrison

12 years ago

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

> On Tue, Aug 19, 2014 at 3:32 AM, Andi Gutmans <andi@zend.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.

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

Paul Dragoonis

12 years ago

On Wed, Aug 20, 2014 at 3:41 PM, Levi Morrison <levim@php.net> wrote:

> On Wed, Aug 20, 2014 at 7:30 AM, Paul Dragoonis <dragoonis@gmail.com>
> wrote:
> > On Tue, Aug 19, 2014 at 3:32 AM, Andi Gutmans <andi@zend.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.

Pascal MARTIN

12 years ago

On 18/08/2014 18:41, Nikita Popov wrote:

>
> 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

Nikita Popov

12 years ago

On Mon, Aug 18, 2014 at 6:41 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> 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

Sebastian Bergmann

12 years ago

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

Nikita Popov

12 years ago

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

Stas Malyshev

12 years ago

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/

Trevor Suarez

12 years ago

>
> 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

Stas Malyshev

12 years ago

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/

Benjamin Eberlei

12 years ago

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.

Andrew Faulds

12 years ago

On 26 Aug 2014, at 00:53, Benjamin Eberlei <kontakt@beberlei.de> wrote:

> 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/

Ferenc Kovacs

12 years ago

On Tue, Aug 26, 2014 at 1:58 AM, Andrea Faulds <ajf@ajf.me> wrote:

>
> On 26 Aug 2014, at 00:53, Benjamin Eberlei <kontakt@beberlei.de> wrote:
>
> > 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

Nikita Popov

12 years ago

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.

Sebastian Bergmann

11 years ago

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.

Ferenc Kovacs

11 years ago

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.
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>

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

Sebastian Bergmann

11 years ago

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)?