[RFC][VOTING] Context Sensitive Lexer

Am 04.03.2015 um 09:52 schrieb Nikita Popov:

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

I trust Nikita's expertise and have removed my +1 vote. I really want
to have what this RFC aims for but not at the price of robustness
(regressions) and flexibility (impediment for future feature
development).

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

I had originally assumed that if this proposal is limited to method names
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 = 42;

but also

const FOO = 42, BAR = 24, BAZ = 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 = 42, BAR = [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 visibility
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 bunch
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 use
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 manner.

Nikita

Despite the problems you pointed out are already fixed I understand your
position. In fact, the implementation must be better than this and I'm
already working on a better implementation regardless of this voting
passing or not.

I've been thinking about dropping this voting since yesterday but, seeing
the bright side, at least the current polling showed that most people want
the feature and the ones that voted "no" did it because of the
implementation and this is already enough information for me to continue
working to find other alternatives :)

I'll try to have a better patch, using lexing feedback instead, until the
end of the voting (in 11 days) so at least we can discuss it for PHP 7.1
instead.

To all:

Some people are voting on the feature without taking the implementation
itself in consideration, and it's ok if voters are doing that because not
everyone is able to evaluate the implementation. So I'm also thinking about
voting only for the feature for now.

If the implementation becomes good enough - according to internals - in
time for PHP7 we could merge it, otherwise we postpone it to PHP 7.1 or
until we get an implementation that is good enough. In that case the
feature could be approved and the implementation evaluated later only by
the ones who feel capable to.

What do you think about that? I don't know how this could fit (if it fits)
on the RFC process, so opinions are welcome. If this idea doesn't fit well
with the current RFC process, then I'll do another proposal targeting PHP
7.1 anyway.

Thanks,
Márcio