Merge pull request #36385 from ahoppen/pr/no-range-in-parsedrawsyntaxnode

[libSyntax] Only keep track of range in ParsedRawSyntaxNode if verifying element ranges

Author: ahoppen

include/swift/Parse/ParsedRawSyntaxNode.h

@@ -21,6 +21,16 @@
 #include "swift/Syntax/SyntaxKind.h"
 #include "llvm/Support/Debug.h"
 
+#ifndef NDEBUG
+/// Whether \c ParsedRawSyntaxNode should keep track of its range and verify
+/// that the children of layout nodes have consecutive ranges.
+/// Because this significantly changes the way, \c ParsedRawSyntaxNode and
+/// \c ParsedRawSyntaxNodeRecorder are being compiled, this is a separate
+/// constant from \c NDEBUG, so that it can be toggled independently to \c
+/// NDEBUG during development.
+#define PARSEDRAWSYNTAXNODE_VERIFY_RANGES 1
+#endif
+
 namespace swift {
 
 typedef const void *OpaqueSyntaxNode;
@@ -48,8 +58,13 @@ class ParsedRawSyntaxNode {
   /// SyntaxParseActions, which created it.
   RecordedOrDeferredNode Data;
 
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
   /// The range of this node, including trivia.
+  /// Only store this as a member when it's actually needed to keep \c
+  /// ParsedRawSyntaxNode as small as possible, which improves performance
+  /// when it is being passed around.
   CharSourceRange Range;
+#endif
   uint16_t SynKind;
   uint16_t TokKind;
   /// Primary used for capturing a deferred missing token.
@@ -62,24 +77,46 @@ class ParsedRawSyntaxNode {
   // MARK: - Constructors
 
   ParsedRawSyntaxNode()
-      : Data(nullptr, DataKind::Null), Range(),
+      : Data(nullptr, DataKind::Null),
         SynKind(uint16_t(syntax::SyntaxKind::Unknown)),
         TokKind(uint16_t(tok::unknown)) {}
 
-  ParsedRawSyntaxNode(OpaqueSyntaxNode Opaque, CharSourceRange Range,
-                      syntax::SyntaxKind SynKind, tok TokKind, DataKind DK,
-                      bool IsMissing)
-      : Data(Opaque, DK), Range(Range), SynKind(uint16_t(SynKind)),
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
+  ParsedRawSyntaxNode(RecordedOrDeferredNode Data, syntax::SyntaxKind SynKind,
+                      tok TokKind, bool IsMissing, CharSourceRange Range)
+      : Data(Data), Range(Range), SynKind(uint16_t(SynKind)),
         TokKind(uint16_t(TokKind)), IsMissing(IsMissing) {
     assert(getKind() == SynKind && "Syntax kind with too large value!");
     assert(getTokenKind() == TokKind && "Token kind with too large value!");
   }
 
+  ParsedRawSyntaxNode(OpaqueSyntaxNode Opaque, syntax::SyntaxKind SynKind,
+                      tok TokKind, DataKind DK, bool IsMissing,
+                      CharSourceRange Range)
+      : ParsedRawSyntaxNode(RecordedOrDeferredNode(Opaque, DK), SynKind,
+                            TokKind, IsMissing, Range) {}
+#else
+  ParsedRawSyntaxNode(RecordedOrDeferredNode Data, syntax::SyntaxKind SynKind,
+                      tok TokKind, bool IsMissing)
+      : Data(Data), SynKind(uint16_t(SynKind)), TokKind(uint16_t(TokKind)),
+        IsMissing(IsMissing) {
+    assert(getKind() == SynKind && "Syntax kind with too large value!");
+    assert(getTokenKind() == TokKind && "Token kind with too large value!");
+  }
+
+  ParsedRawSyntaxNode(OpaqueSyntaxNode Opaque, syntax::SyntaxKind SynKind,
+                      tok TokKind, DataKind DK, bool IsMissing)
+      : ParsedRawSyntaxNode(RecordedOrDeferredNode(Opaque, DK), SynKind,
+                            TokKind, IsMissing) {}
+#endif
+
   ParsedRawSyntaxNode &operator=(ParsedRawSyntaxNode &&other) {
     assert(ensureDataIsNotRecorded() &&
            "recorded data is being destroyed by assignment");
     Data = std::move(other.Data);
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
     Range = std::move(other.Range);
+#endif
     SynKind = std::move(other.SynKind);
     TokKind = std::move(other.TokKind);
     IsMissing = std::move(other.IsMissing);
@@ -91,9 +128,7 @@ class ParsedRawSyntaxNode {
     *this = std::move(other);
   }
 
-  static ParsedRawSyntaxNode null() {
-    return ParsedRawSyntaxNode{};
-  }
+  static ParsedRawSyntaxNode null() { return ParsedRawSyntaxNode(); }
 
   ~ParsedRawSyntaxNode() {
     assert(ensureDataIsNotRecorded() && "recorded data is being destructed");
@@ -152,8 +187,13 @@ class ParsedRawSyntaxNode {
   }
   bool isMissing() const { return IsMissing; }
 
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
   /// Returns the range of this node including leading and trailing trivia.
+  ///
+  /// This method is only present if \c ParsedRawSyntaxNode is keeping track
+  /// of its range to verify element ranges.
   CharSourceRange getRange() const { return Range; }
+#endif
 
   size_t
   getDeferredNumChildren(const SyntaxParsingContext *SyntaxContext) const;
@@ -174,13 +214,17 @@ class ParsedRawSyntaxNode {
     SynKind = uint16_t(syntax::SyntaxKind::Unknown);
     TokKind = uint16_t(tok::unknown);
     IsMissing = false;
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
     Range = CharSourceRange();
+#endif
   }
 
   ParsedRawSyntaxNode unsafeCopy() const {
     ParsedRawSyntaxNode copy;
     copy.Data = Data;
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
     copy.Range = Range;
+#endif
     copy.SynKind = SynKind;
     copy.TokKind = TokKind;
     copy.IsMissing = IsMissing;

include/swift/Parse/ParsedRawSyntaxRecorder.h

@@ -20,12 +20,12 @@
 #define SWIFT_PARSE_PARSEDRAWSYNTAXRECORDER_H
 
 #include "swift/Basic/LLVM.h"
+#include "swift/Parse/ParsedRawSyntaxNode.h"
 #include <memory>
 
 namespace swift {
 
 class CharSourceRange;
-class ParsedRawSyntaxNode;
 struct ParsedTrivia;
 class ParsedTriviaPiece;
 class SyntaxParseActions;
@@ -38,6 +38,18 @@ namespace syntax {
 enum class SyntaxKind : uint16_t;
 }
 
+/// The information returned from the \c lookupNode method in \c
+/// SyntaxParseActions.
+struct ParseLookupResult {
+  ParsedRawSyntaxNode Node;
+
+  /// The length of \c Node spelled out in source, including trivia.
+  size_t Length;
+
+  ParseLookupResult(ParsedRawSyntaxNode &&Node, size_t Length)
+      : Node(std::move(Node)), Length(Length) {}
+};
+
 class ParsedRawSyntaxRecorder final {
   std::shared_ptr<SyntaxParseActions> SPActions;
 
@@ -92,8 +104,8 @@ class ParsedRawSyntaxRecorder final {
   ParsedRawSyntaxNode makeDeferredMissing(tok tokKind, SourceLoc loc);
 
   /// Used for incremental re-parsing.
-  ParsedRawSyntaxNode lookupNode(size_t lexerOffset, SourceLoc loc,
-                                 syntax::SyntaxKind kind);
+  ParseLookupResult lookupNode(size_t lexerOffset, SourceLoc loc,
+                               syntax::SyntaxKind kind);
 
   /// For a deferred layout node \p parent, retrieve the deferred child node
   /// at \p ChildIndex.
@@ -103,8 +115,11 @@ class ParsedRawSyntaxRecorder final {
   /// For a deferred layout node \p node, retrieve the number of children.
   size_t getDeferredNumChildren(const ParsedRawSyntaxNode &node) const;
 
-#ifndef NDEBUG
-  static void verifyElementRanges(ArrayRef<ParsedRawSyntaxNode> elements);
+#ifdef PARSEDRAWSYNTAXNODE_VERIFY_RANGES
+  /// Verify that the ranges of \p elements are all consecutive and return the
+  /// range spanned by all \p elements.
+  static CharSourceRange
+  verifyElementRanges(ArrayRef<ParsedRawSyntaxNode> elements);
 #endif
 };
 

include/swift/Parse/SyntaxParseActions.h

@@ -71,12 +71,11 @@ struct DeferredNodeInfo {
   syntax::SyntaxKind SyntaxKind;
   tok TokenKind;
   bool IsMissing;
-  CharSourceRange Range;
 
   DeferredNodeInfo(RecordedOrDeferredNode Data, syntax::SyntaxKind SyntaxKind,
-                   tok TokenKind, bool IsMissing, CharSourceRange Range)
+                   tok TokenKind, bool IsMissing)
       : Data(Data), SyntaxKind(SyntaxKind), TokenKind(TokenKind),
-        IsMissing(IsMissing), Range(Range) {}
+        IsMissing(IsMissing) {}
 };
 
 // MARK: - SyntaxParseActions
@@ -133,12 +132,24 @@ class SyntaxParseActions {
   /// which created it, to retrieve children.
   /// This method assumes that \p node represents a *deferred* layout node.
   /// This methods returns all information needed to construct a \c
-  /// ParsedRawSyntaxNode of a child node. \p node is the parent node for which
-  /// the child at position \p ChildIndex should be retrieved. Furthmore, \p
-  /// node starts at \p StartLoc.
+  /// ParsedRawSyntaxNode of a child node, except for the range which can be
+  /// retrieved using \c getDeferredChildRange if element ranges should be
+  /// verified
+  /// \p node is the parent node for which the child at position \p ChildIndex
+  /// should be retrieved. Furthmore, \p node starts at \p StartLoc.
   virtual DeferredNodeInfo getDeferredChild(OpaqueSyntaxNode node,
-                                            size_t childIndex,
-                                            SourceLoc startLoc) = 0;
+                                            size_t childIndex) const = 0;
+
+  /// To verify \c ParsedRawSyntaxNode element ranges, the range of child nodes
+  /// returend by \c getDeferredChild needs to be determined. That's what this
+  /// method does.
+  /// It assumes that \p node is a deferred layout node starting at \p startLoc
+  /// and returns the range of the child node at \p childIndex.
+  /// This method is not designed with performance in mind. Do not use in
+  /// performance-critical code.
+  virtual CharSourceRange getDeferredChildRange(OpaqueSyntaxNode node,
+                                                size_t childIndex,
+                                                SourceLoc startLoc) const = 0;
 
   /// Return the number of children, \p node has. These can be retrieved using
   /// \c getDeferredChild.

include/swift/SyntaxParse/SyntaxTreeCreator.h

@@ -83,8 +83,12 @@ class SyntaxTreeCreator final : public SyntaxParseActions {
   OpaqueSyntaxNode recordDeferredToken(OpaqueSyntaxNode deferred) override;
   OpaqueSyntaxNode recordDeferredLayout(OpaqueSyntaxNode deferred) override;
 
-  DeferredNodeInfo getDeferredChild(OpaqueSyntaxNode node, size_t ChildIndex,
-                                    SourceLoc StartLoc) override;
+  DeferredNodeInfo getDeferredChild(OpaqueSyntaxNode node,
+                                    size_t ChildIndex) const override;
+
+  CharSourceRange getDeferredChildRange(OpaqueSyntaxNode node,
+                                        size_t ChildIndex,
+                                        SourceLoc StartLoc) const override;
 
   size_t getDeferredNumChildren(OpaqueSyntaxNode node) override;
 };