[libSyntax] Incremental Syntax Parsing #16340

ahoppen · 2018-05-03T15:50:18Z

This is a work in progress PR to add incremental syntax parsing to the Swift compiler. It accompanies the proposal here and discussion thread here.

harlanhaskins · 2018-05-03T18:43:00Z

include/swift/Syntax/RawSyntax.h

@@ -232,18 +232,24 @@ class RawSyntax final
      /// memory.
      unsigned ManualMemory : 1;
    };
-    enum { NumRawSyntaxBits = bitmax(NumSyntaxKindBits, 8) + 1 };
+    enum { NumRawSyntaxBits = bitmax(NumSyntaxKindBits, 8) + 1 + 1 };


Good catch!

harlanhaskins · 2018-05-07T17:13:53Z

lib/Parse/SyntaxParsingCache.cpp

@@ -15,6 +15,45 @@
 using namespace swift;
 using namespace swift::syntax;

+ArrayRef<TriviaPiece> getNextChildsLeadingTrivia(const Syntax *Node) {


Would be nice to make this getNextChild, because that's a traversal I've found very necessary.

Sounds good. I will keep it in mind when I refactor these methods to provide a (hopefully faster) cache based lookup.

ahoppen · 2018-05-15T19:22:52Z

@swift-ci Please smoke test

ahoppen · 2018-05-21T16:08:51Z

@swift-ci Please smoke test

ahoppen · 2018-05-21T17:02:41Z

@swift-ci Please smoke test

ahoppen · 2018-05-21T17:05:45Z

@swift-ci Please smoke test

ahoppen · 2018-05-21T18:40:08Z

@swift-ci Please smoke test

nkcsgexi · 2018-05-21T19:45:15Z

include/swift/Syntax/RawSyntax.h

@@ -434,6 +442,28 @@ class RawSyntax final
    return getLayout()[Index];
  }

