Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-25 Thread Andrea Faulds

Hi,

Xinchen Hui wrote:

Hey:

On Wed, Nov 25, 2015 at 5:57 PM, Nikita Popov  wrote:



Imho this additional change is not necessary, it only makes the parser
more complicated.

However something missing from the original patch is handling of relative
names like namespace\int. Instead of checking for ast->attr == ZEND_NAME_FQ
it should check for ast->attr != ZEND_NAME_NOT_FQ.



PS: However, namespace\int will result error while checking valid
classname, as int is reserved keywords.

so I think check for == ZEND_NAME_FQ is enough here.



This is how I feel as well. You can't make a class with that name anyway 
(at least for now), so we don't need to prohibit it. \int was a problem 
because it was interpreted the same as 'int'.


Thanks.

--
Andrea Faulds
http://ajf.me/

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



Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-25 Thread Nikita Popov
On Wed, Nov 25, 2015 at 4:47 AM, Xinchen Hui  wrote:

> Hey:
>
>
>
> On Wed, Nov 25, 2015 at 4:49 AM, Bob Weinand  wrote:
>
> > > Am 24.11.2015 um 20:30 schrieb Matteo Beccati :
> > >
> > > On 24/11/2015 18:50, Andrea Faulds wrote:
> > >> There's no syntax change. We'd be adding another fatal error to
> > >> zend_compile.c triggered by a flag on the token. No messing around
> with
> > >> the parser.
> > >>
> > >> I understand your concern about the risk, but it's the kind of change
> > >> that wouldn't break anything without it being tremendously obvious.
> > >
> > > I agree and we should be still in time for RC8.
> > >
> > >
> > > Cheers
> > > --
> > > Matteo Beccati
> >
> > Hey,
> >
> > I fixed the issue via
> >
> http://git.php.net/?p=php-src.git;a=commitdiff;h=569763cb1ac67f56e7743062ca8b3b7c650c1254
> >
> > I think too this should go into PHP 7.0.0 as it is some type of a
> language
> > related change (even if not directly failing in parser...)
> >
> >
> I've made a improvement to the fix(
>
> https://github.com/php/php-src/commit/00865ae22f2c5fdee9e500ce79d442467e0a0899
> )
> ,
>
> before this, \array will result a syntax , but \int result a compiler
> error, which seems a little in-consistent.
>

Imho this additional change is not necessary, it only makes the parser more
complicated.

However something missing from the original patch is handling of relative
names like namespace\int. Instead of checking for ast->attr == ZEND_NAME_FQ
it should check for ast->attr != ZEND_NAME_NOT_FQ.

Nikita


Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-25 Thread Xinchen Hui
Hey:

On Wed, Nov 25, 2015 at 5:57 PM, Nikita Popov  wrote:

> On Wed, Nov 25, 2015 at 4:47 AM, Xinchen Hui  wrote:
>
>> Hey:
>>
>>
>>
>> On Wed, Nov 25, 2015 at 4:49 AM, Bob Weinand  wrote:
>>
>> > > Am 24.11.2015 um 20:30 schrieb Matteo Beccati :
>> > >
>> > > On 24/11/2015 18:50, Andrea Faulds wrote:
>> > >> There's no syntax change. We'd be adding another fatal error to
>> > >> zend_compile.c triggered by a flag on the token. No messing around
>> with
>> > >> the parser.
>> > >>
>> > >> I understand your concern about the risk, but it's the kind of change
>> > >> that wouldn't break anything without it being tremendously obvious.
>> > >
>> > > I agree and we should be still in time for RC8.
>> > >
>> > >
>> > > Cheers
>> > > --
>> > > Matteo Beccati
>> >
>> > Hey,
>> >
>> > I fixed the issue via
>> >
>> http://git.php.net/?p=php-src.git;a=commitdiff;h=569763cb1ac67f56e7743062ca8b3b7c650c1254
>> >
>> > I think too this should go into PHP 7.0.0 as it is some type of a
>> language
>> > related change (even if not directly failing in parser...)
>> >
>> >
>> I've made a improvement to the fix(
>>
>> https://github.com/php/php-src/commit/00865ae22f2c5fdee9e500ce79d442467e0a0899
>> )
>> ,
>>
>> before this, \array will result a syntax , but \int result a compiler
>> error, which seems a little in-consistent.
>>
>
> Imho this additional change is not necessary, it only makes the parser
> more complicated.
>
> However something missing from the original patch is handling of relative
> names like namespace\int. Instead of checking for ast->attr == ZEND_NAME_FQ
> it should check for ast->attr != ZEND_NAME_NOT_FQ.
>

