Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108763
MIME-Version: 1.0
References: <CAF+90c9TWvj9Podt3uCdf43SzwweLfjn1H3DbWnt+pSE5UKYSw@mail.gmail.com>
 <CAF+90c_MShaa_-8Jtor6V6e8fJRfwxBLH67gJgmfA8BrqpiNWA@mail.gmail.com> <BYAPR05MB65350407CBA40FD65E34003DDEED0@BYAPR05MB6535.namprd05.prod.outlook.com>
In-Reply-To: <BYAPR05MB65350407CBA40FD65E34003DDEED0@BYAPR05MB6535.namprd05.prod.outlook.com>
Date: Tue, 25 Feb 2020 22:13:54 +0100
Message-ID: <CAF+90c_WwhNppa3MjZ=vQKReU8FqEkpuBpuXHCiRbA7GJQdPNg@mail.gmail.com>
To: Theodore Brown <theodorejb@outlook.com>
Cc: php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000019af6e059f6cf840"
Subject: Re: [PHP-DEV] Re: [RFC] token_get_all() TOKEN_AS_OBJECT mode
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000019af6e059f6cf840
Content-Type: text/plain; charset="UTF-8"

On Tue, Feb 25, 2020 at 7:16 PM Theodore Brown <theodorejb@outlook.com>
wrote:

> On Tue, Feb 25, 2020 at 9:59 AM Nikita Popov <nikita.ppv@gmail.com> wrote:
>
> > On Thu, Feb 13, 2020 at 10:47 AM Nikita Popov <nikita.ppv@gmail.com>
> wrote:
> >
> > > This has been discussed a while ago already, now as a proper proposal:
> > > https://wiki.php.net/rfc/token_as_object
> > >
> > > An open question is whether (at least to start with) PhpToken should be
> > > just a data container, or whether we want to add some helper methods
> to it.
> > > If this generates too much bikeshed, I'll drop methods from the
> proposal.
> >
> > I think this proposal is in a pretty decent shape now, and I'd like to
> move
> > it to voting soon. The only remaining open question is whether we want to
> > add any additional predefined methods. As the class can now be extended,
> > every library can add their own methods, but there might still be value
> in
> > providing some things by default, primarily for performance reason. For
> > example, the proposed is() method can be a good bit more efficient when
> > implemented directly in extension code.
> >
> > Any feedback on this point?
>
> Hi Nikita,
>
> Thanks for this RFC. The proposed `PhpToken` class will definitely make it
> easier to work with parsed tokens (I struggled with this when working on
> the migration script for deprecated alternate array/string offset syntax).
>
> In regards to the open question of additional methods, the `getTokenName()`
> method would be very welcome. `isIgnorable()` would also be helpful
> sometimes, though I don't care much one way or another if it's added.
>
> But I'm really skeptical about the value of the `is($kind)` method. Code
> working with tokens should know the type of value being checked (ID int,
> text content, or array of one or the other), and a method that accepts
> any of three types can make it harder to understand what's going on.
>
> For example, the proposed `isIgnorable()` method doesn't need to use
> `is()`, it could just check whether the `id` property is in the array
> of token constants.
>

To provide some context, a method like is() is primarily useful in the
implementation of other methods accepting a token-descriptor to search for.
Token stream implementation commonly have many helper methods along the
lines of

public function findRight($pos, $findTokenType) {
    $tokens = $this->tokens;
    for ($count = \count($tokens); $pos < $count; $pos++) {
        if ($tokens[$pos]->is($findTokenType)) {
            return $pos;
        }
    }
    return -1;
}

which search for tokens in different directions, skip tokens, check that
tokens exist while skipping whitespace etc. Having a fast primitive to
perform the token type check would be quite useful for that, I think.

Regards,
Nikita

--00000000000019af6e059f6cf840--