Page MenuHomePhabricator

Add support in XHPAST for "(new X)->y" syntax (added in 5.4)
ClosedPublic

Authored by Firehed on Feb 5 2014, 8:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 11:43 PM
Unknown Object (File)
Sun, Nov 17, 4:57 PM
Unknown Object (File)
Thu, Nov 14, 4:59 AM
Unknown Object (File)
Mon, Nov 11, 12:10 AM
Unknown Object (File)
Sat, Nov 9, 4:31 PM
Unknown Object (File)
Sat, Nov 2, 6:58 AM
Unknown Object (File)
Tue, Oct 29, 2:13 PM
Unknown Object (File)
Mon, Oct 28, 2:18 PM

Details

Summary

Ref T4334. Allows XHPAST to parse the access-on-instanciation grammar added in 5.4 Refactors some of the other grammar to be closer to Zend's parser. Adds positive and negative tests for allowed syntax.

Test Plan

arc unit
Ran generate_php_symbols.php script on files that previously errored out with "unexpected T_OBJECT_OPERATOR", now there are no errors

Diff Detail

Repository
rPHU libphutil
Branch
xhpast_access_instanciation
Lint
Lint Passed
Unit
Tests Passed

Event Timeline

One small thing inline -- does that make sense? The rest of this looks great to me. Thanks for digging in!

support/xhpast/parser.y
2311–2312

I think this should either look like the parenthesis_expr rule does, and generate a n_PARENTHETICAL_EXPRESSION node, or look like the yield_expr rule does and do NEXPAND($1, $2, $3). Since the parens are required, maybe the NEXPAND is more consistent.

Specifically, what NEXPAND does is "expand" the node to include more tokens. So the current parse will look like this, with these tokens as part of the node:

"(" "new" "a" ")"
    ^^^^^^^^^

If you NEXPAND it, you'll get this:

"(" "new" "a" ")"
^^^^^^^^^^^^^^^^^

This generally doesn't matter when working with the tree, but one practical effect is that getConcreteString() on the n_NEW node will print out "new a" in the former case, and "(new a)" in the latter case. Since the parentheses are required for the node to be valid, this seems like a sensible interpretation.

The alternative would add an n_PARENTHETICAL_EXPRESSION node, so we'd get this:

"(" "new" "a" ")"
    ^^^^^^^^^      n_NEW
^^^^^^^^^^^^^^^^^  n_PARENTHETICAL_EXPRESSION

This seems like a less expected result than calling the whole thing an n_NEW to me, though.

Firehed updated this revision to Unknown Object (????).Feb 5 2014, 9:29 PM
epriestley closed this revision.

Closed by commit rPHUe719f331aa1a (authored by @Firehed, committed by @epriestley).