PS: However, namespace\int will result error while checking valid
classname, as int is reserved keywords.

so I think check for == ZEND_NAME_FQ is enough here.

thanks

>
>

>
> Nikita
>



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


Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-25 Thread Xinchen Hui
Hey:


On Wed, Nov 25, 2015 at 5:57 PM, Nikita Popov  wrote:

> On Wed, Nov 25, 2015 at 4:47 AM, Xinchen Hui  wrote:
>
>> Hey:
>>
>>
>>
>> On Wed, Nov 25, 2015 at 4:49 AM, Bob Weinand  wrote:
>>
>> > > Am 24.11.2015 um 20:30 schrieb Matteo Beccati :
>> > >
>> > > On 24/11/2015 18:50, Andrea Faulds wrote:
>> > >> There's no syntax change. We'd be adding another fatal error to
>> > >> zend_compile.c triggered by a flag on the token. No messing around
>> with
>> > >> the parser.
>> > >>
>> > >> I understand your concern about the risk, but it's the kind of change
>> > >> that wouldn't break anything without it being tremendously obvious.
>> > >
>> > > I agree and we should be still in time for RC8.
>> > >
>> > >
>> > > Cheers
>> > > --
>> > > Matteo Beccati
>> >
>> > Hey,
>> >
>> > I fixed the issue via
>> >
>> http://git.php.net/?p=php-src.git;a=commitdiff;h=569763cb1ac67f56e7743062ca8b3b7c650c1254
>> >
>> > I think too this should go into PHP 7.0.0 as it is some type of a
>> language
>> > related change (even if not directly failing in parser...)
>> >
>> >
>> I've made a improvement to the fix(
>>
>> https://github.com/php/php-src/commit/00865ae22f2c5fdee9e500ce79d442467e0a0899
>> )
>> ,
>>
>> before this, \array will result a syntax , but \int result a compiler
>> error, which seems a little in-consistent.
>>
>
> Imho this additional change is not necessary, it only makes the parser
> more complicated.
>
> However something missing from the original patch is handling of relative
> names like namespace\int. Instead of checking for ast->attr == ZEND_NAME_FQ
> it should check for ast->attr != ZEND_NAME_NOT_FQ.
>
> yeah, you are right,  namespace\array still behavior difference from
namespace\int.

I am going to revert my part.

thanks

> Nikita
>



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


Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Bob Weinand
> Am 24.11.2015 um 20:30 schrieb Matteo Beccati :
> 
> On 24/11/2015 18:50, Andrea Faulds wrote:
>> There's no syntax change. We'd be adding another fatal error to
>> zend_compile.c triggered by a flag on the token. No messing around with
>> the parser.
>> 
>> I understand your concern about the risk, but it's the kind of change
>> that wouldn't break anything without it being tremendously obvious.
> 
> I agree and we should be still in time for RC8.
> 
> 
> Cheers
> -- 
> Matteo Beccati

Hey,

I fixed the issue via 
http://git.php.net/?p=php-src.git;a=commitdiff;h=569763cb1ac67f56e7743062ca8b3b7c650c1254

I think too this should go into PHP 7.0.0 as it is some type of a language 
related change (even if not directly failing in parser...)

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



Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Anthony Ferrara
Sebastian,

