Skip to content

Commit 6e5215f

Browse files
committed
Re-enable recursion contexts now that we can rely on depth being a distinguisher
1 parent c690d09 commit 6e5215f

File tree

6 files changed

+57
-14
lines changed

6 files changed

+57
-14
lines changed

IncrementalParsing.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,8 @@ Tree fixup is actually the most expensive part of the parser data right now, and
5757
- We use the parselistener interface as an easy way to ensure we get to see entry/
5858
exit events at the right time. This turned out to be easier than handling
5959
recursion/left factoring through overriding the relevant parser interface pieces.
60-
- It is not possible to distinguish between recursive contexts because they only
61-
store the invoking state, not the state of the parser. That means it is not apparently
62-
possible to get the kind of unique key we need. As a result, we will not reuse recursive
63-
contexts (but will reuse pieces from below them).
60+
- Top level recursion contexts are now reused, but we won't reuse individaul recursion contexts yet.
61+
6462

6563
#### References
6664

src/IncrementalParser.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ export abstract class IncrementalParser extends Parser
8282
}
8383
// See if we have seen this state before at this starting point.
8484
let existingCtx = this.parseData.tryGetContext(
85+
parentCtx ? parentCtx.depth() + 1 : 1,
8586
this.state,
8687
ruleIndex,
8788
this._input.LT(1).tokenIndex,

src/IncrementalParserData.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,23 +242,37 @@ export class IncrementalParserData {
242242
/**
243243
* Try to see if we have existing context for this state, rule and token position that may be reused.
244244
*
245+
* @param depth Current rule depth
245246
* @param state Parser state number - currently ignored.
246247
* @param ruleIndex Rule number
247248
* @param tokenIndex Token index in the *new* token stream
248249
*/
249-
public tryGetContext(state: number, ruleIndex: number, tokenIndex: number) {
250-
return this.ruleStartMap.get(this.getKey(state, ruleIndex, tokenIndex));
250+
public tryGetContext(
251+
depth: number,
252+
state: number,
253+
ruleIndex: number,
254+
tokenIndex: number,
255+
) {
256+
return this.ruleStartMap.get(
257+
this.getKey(depth, state, ruleIndex, tokenIndex),
258+
);
251259
}
252260

253261
private getKeyFromContext(ctx: IncrementalParserRuleContext) {
254262
return this.getKey(
263+
ctx.depth(),
255264
ctx.invokingState,
256265
ctx.ruleIndex,
257266
ctx.start.tokenIndex,
258267
);
259268
}
260-
private getKey(state: number, rule: number, tokenindex: number) {
261-
return `${state},${rule},${tokenindex}`;
269+
private getKey(
270+
depth: number,
271+
state: number,
272+
rule: number,
273+
tokenindex: number,
274+
) {
275+
return `${depth},${rule},${tokenindex}`;
262276
}
263277
/**
264278
* Index a given parse tree and adjust the min/max ranges

src/IncrementalParserRuleContext.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,13 @@
55

66
import { ParserRuleContext } from "./ParserRuleContext";
77
import { Interval } from "./misc/Interval";
8+
import { RuleContext } from "./RuleContext";
89

910
export class IncrementalParserRuleContext extends ParserRuleContext {
11+
/* Avoid having to recompute depth on every single depth call */
12+
private cachedDepth: number | undefined = undefined;
13+
private cachedParent: RuleContext | undefined = undefined;
14+
1015
// This is an epoch number that can be used to tell which pieces were
1116
// modified during a given incremental parse. The incremental parser
1217
// adds the current epoch number to all rule contexts it creates.
@@ -40,4 +45,28 @@ export class IncrementalParserRuleContext extends ParserRuleContext {
4045
set minMaxTokenIndex(index: Interval) {
4146
this._minMaxTokenIndex = index;
4247
}
48+
49+
/**
50+
* Compute the depth of this context in the parse tree.
51+
*
52+
* @notes The incremental parser uses a caching implemntation.
53+
*
54+
*/
55+
public depth(): number {
56+
if (
57+
this.cachedParent !== undefined &&
58+
this.cachedParent === this._parent
59+
) {
60+
return this.cachedDepth as number;
61+
}
62+
let n = 1;
63+
if (this._parent) {
64+
let parentDepth = this._parent.depth();
65+
this.cachedParent = this._parent;
66+
this.cachedDepth = n = parentDepth + 1;
67+
} else {
68+
this.cachedDepth = n = 1;
69+
}
70+
return n;
71+
}
4372
}

test/tool/TestIncremental.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,9 @@ const JAVA_PROGRAM_2_EXPECTATIONS: XPathExpectation[] = [
174174
text: "publicstatic",
175175
xpathRule: "//modifiers",
176176
},
177-
// This requires making recursive contexts work
178-
/*{
177+
/* This requires reusing individual recursion contexts */
178+
/*
179+
{
179180
class: ExpressionContext,
180181
text: "System.out.println",
181182
xpathRule: "//statementExpression/expression/expression",
@@ -590,8 +591,9 @@ export class TestIncremental {
590591
for (let i = 0; i < JAVA_PROGRAM_2_EXPECTATIONS.length - 1; ++i) {
591592
JAVA_PROGRAM_2_EXPECTATIONS[i].epoch = startingEpoch + 1;
592593
}
593-
JAVA_PROGRAM_2_EXPECTATIONS[JAVA_PROGRAM_2_EXPECTATIONS.length - 1].epoch =
594-
startingEpoch + 2;
594+
JAVA_PROGRAM_2_EXPECTATIONS[
595+
JAVA_PROGRAM_2_EXPECTATIONS.length - 1
596+
].epoch = startingEpoch + 2;
595597
this.verifyXPathExpectations(
596598
parser,
597599
secondTree,

tool/resources/org/antlr/v4/tool/templates/codegen/TypeScript/TypeScript.stg

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -424,8 +424,7 @@ LeftRecursiveRuleFunction(currentRule,args,code,locals,ruleCtx,altLabelCtxs,
424424
// Check whether we need to execute this rule.
425425
let guardResult = this.guardRule(this._ctx as IncrementalParserRuleContext, <currentRule.startState>, <parser.name>.RULE_<currentRule.name>) as <currentRule.ctxType>;
426426
// If we found an existing context that is valid, return it.
427-
// NOTE: Recursive context reuse is currently disabled
428-
if (0 && guardResult) {
427+
if (guardResult) {
429428
this._input.seek(guardResult.stop!.tokenIndex + 1);
430429
return guardResult;
431430
}

0 commit comments

Comments
 (0)