Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:84455
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.53 as permitted sender)
MIME-Version: 1.0
Reply-To: marcio3w@gmail.com
In-Reply-To: <CAOsHV+u+pHiDcVjbaOpjcmjEm+9r1GRvprLm2oQwuYFprhXfiw@mail.gmail.com>
References: <CAOsHV+uuVvyg+gjLoNkn+DxqTS6C4K4T5cPGJArW=ERBFeyPBA@mail.gmail.com>
 <CAF+90c-hwVxCaVTX6_yt4LT5wEi5Xf-2J+aDROeW=8148YicMg@mail.gmail.com> <CAOsHV+u+pHiDcVjbaOpjcmjEm+9r1GRvprLm2oQwuYFprhXfiw@mail.gmail.com>
Date: Mon, 9 Mar 2015 02:47:47 -0300
Message-ID: <CAOsHV+sF67RkS7p9HHrMdaDxRrp_m00mMBivwvDeJ8htOjKUtg@mail.gmail.com>
To: Marcio Almada <marcio3w@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11340c68238e250510d4944c
Subject: Re: [PHP-DEV][RFC][VOTING] Context Sensitive Lexer
From: marcio.web2@gmail.com (Marcio Almada)

--001a11340c68238e250510d4944c
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Hi,

Just passing by to announce I already have a working version of the new
patch: https://github.com/php/php-src/pull/1158

The patch is 100% compatible with the proposed one with the advantages:

   - Has no regression or forward compatibility risks and is highly
   predictable
   - Has an very small footprint compared to the previous attempt involving
   a pure lexical approach
   - Is highly configurable, to make a word semi-reserved you only have to
   edit a single inclusive list in a parser rule.
   - Requires a single compile time check

More than ever, I'd like to advice voters to vote for the feature as the
new implementation is already on it's way. There still some work to be
done, please refer to the task list on the pull request to see what still
needs to be done.

It would be nice to have the new patch reviewed too.

Thanks,
M=C3=A1rcio


2015-03-04 14:29 GMT-03:00 Marcio Almada <marcio.web2@gmail.com>:

> Hi
>
> 2015-03-04 5:52 GMT-03:00 Nikita Popov <nikita.ppv@gmail.com>:
>
>>
>> After reviewing the implementation, I've decided to vote "no" on this RF=
C.
>>
>> I had originally assumed that if this proposal is limited to method name=
s
>> and class constants only the implementation should be pretty simple and
>> robust. However it turned out that there are a number of complications, =
the
>> two primary ones being:
>>
>> a) We allow to define multiple class constants in one declaration. I.e.
>> you can write not only
>>
>>     const FOO =3D 42;
>>
>> but also
>>
>>     const FOO =3D 42, BAR =3D 24, BAZ =3D 0;
>>
>> This is further complicated by the fact that we allow many types of
>> (static scalar) expressions to be used as values for constants, so a
>> declaration might actually look like
>>
>>     const FOO =3D 42, BAR =3D [24, BAZ];
>>
>> as well.
>>
>> b) The trait adaptations syntax allows using method names without a
>> generic prefix like "::" or "->" in a number of ways. E.g. in
>>
>>     use A { A::foo as bar; }
>>
>> foo and bar are method names. On the other hand in
>>
>>     use A { A::foo as public; }
>>
>> the "public" is not a method name, it is used for trait method visibilit=
y
>> aliasing. Trait aliasing (but not precedence!) adaptations also allow
>> non-absolute method references on the left hand side, so
>>
>>     use A { foo as bar; }
>>
>> without the leading A:: is possible as well.
>>
>> In order to support these kinds of things, the lexer has to track a bunc=
h
>> of state (e.g. to know if we're currently in a trait adaptation list as
>> opposed to a namespace or closure "use" list) and also make extensive us=
e
>> of lexer lookahead. We also have to accurately track nesting of braces,
>> which is easy to get wrong when considering details like string
>> interpolation syntax.
>>
>> After a number of issues was fixed by Marcio, the patch still contains a
>> few lexer rules that I don't really understand, but which seem to be
>> required to avoid regressions in some edge case.
>>
>> So, tl;dr: I think the patch is too risky. Even if we can make sure that
>> we've covered all the current edge-cases and don't regress anything, I'm
>> afraid that this will cause complications with future changes. This ends=
 up
>> replicating too many parsing aspects in the lexer in a rather ad-hoc man=
ner.
>>
>> Nikita
>>
>
>

--001a11340c68238e250510d4944c--