On Tue, Nov 24, 2015 at 10:10 AM, Sebastian Bergmann  wrote:
>  The following is currently valid PHP 7 code
>
>function a(\int $i) {}
>
>  Is it intentional that the \ in front of the "int" is allowed? IMHO, this
>  confusing notation must not be allowed.

Yeah, that is a problem. We should fix that prior to gold. I'll take a
peak at it when I get some time, but if someone else has a few minutes
and wants to tackle it go for it.

Anthony

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



Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Xinchen Hui
Hey:



On Wed, Nov 25, 2015 at 4:49 AM, Bob Weinand  wrote:

> > Am 24.11.2015 um 20:30 schrieb Matteo Beccati :
> >
> > On 24/11/2015 18:50, Andrea Faulds wrote:
> >> There's no syntax change. We'd be adding another fatal error to
> >> zend_compile.c triggered by a flag on the token. No messing around with
> >> the parser.
> >>
> >> I understand your concern about the risk, but it's the kind of change
> >> that wouldn't break anything without it being tremendously obvious.
> >
> > I agree and we should be still in time for RC8.
> >
> >
> > Cheers
> > --
> > Matteo Beccati
>
> Hey,
>
> I fixed the issue via
> http://git.php.net/?p=php-src.git;a=commitdiff;h=569763cb1ac67f56e7743062ca8b3b7c650c1254
>
> I think too this should go into PHP 7.0.0 as it is some type of a language
> related change (even if not directly failing in parser...)
>
>
I've made a improvement to the fix(
https://github.com/php/php-src/commit/00865ae22f2c5fdee9e500ce79d442467e0a0899)
,

before this, \array will result a syntax , but \int result a compiler
error, which seems a little in-consistent.

thanks

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



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


[PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Sebastian Bergmann

 The following is currently valid PHP 7 code

 http://www.php.net/unsub.php



Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Ryan Pallas
On Tue, Nov 24, 2015 at 8:10 AM, Sebastian Bergmann 
wrote:

>  The following is currently valid PHP 7 code
>
>function a(\int $i) {}
>
>  Is it intentional that the \ in front of the "int" is allowed? IMHO, this
>  confusing notation must not be allowed.
>
>
> Why not? Its a root level type, you can prefix a \ on any other root level
type that's valid for use in type hinting.

function a(\DateTime $d) {}
function b(\SplFileObject $f) {}

Also, this is the only way to get some IDEs to recognize the type when in a
namespace (at least currently).


RE: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Theodore Brown
> On Tue, Nov 24, 2015 at 8:10 AM, Sebastian Bergmann 
> wrote:
>
>> The following is currently valid PHP 7 code
>>
>> > function a(\int $i) {}
>>
>> Is it intentional that the \ in front of the "int" is allowed? IMHO, this
>> confusing notation must not be allowed.
>>
>>
>> Why not? Its a root level type, you can prefix a \ on any other root level
> type that's valid for use in type hinting.
>
> function a(\DateTime $d) {}
> function b(\SplFileObject $f) {}
>
> Also, this is the only way to get some IDEs to recognize the type when in a
> namespace (at least currently).

The difference is that DateTime and \DateTime mean different things inside a 
namespace. int and \int always mean the same thing, so it seems confusing to 
allow the latter syntax as if it means something different.
  
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Sebastian Bergmann

On 11/24/2015 04:27 PM, Ryan Pallas wrote:

Also, this is the only way to get some IDEs to recognize the type when in a
namespace (at least currently).


 Then the IDEs have to be fixed.

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



Re: [PHP-DEV] Scalar Type Declaration Syntax Weirdness

2015-11-24 Thread Sebastian Bergmann

On 11/24/2015 04:42 PM, Theodore Brown wrote:

The difference is that DateTime and \DateTime mean different things inside a 
namespace.
int and \int always mean the same thing, so it seems confusing to allow the 
latter syntax
as if it means something different.


 That. And the fact that DateTime, for instance, is a built-in class
 whereas int is a type (I know that classes are types but you know what I
 mean).

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