Merge pull request #27789 from rintaro/syntaxparse-assertion-layout

rintaro · web-flow · commit fea97d2a137e · 2019-10-22T01:11:49.000+02:00
[SyntaxParse] Assert that the children of syntax tree nodes have contiguous ranges
diff --git a/include/swift/Parse/ParsedRawSyntaxNode.h b/include/swift/Parse/ParsedRawSyntaxNode.h
@@ -52,6 +52,7 @@ class ParsedRawSyntaxNode {
   };
   struct DeferredLayoutNode {
     MutableArrayRef<ParsedRawSyntaxNode> Children;
+    CharSourceRange Range;
   };
   struct DeferredTokenNode {
     const ParsedTriviaPiece *TriviaPieces;
@@ -72,9 +73,9 @@ class ParsedRawSyntaxNode {
   /// Primary used for capturing a deferred missing token.
   bool IsMissing = false;
 
-  ParsedRawSyntaxNode(syntax::SyntaxKind k,
+  ParsedRawSyntaxNode(syntax::SyntaxKind k, CharSourceRange r,
                       MutableArrayRef<ParsedRawSyntaxNode> deferredNodes)
-    : DeferredLayout({deferredNodes}),
+    : DeferredLayout({deferredNodes, r}),
       SynKind(uint16_t(k)), TokKind(uint16_t(tok::unknown)),
       DK(DataKind::DeferredLayout) {
     assert(getKind() == k && "Syntax kind with too large value!");
@@ -109,10 +110,12 @@ class ParsedRawSyntaxNode {
   }
 
   ParsedRawSyntaxNode(syntax::SyntaxKind k, tok tokKind,
-                      CharSourceRange r, OpaqueSyntaxNode n)
+                      CharSourceRange r, OpaqueSyntaxNode n,
+                      bool IsMissing = false)
     : RecordedData{n, r},
       SynKind(uint16_t(k)), TokKind(uint16_t(tokKind)),
-      DK(DataKind::Recorded) {
+      DK(DataKind::Recorded),
+      IsMissing(IsMissing) {
     assert(getKind() == k && "Syntax kind with too large value!");
     assert(getTokenKind() == tokKind && "Token kind with too large value!");
   }
@@ -209,9 +212,20 @@ class ParsedRawSyntaxNode {
     return copy;
   }
 
+  CharSourceRange getDeferredRange() const {
+    switch (DK) {
+    case DataKind::DeferredLayout:
+      return getDeferredLayoutRange();
+    case DataKind::DeferredToken:
+      return getDeferredTokenRangeWithTrivia();
+    default:
+      llvm_unreachable("node not deferred");
+    }
+  }
+
   // Recorded Data ===========================================================//
 
-  CharSourceRange getRange() const {
+  CharSourceRange getRecordedRange() const {
     assert(isRecorded());
     return RecordedData.Range;
   }
@@ -228,6 +242,10 @@ class ParsedRawSyntaxNode {
 
   // Deferred Layout Data ====================================================//
 
+  CharSourceRange getDeferredLayoutRange() const {
+    assert(DK == DataKind::DeferredLayout);
+    return DeferredLayout.Range;
+  }
   ArrayRef<ParsedRawSyntaxNode> getDeferredChildren() const {
     assert(DK == DataKind::DeferredLayout);
     return DeferredLayout.Children;
@@ -259,6 +277,19 @@ class ParsedRawSyntaxNode {
 
   // Deferred Token Data =====================================================//
 
+  CharSourceRange getDeferredTokenRangeWithTrivia() const {
+    assert(DK == DataKind::DeferredToken);
+    auto leadTriviaPieces = getDeferredLeadingTriviaPieces();
+    auto trailTriviaPieces = getDeferredTrailingTriviaPieces();
+
+    auto leadTriviaLen = ParsedTriviaPiece::getTotalLength(leadTriviaPieces);
+    auto trailTriviaLen = ParsedTriviaPiece::getTotalLength(trailTriviaPieces);
+
+    SourceLoc begin = DeferredToken.TokLoc.getAdvancedLoc(-leadTriviaLen);
+    unsigned len = leadTriviaLen + DeferredToken.TokLength + trailTriviaLen;
+
+    return CharSourceRange{begin, len};
+  }
   CharSourceRange getDeferredTokenRange() const {
     assert(DK == DataKind::DeferredToken);
     return CharSourceRange{DeferredToken.TokLoc, DeferredToken.TokLength};
diff --git a/include/swift/Parse/ParsedRawSyntaxRecorder.h b/include/swift/Parse/ParsedRawSyntaxRecorder.h
@@ -73,6 +73,10 @@ class ParsedRawSyntaxRecorder {
   /// Used for incremental re-parsing.
   ParsedRawSyntaxNode lookupNode(size_t lexerOffset, SourceLoc loc,
                                  syntax::SyntaxKind kind);
+
+  #ifndef NDEBUG
+  static void verifyElementRanges(ArrayRef<ParsedRawSyntaxNode> elements);
+  #endif
 };
 
 } // end namespace swift
diff --git a/include/swift/Parse/ParsedSyntaxBuilders.h.gyb b/include/swift/Parse/ParsedSyntaxBuilders.h.gyb
@@ -62,9 +62,9 @@ public:
 %     end
 
   Parsed${node.name} build();
-  Parsed${node.name} makeDeferred();
 
 private:
+  Parsed${node.name} makeDeferred();
   Parsed${node.name} record();
   void finishLayout(bool deferred);
 };
diff --git a/include/swift/Parse/ParsedSyntaxRecorder.h.gyb b/include/swift/Parse/ParsedSyntaxRecorder.h.gyb
@@ -43,23 +43,22 @@ struct ParsedSyntaxRecorder {
 %     end
 %     child_params = ', '.join(child_params)
 private:
-  static Parsed${node.name} record${node.syntax_kind}(${child_params},
+  static Parsed${node.name} record${node.syntax_kind}(MutableArrayRef<ParsedRawSyntaxNode> layout,
                                               ParsedRawSyntaxRecorder &rec);
-public:
-  static Parsed${node.name} defer${node.syntax_kind}(${child_params},
+  static Parsed${node.name} defer${node.syntax_kind}(MutableArrayRef<ParsedRawSyntaxNode> layout,
                                               SyntaxParsingContext &SPCtx);
+public:
   static Parsed${node.name} make${node.syntax_kind}(${child_params},
                                               SyntaxParsingContext &SPCtx);
 %   elif node.is_syntax_collection():
 private:
   static Parsed${node.name} record${node.syntax_kind}(
-      MutableArrayRef<Parsed${node.collection_element_type}> elts,
+      MutableArrayRef<ParsedRawSyntaxNode> layout,
       ParsedRawSyntaxRecorder &rec);
-
-public:
   static Parsed${node.name} defer${node.syntax_kind}(
-      MutableArrayRef<Parsed${node.collection_element_type}> elts,
+      MutableArrayRef<ParsedRawSyntaxNode> layout,
       SyntaxParsingContext &SPCtx);
+public:
   static Parsed${node.name} make${node.syntax_kind}(
       MutableArrayRef<Parsed${node.collection_element_type}> elts,
       SyntaxParsingContext &SPCtx);
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -532,6 +532,10 @@ class Parser {
     return consumeToken();
   }
 
+  SourceLoc leadingTriviaLoc() {
+    return Tok.getLoc().getAdvancedLoc(-LeadingTrivia.getLength());
+  }
+
   SourceLoc consumeIdentifier(Identifier *Result = nullptr,
                               bool allowDollarIdentifier = false) {
     assert(Tok.isAny(tok::identifier, tok::kw_self, tok::kw_Self));
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
@@ -4615,9 +4615,9 @@ ParserStatus Parser::parseGetSet(ParseDeclOptions Flags,
     accessors.LBLoc = consumeToken(tok::l_brace);
     // Give syntax node an empty accessor list.
     if (SyntaxContext->isEnabled()) {
-      SourceLoc listLoc = accessors.LBLoc.getAdvancedLoc(1);
+      SourceLoc listLoc = leadingTriviaLoc();
       SyntaxContext->addSyntax(
-        ParsedSyntaxRecorder::makeBlankAccessorList(listLoc, *SyntaxContext));
+          ParsedSyntaxRecorder::makeBlankAccessorList(listLoc, *SyntaxContext));
     }
     accessors.RBLoc = consumeToken(tok::r_brace);
 
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
@@ -1183,7 +1183,7 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,
         // Add dummy blank argument list to the call expression syntax.
         SyntaxContext->addSyntax(
             ParsedSyntaxRecorder::makeBlankTupleExprElementList(
-                Tok.getLoc(), *SyntaxContext));
+                leadingTriviaLoc(), *SyntaxContext));
       }
 
       ParserResult<Expr> closure =
@@ -1466,6 +1466,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
         // name, not a binding, because it is the start of an enum pattern or
         // call pattern.
         peekToken().isNot(tok::period, tok::period_prefix, tok::l_paren)) {
+      DeferringContextRAII Deferring(*SyntaxContext);
       Identifier name;
       SourceLoc loc = consumeIdentifier(&name, /*allowDollarIdentifier=*/true);
       auto introducer = (InVarOrLetPattern != IVOLP_InVar
@@ -1474,10 +1475,10 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
       auto pattern = createBindingFromPattern(loc, name, introducer);
       if (SyntaxContext->isEnabled()) {
         ParsedPatternSyntax PatternNode =
-            ParsedSyntaxRecorder::deferIdentifierPattern(
+            ParsedSyntaxRecorder::makeIdentifierPattern(
                                     SyntaxContext->popToken(), *SyntaxContext);
         ParsedExprSyntax ExprNode =
-            ParsedSyntaxRecorder::deferUnresolvedPatternExpr(std::move(PatternNode),
+            ParsedSyntaxRecorder::makeUnresolvedPatternExpr(std::move(PatternNode),
                                                              *SyntaxContext);
         SyntaxContext->addSyntax(std::move(ExprNode));
       }
@@ -1587,7 +1588,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
         // Add dummy blank argument list to the call expression syntax.
         SyntaxContext->addSyntax(
             ParsedSyntaxRecorder::makeBlankTupleExprElementList(
-                Tok.getLoc(), *SyntaxContext));
+                leadingTriviaLoc(), *SyntaxContext));
       }
 
       ParserResult<Expr> closure =
@@ -2101,7 +2102,7 @@ DeclName Parser::parseUnqualifiedDeclName(bool afterDot,
     if (SyntaxContext->isEnabled())
       SyntaxContext->addSyntax(
           ParsedSyntaxRecorder::makeBlankDeclNameArgumentList(
-            Tok.getLoc(), *SyntaxContext));
+              leadingTriviaLoc(), *SyntaxContext));
     consumeToken(tok::r_paren);
     loc = DeclNameLoc(baseNameLoc);
     SmallVector<Identifier, 2> argumentLabels;
@@ -3303,9 +3304,8 @@ ParserResult<Expr> Parser::parseExprCollection() {
   // [] is always an array.
   if (Tok.is(tok::r_square)) {
     if (SyntaxContext->isEnabled())
-      SyntaxContext->addSyntax(
-          ParsedSyntaxRecorder::makeBlankArrayElementList(
-                                Tok.getLoc(), *SyntaxContext));
+      SyntaxContext->addSyntax(ParsedSyntaxRecorder::makeBlankArrayElementList(
+          leadingTriviaLoc(), *SyntaxContext));
     RSquareLoc = consumeToken(tok::r_square);
     ArrayOrDictContext.setCreateSyntax(SyntaxKind::ArrayExpr);
     return makeParserResult(
diff --git a/lib/Parse/ParsedRawSyntaxNode.cpp b/lib/Parse/ParsedRawSyntaxNode.cpp
@@ -21,18 +21,34 @@ ParsedRawSyntaxNode
 ParsedRawSyntaxNode::makeDeferred(SyntaxKind k,
                                   MutableArrayRef<ParsedRawSyntaxNode> deferredNodes,
                                   SyntaxParsingContext &ctx) {
+  CharSourceRange range;
   if (deferredNodes.empty()) {
-    return ParsedRawSyntaxNode(k, {});
+    return ParsedRawSyntaxNode(k, range, {});
   }
   ParsedRawSyntaxNode *newPtr =
     ctx.getScratchAlloc().Allocate<ParsedRawSyntaxNode>(deferredNodes.size());
 
-  // uninitialized move;
+#ifndef NDEBUG
+  ParsedRawSyntaxRecorder::verifyElementRanges(deferredNodes);
+#endif
   auto ptr = newPtr;
-  for (auto &node : deferredNodes)
-    :: new (static_cast<void *>(ptr++)) ParsedRawSyntaxNode(std::move(node));
+  for (auto &node : deferredNodes) {
+    // Cached range.
+    if (!node.isNull() && !node.isMissing()) {
+      auto nodeRange = node.getDeferredRange();
+      if (nodeRange.isValid()) {
+        if (range.isInvalid())
+          range = nodeRange;
+        else
+          range.widen(nodeRange);
+      }
+    }
 
-  return ParsedRawSyntaxNode(k, makeMutableArrayRef(newPtr, deferredNodes.size()));
+    // uninitialized move;
+    :: new (static_cast<void *>(ptr++)) ParsedRawSyntaxNode(std::move(node));
+  }
+  return ParsedRawSyntaxNode(k, range,
+                             makeMutableArrayRef(newPtr, deferredNodes.size()));
 }
 
 ParsedRawSyntaxNode
diff --git a/lib/Parse/ParsedRawSyntaxRecorder.cpp b/lib/Parse/ParsedRawSyntaxRecorder.cpp
@@ -53,7 +53,8 @@ ParsedRawSyntaxNode
 ParsedRawSyntaxRecorder::recordMissingToken(tok tokenKind, SourceLoc loc) {
   CharSourceRange range{loc, 0};
   OpaqueSyntaxNode n = SPActions->recordMissingToken(tokenKind, loc);
-  return ParsedRawSyntaxNode{SyntaxKind::Token, tokenKind, range, n};
+  return ParsedRawSyntaxNode{SyntaxKind::Token, tokenKind, range, n,
+                             /*isMissing=*/true};
 }
 
 static ParsedRawSyntaxNode
@@ -74,6 +75,9 @@ getRecordedNode(ParsedRawSyntaxNode node, ParsedRawSyntaxRecorder &rec) {
 ParsedRawSyntaxNode
 ParsedRawSyntaxRecorder::recordRawSyntax(SyntaxKind kind,
                                          MutableArrayRef<ParsedRawSyntaxNode> elements) {
+#ifndef NDEBUG
+  ParsedRawSyntaxRecorder::verifyElementRanges(elements);
+#endif
   CharSourceRange range;
   SmallVector<OpaqueSyntaxNode, 16> subnodes;
   if (!elements.empty()) {
@@ -84,11 +88,11 @@ ParsedRawSyntaxRecorder::recordRawSyntax(SyntaxKind kind,
       if (subnode.isNull()) {
         subnodes.push_back(nullptr);
       } else if (subnode.isRecorded()) {
-        localRange = subnode.getRange();
+        localRange = subnode.getRecordedRange();
         subnodes.push_back(subnode.takeOpaqueNode());
       } else {
         auto recorded = getRecordedNode(subnode.copyDeferred(), *this);
-        localRange = recorded.getRange();
+        localRange = recorded.getRecordedRange();
         subnodes.push_back(recorded.takeOpaqueNode());
       }
 
@@ -128,3 +132,21 @@ ParsedRawSyntaxRecorder::lookupNode(size_t lexerOffset, SourceLoc loc,
   CharSourceRange range{loc, unsigned(length)};
   return ParsedRawSyntaxNode{kind, tok::unknown, range, n};
 }
+
+#ifndef NDEBUG
+void ParsedRawSyntaxRecorder::verifyElementRanges(ArrayRef<ParsedRawSyntaxNode> elements) {
+  SourceLoc prevEndLoc;
+  for (const auto &elem: elements) {
+    if (elem.isMissing() || elem.isNull())
+      continue;
+    CharSourceRange range = elem.isRecorded()
+      ? elem.getRecordedRange()
+      : elem.getDeferredRange();
+    if (range.isValid()) {
+      assert((prevEndLoc.isInvalid() || range.getStart() == prevEndLoc)
+             && "Non-contiguous child ranges?");
+      prevEndLoc = range.getEnd();
+    }
+  }
+}
+#endif
diff --git a/lib/Parse/ParsedSyntaxBuilders.cpp.gyb b/lib/Parse/ParsedSyntaxBuilders.cpp.gyb
@@ -74,7 +74,7 @@ Parsed${node.name}Builder::makeDeferred() {
 
 Parsed${node.name}
 Parsed${node.name}Builder::build() {
-  if (SPCtx.isBacktracking())
+  if (SPCtx.shouldDefer())
     return makeDeferred();
   return record();
 }
diff --git a/lib/Parse/ParsedSyntaxRecorder.cpp.gyb b/lib/Parse/ParsedSyntaxRecorder.cpp.gyb
diff --git a/lib/Parse/SyntaxParsingContext.cpp b/lib/Parse/SyntaxParsingContext.cpp
diff --git a/test/IDE/complete_expr_postfix_begin.swift b/test/IDE/complete_expr_postfix_begin.swift
diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp

Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ Parsed${node.name}Builder::makeDeferred() {`
`74`	`74`
`75`	`75`	`Parsed${node.name}`
`76`	`76`	`Parsed${node.name}Builder::build() {`
`77`		`- if (SPCtx.isBacktracking())`
	`77`	`+ if (SPCtx.shouldDefer())`
`78`	`78`	`return makeDeferred();`
`79`	`79`	`return record();`
`80`	`80`	`}`