+  /// Return the number of bytes this node takes when spelled out in the source
+  size_t getTextLength() {


can we re-use accumulateAbsolutePosition to calculate the length?

I'm using accumulateAbsolutePosition now to compute the length of a token. For all other nodes I'm still computing the value while constructing the tree because it is faster.

nkcsgexi

Looking good overall! Some comments so far.

nkcsgexi · 2018-05-21T19:47:12Z

include/swift/Syntax/SyntaxKind.h.gyb

@@ -71,6 +71,8 @@ bool isTokenKind(SyntaxKind Kind);

 bool isUnknownKind(SyntaxKind Kind);

+bool shallBeOmittedWhenNoChildren(SyntaxKind Kind);


I presume only Parser can omit. Can we move the definition to Parser? leaving it in the node may mislead clients.

I didn't want to create a gyb file in the parser for this. It is currently still nice and gyb-free.

Maybe we could add this function to the swift namespace instead of the swift::syntax one and call it parserShallOmitIfNoChildern

yeah, it works too, as long as we make it explicit that it's a parser thing rather than for syntax nodes in general.

nkcsgexi · 2018-05-21T19:52:54Z

lib/Parse/SyntaxParsingCache.cpp

+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors


nitpick: 2018

nkcsgexi · 2018-05-21T20:12:41Z

include/swift/Parse/SyntaxParsingCache.h

+  llvm::Optional<Syntax> lookUp(size_t NewPosition, SyntaxKind Kind);
+
+  /// Turn recording of reused ranges on
+  void recordReuseInformation() { RecordReuseInformation = true; }


A better name is setRecordReuseInformation().

nkcsgexi · 2018-05-21T20:24:56Z

include/swift/Parse/SyntaxParsingContext.h

@@ -112,6 +113,9 @@ class alignas(1 << SyntaxAlignInBits) SyntaxParsingContext {
  // Reference to the
  SyntaxParsingContext *&CtxtHolder;

+  /// A cache of nodes that can be reused when creating the current syntax tree
+  SyntaxParsingCache *SyntaxParsingCache = nullptr;


We could move this instance to RootContextData. It will make it clear that all SyntaxParsingContext share a single cache entry. Same with Arena.

nkcsgexi · 2018-05-21T20:30:59Z

lib/Parse/SyntaxParsingContext.cpp

+    return 0;
+  }
+  // Disable this node and push the cached nodes into the storage
+  disable();


Instead of calling disable(), can we define another AccumulationMode called LoadedFromCache to be more specific? Probably, we can assert LoadedFromCache context doesn't spawn any child context.

ahoppen · 2018-05-21T22:55:44Z

@swift-ci Please smoke test

This greatly improves the ergonomics of writing tests and outweighs the ability to test which whitespaces get parsed since their parsing overhead should be minimal.

…parsed functions

…segments

ahoppen · 2018-05-22T16:03:29Z

@swift-ci Please smoke test

Creating a new CodeBlockItem meant that when doing an edit in the error nodes, the prefix gets reused and thus the code is parsed as invalid although it is not.

ahoppen · 2018-05-22T16:08:31Z

@swift-ci Please smoke test

harlanhaskins · 2018-05-22T17:40:32Z

🎉🎉🎉

harlanhaskins reviewed May 3, 2018

View reviewed changes

ahoppen force-pushed the incremental-syntax-parsing branch 4 times, most recently from 01d51a8 to b53baca Compare May 4, 2018 23:19

harlanhaskins reviewed May 7, 2018

View reviewed changes

ahoppen force-pushed the incremental-syntax-parsing branch 6 times, most recently from 95a5195 to 6c1434b Compare May 11, 2018 21:58

ahoppen mentioned this pull request May 15, 2018

[libSyntax] Syntax coloring using libSyntax #16636

Closed

ahoppen force-pushed the incremental-syntax-parsing branch from 6c1434b to 7a1b1ee Compare May 21, 2018 16:08

ahoppen force-pushed the incremental-syntax-parsing branch from 7a1b1ee to 59bfeb3 Compare May 21, 2018 17:01

ahoppen force-pushed the incremental-syntax-parsing branch from 59bfeb3 to a0776e6 Compare May 21, 2018 18:37

nkcsgexi reviewed May 21, 2018

View reviewed changes

jessesquires mentioned this pull request May 21, 2018

[110] Issue #110 - May 31, 2018 SwiftWeekly/swiftweekly.github.io#393

Closed

ahoppen force-pushed the incremental-syntax-parsing branch 2 times, most recently from 8040453 to 8cc0d25 Compare May 21, 2018 22:37

ahoppen changed the title ~~[WIP] Incremental Syntax Parsing~~ [libSyntax] Incremental Syntax Parsing May 21, 2018

ahoppen force-pushed the incremental-syntax-parsing branch from 8cc0d25 to e49f2b2 Compare May 21, 2018 22:54

nkcsgexi requested a review from rintaro May 21, 2018 23:38

ahoppen added 15 commits May 22, 2018 08:52

[incrParse] Add option to the test utility to print visual reuse info

723e2be

[incrParse] Test utility: Put swift-syntax-test args in quotes if needed

8044907

[incrParse] Add verification of reparsed regions to swift-syntax-test

d026b2d

[incrParse] Allow testing of multiple edits per line

6b3ac23

[incrParse] Add support for test cases that verify the reused regions

9bdc54a

[incrParse] Allow whitespaces to be reparsed in test

c31a880

This greatly improves the ergonomics of writing tests and outweighs the ability to test which whitespaces get parsed since their parsing overhead should be minimal.

[incrParse] Allow reuse of MemberDeclListItems

9d3233c

[incrParse] Fix parsing of nodes covering no source text

e1a99ef

[incrParse] Add some more tests

17c14a0

[libSyntax] Add a debug dump function to SyntaxParsingContext

9a3ff5b

[incrParse] Fix lexer offset issue when missing tokens get synthesized

2c02b1e

[incrParse] Minor improvements to the test utility

2fbb875

[libSyntax] Omit unknown nodes if they do not have any children

a137e0d

[incrParse] Fix swift-syntax-test not complaining about unexpected re…

b26dd11

…parsed functions

[libSyntax] Fix parsing of StringLiterals with invalid interpolation …

082086c

…segments

ahoppen force-pushed the incremental-syntax-parsing branch from 4eb19c6 to e2e8972 Compare May 22, 2018 16:02

ahoppen added 8 commits May 22, 2018 09:07

[incrParse] Reparse a node if the next leaf node has been modified

b2ebc96

[incrParse] Add error nodes to the end of a CodeBlockItem

1b3baf9

Creating a new CodeBlockItem meant that when doing an edit in the error nodes, the prefix gets reused and thus the code is parsed as invalid although it is not.

[libSyntax] Improve syntax related dump functions

15b2bae

[incrParse] Aesthetic improvements to the test utility

c733b5c

[libSyntax] Adjust tests for improved syntax parsing behaviour

bc52823

[libSyntax] Fix SwiftSyntax CodeBlockItemList test

a791a3d

[libSyntax] Rename recordReuseInformation to setRecordReuseInformation

2decf8f

[libSyntax] Store shared SyntaxParsingContext data in RootContextData

4e44e68

ahoppen force-pushed the incremental-syntax-parsing branch from e2e8972 to 4e44e68 Compare May 22, 2018 16:08

ahoppen merged commit 9d6ff6d into swiftlang:master May 22, 2018

ahoppen deleted the incremental-syntax-parsing branch June 11, 2018 21:20

		@@ -71,6 +71,8 @@ bool isTokenKind(SyntaxKind Kind);

		bool isUnknownKind(SyntaxKind Kind);

		bool shallBeOmittedWhenNoChildren(SyntaxKind Kind);

[libSyntax] Incremental Syntax Parsing #16340

[libSyntax] Incremental Syntax Parsing #16340

Uh oh!

Conversation

ahoppen commented May 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen commented May 15, 2018

Uh oh!

ahoppen commented May 21, 2018

Uh oh!

ahoppen commented May 21, 2018

Uh oh!

ahoppen commented May 21, 2018

Uh oh!

ahoppen commented May 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nkcsgexi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen commented May 21, 2018

Uh oh!

ahoppen commented May 22, 2018

Uh oh!

ahoppen commented May 22, 2018

Uh oh!

harlanhaskins commented May 22, 2018

Uh oh!

Uh oh!