Skip to content

Commit

Permalink
Fix #738 incorrect start line for traits
Browse files Browse the repository at this point in the history
Empty productions are supposed to be assigned the start attributes
of the lookahead token. Currently, this happens by assigning above
the current stack position when the token it read.

This fails in a situation where we first reduce an empty production
higher up in the stack, and then again reduce an empty production
lower in the stack, without consuming the lookahead token in the
meantime.

Fix this by moving the assignment into the reduction phase. We
also need to do this for error productions, which are effectively
empty.
  • Loading branch information
nikic committed Dec 20, 2020
1 parent 893a5bc commit d3d1ee4
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 45 deletions.
13 changes: 8 additions & 5 deletions lib/PhpParser/ParserAbstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,7 @@ protected function doParse() {
));
}

// This is necessary to assign some meaningful attributes to /* empty */ productions. They'll get
// the attributes of the next token, even though they don't contain it themselves.
$this->startAttributeStack[$stackPos+1] = $startAttributes;
$this->endAttributeStack[$stackPos+1] = $endAttributes;
// Allow productions to access the start attributes of the lookahead token.
$this->lookaheadStartAttributes = $startAttributes;

//$this->traceRead($symbol);
Expand Down Expand Up @@ -294,7 +291,8 @@ protected function doParse() {

/* Goto - shift nonterminal */
$lastEndAttributes = $this->endAttributeStack[$stackPos];
$stackPos -= $this->ruleToLength[$rule];
$ruleLength = $this->ruleToLength[$rule];
$stackPos -= $ruleLength;
$nonTerminal = $this->ruleToNonTerminal[$rule];
$idx = $this->gotoBase[$nonTerminal] + $stateStack[$stackPos];
if ($idx >= 0 && $idx < $this->gotoTableSize && $this->gotoCheck[$idx] === $nonTerminal) {
Expand All @@ -307,6 +305,10 @@ protected function doParse() {
$stateStack[$stackPos] = $state;
$this->semStack[$stackPos] = $this->semValue;
$this->endAttributeStack[$stackPos] = $lastEndAttributes;
if ($ruleLength === 0) {
// Empty productions use the start attributes of the lookahead token.
$this->startAttributeStack[$stackPos] = $this->lookaheadStartAttributes;
}
} else {
/* error */
switch ($this->errorState) {
Expand Down Expand Up @@ -340,6 +342,7 @@ protected function doParse() {

// We treat the error symbol as being empty, so we reset the end attributes
// to the end attributes of the last non-error symbol
$this->startAttributeStack[$stackPos] = $this->lookaheadStartAttributes;
$this->endAttributeStack[$stackPos] = $this->endAttributeStack[$stackPos - 1];
$this->endAttributes = $this->endAttributeStack[$stackPos - 1];
break;
Expand Down
3 changes: 0 additions & 3 deletions test/code/parser/errorHandling/recovery.test
Original file line number Diff line number Diff line change
Expand Up @@ -1490,9 +1490,6 @@ array(
)
)
)
comments: array(
0: /** @var ?string */
)
)
)
)
Expand Down
102 changes: 65 additions & 37 deletions test/code/parser/stmt/class/class_position.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,63 +2,91 @@ Class position
-----
<?php

class A {
}
if (1);

try {
} catch (Exception $e) {
}

class B {
}

?>
class C {}
-----
!!positions
array(
0: Stmt_Class[3:1 - 4:1](
0: Stmt_If[3:1 - 3:7](
cond: Scalar_LNumber[3:5 - 3:5](
value: 1
)
stmts: array(
)
elseifs: array(
)
else: null
)
1: Stmt_Class[5:1 - 5:10](
attrGroups: array(
)
flags: 0
name: Identifier[3:7 - 3:7](
name: A
name: Identifier[5:7 - 5:7](
name: C
)
extends: null
implements: array(
)
stmts: array(
)
)
1: Stmt_TryCatch[6:1 - 8:1](
)
-----
<?php

if (1);

trait X {}
-----
!!positions
array(
0: Stmt_If[3:1 - 3:7](
cond: Scalar_LNumber[3:5 - 3:5](
value: 1
)
stmts: array(
)
catches: array(
0: Stmt_Catch[7:3 - 8:1](
types: array(
0: Name[7:10 - 7:18](
parts: array(
0: Exception
)
)
)
var: Expr_Variable[7:20 - 7:21](
name: e
)
stmts: array(
)
)
)
finally: null
elseifs: array(
)
else: null
)
2: Stmt_Class[10:1 - 11:1](
1: Stmt_Trait[5:1 - 5:10](
attrGroups: array(
)
flags: 0
name: Identifier[10:7 - 10:7](
name: B
name: Identifier[5:7 - 5:7](
name: X
)
extends: null
implements: array(
stmts: array(
)
)
)
-----
<?php

if (1);

interface X {}
-----
!!positions
array(
0: Stmt_If[3:1 - 3:7](
cond: Scalar_LNumber[3:5 - 3:5](
value: 1
)
stmts: array(
)
elseifs: array(
)
else: null
)
1: Stmt_Interface[5:1 - 5:14](
attrGroups: array(
)
name: Identifier[5:11 - 5:11](
name: X
)
extends: array(
)
stmts: array(
)
Expand Down

0 comments on commit d3d1ee4

Please sign in to comment.