Skip to content

Scan less during classification. #3530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CyrusNajmabadi merged 3 commits into from
Jun 18, 2015
Merged

Scan less during classification. #3530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3bb7be9
Scan less during classification.
CyrusNajmabadi Jun 16, 2015
d206f62
Squueze perf in syntactic classification.
CyrusNajmabadi Jun 17, 2015
151306f
PR feedback.
CyrusNajmabadi Jun 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/compiler/scanner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,31 @@ namespace ts {
return ch >= CharacterCodes._0 && ch <= CharacterCodes._7;
}

export function couldStartTrivia(text: string, pos: number): boolean {
// Keep in sync with skipTrivia
let ch = text.charCodeAt(pos);
switch (ch) {
case CharacterCodes.carriageReturn:
case CharacterCodes.lineFeed:
case CharacterCodes.tab:
case CharacterCodes.verticalTab:
case CharacterCodes.formFeed:
case CharacterCodes.space:
case CharacterCodes.slash:
// starts of normal trivia
case CharacterCodes.lessThan:
case CharacterCodes.equals:
case CharacterCodes.greaterThan:
// Starts of conflict marker trivia
return true;
default:
return ch > CharacterCodes.maxAsciiCharacter;
}
}

/* @internal */
export function skipTrivia(text: string, pos: number, stopAfterLineBreak?: boolean): number {
// Keep in sync with couldStartTrivia
while (true) {
let ch = text.charCodeAt(pos);
switch (ch) {
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,12 @@ namespace ts {
return start <= textSpanEnd(span) && end >= span.start;
}

export function decodedTextSpanIntersectsWith(start1: number, length1: number, start2: number, length2: number) {
let end1 = start1 + length1;
let end2 = start2 + length2;
return start2 <= end1 && end2 >= start1;

@DanielRosenwasser DanielRosenwasser Jun 17, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep the comparison directions consistent? It reads a bit easier.

@CyrusNajmabadi CyrusNajmabadi Jun 18, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrmm. i could. but that actually seems harder to read. With the comparisons now what we're doing is always comparing the second span to the first span. If i flip so that it's two <= comparisons, then we'll be first comparing the second span to the first, and then the first span to the second.

@DanielRosenwasser DanielRosenwasser Jun 18, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I see the gain in nominal over relational consistency. Maybe just leave a comment like

// Covers
// (1)
//        s1-----------e1
//              s2-----------e2
//
// (2)
//              s1-----------e1
//        s2-----------e2
//
// (3)
//        s1-----------e1
//            s2---e2
//
// (4)
//            s1---e1
//        s2-----------e2

or if you don't see huge utility in it, don't.

Also this might be better called "overlaps with" instead of "intersects with"

}

@CyrusNajmabadi CyrusNajmabadi Jun 18, 2015

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Overlaps" has very different semantics from "Intersects" from the perspective of the Editor and how they use that terminology. This is following the editor semantics for 'intersects', so i'd like to keep using that name.


export function textSpanIntersectsWithPosition(span: TextSpan, position: number) {
return position <= textSpanEnd(span) && position >= span.start;
}
Expand Down
1 change: 0 additions & 1 deletion src/harness/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ if (testConfigFile !== '') {
if (!option) {
continue;
}
ts.sys.write("Option: " + option + "\r\n");
switch (option) {
case 'compiler':
runners.push(new CompilerBaselineRunner(CompilerTestType.Conformance));
Expand Down
78 changes: 45 additions & 33 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ namespace ts {
}

public getFullWidth(): number {
return this.end - this.getFullStart();
return this.end - this.pos;
}

public getLeadingTriviaWidth(sourceFile?: SourceFile): number {
Expand Down Expand Up @@ -6116,6 +6116,8 @@ namespace ts {
function getEncodedSyntacticClassifications(fileName: string, span: TextSpan): Classifications {
// doesn't use compiler - no need to synchronize with host
let sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName);
let spanStart = span.start;
let spanLength = span.length;

// Make a scanner we can get trivia from.
let triviaScanner = createScanner(ScriptTarget.Latest, /*skipTrivia:*/ false, sourceFile.text);
Expand All @@ -6132,48 +6134,55 @@ namespace ts {
result.push(type);
}

function classifyLeadingTrivia(token: Node): void {
let tokenStart = skipTrivia(sourceFile.text, token.pos, /*stopAfterLineBreak:*/ false);
if (tokenStart === token.pos) {
return;
}

// token has trivia. Classify them appropriately.
function classifyLeadingTriviaAndGetTokenStart(token: Node): number {
triviaScanner.setTextPos(token.pos);
while (true) {
let start = triviaScanner.getTextPos();
// only bother scanning if we have something that could be trivia.
if (!couldStartTrivia(sourceFile.text, start)) {
return start;
}

let kind = triviaScanner.scan();
let end = triviaScanner.getTextPos();
let width = end - start;

// The moment we get something that isn't trivia, then stop processing.
if (!isTrivia(kind)) {
return;
return start;
}

// Only bother with the trivia if it at least intersects the span of interest.
if (textSpanIntersectsWith(span, start, width)) {
if (isComment(kind)) {
classifyComment(token, kind, start, width);
continue;
}
// Don't bother with newlines/whitespace.
if (kind === SyntaxKind.NewLineTrivia || kind === SyntaxKind.WhitespaceTrivia) {
continue;
}

if (kind === SyntaxKind.ConflictMarkerTrivia) {
let text = sourceFile.text;
let ch = text.charCodeAt(start);
// Only bother with the trivia if it at least intersects the span of interest.
if (isComment(kind)) {
classifyComment(token, kind, start, width);

// Classifying a comment might cause us to reuse the trivia scanner
// (because of jsdoc comments). So after we classify the comment make
// sure we set the scanner position back to where it needs to be.
triviaScanner.setTextPos(end);
continue;
}

// for the <<<<<<< and >>>>>>> markers, we just add them in as comments
// in the classification stream.
if (ch === CharacterCodes.lessThan || ch === CharacterCodes.greaterThan) {
pushClassification(start, width, ClassificationType.comment);
continue;
}
if (kind === SyntaxKind.ConflictMarkerTrivia) {
let text = sourceFile.text;
let ch = text.charCodeAt(start);

// for the ======== add a comment for the first line, and then lex all
// subsequent lines up until the end of the conflict marker.
Debug.assert(ch === CharacterCodes.equals);
classifyDisabledMergeCode(text, start, end);
// for the <<<<<<< and >>>>>>> markers, we just add them in as comments
// in the classification stream.
if (ch === CharacterCodes.lessThan || ch === CharacterCodes.greaterThan) {
pushClassification(start, width, ClassificationType.comment);
continue;
}

// for the ======== add a comment for the first line, and then lex all
// subsequent lines up until the end of the conflict marker.
Debug.assert(ch === CharacterCodes.equals);
classifyDisabledMergeCode(text, start, end);
}
}
}
Expand Down Expand Up @@ -6292,12 +6301,14 @@ namespace ts {
}

function classifyToken(token: Node): void {
classifyLeadingTrivia(token);
let tokenStart = classifyLeadingTriviaAndGetTokenStart(token);

if (token.getWidth() > 0) {
let tokenWidth = token.end - tokenStart;
Debug.assert(tokenWidth >= 0);
if (tokenWidth > 0) {
let type = classifyTokenType(token.kind, token);
if (type) {
pushClassification(token.getStart(), token.getWidth(), type);
pushClassification(tokenStart, tokenWidth, type);
}
}
}
Expand Down Expand Up @@ -6402,9 +6413,10 @@ namespace ts {
}

// Ignore nodes that don't intersect the original span to classify.
if (textSpanIntersectsWith(span, element.getFullStart(), element.getFullWidth())) {
if (decodedTextSpanIntersectsWith(spanStart, spanLength, element.pos, element.getFullWidth())) {
let children = element.getChildren(sourceFile);
for (let child of children) {
for (let i = 0, n = children.length; i < n; i++) {
let child = children[i];
if (isToken(child)) {
classifyToken(child);
}
Expand Down
1 change: 0 additions & 1 deletion tests/cases/fourslash/syntacticClassificationsDocComment3.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ verify.syntacticClassificationsAre(
c.punctuation("{"),
c.keyword("number"),
c.comment(" /* } */"),
c.comment("/* } */"),
c.keyword("var"),
c.text("v"),
c.punctuation(";"));