[libSyntax] Remove incremental JSON transfer option

We were only keeping track of `RawSyntax` node IDs to incrementally transfer a syntax tree via JSON. However, AFAICT the incremental JSON transfer option has been superceeded by `SyntaxParseActions`, which are more efficient.

So, let’s clean up and remove the `RawSyntax` node ID and JSON incremental transfer option.

In places that still need a notion of `RawSyntax` identity (like determining the reused syntax regions), use the `RawSyntax`’s pointer instead of the manually created ID.

In `incr_transfer_round_trip.py` always use the code path that uses the `SyntaxParseActions` and remove the transitional code that was still using the incremental JSON transfer but was never called.

Author: ahoppen

include/swift/Parse/SyntaxParsingCache.h

@@ -60,8 +60,8 @@ class SyntaxParsingCache {
   /// the source file that is now parsed incrementally
   llvm::SmallVector<SourceEdit, 4> Edits;
 
-  /// The IDs of all syntax nodes that got reused are collected in this vector.
-  std::unordered_set<SyntaxNodeId> ReusedNodeIds;
+  /// The \c RawSyntax nodes that got reused are collected in this vector.
+  std::unordered_set<const RawSyntax *> ReusedNodes;
 
 public:
   SyntaxParsingCache(SourceFileSyntax OldSyntaxTree)
@@ -83,8 +83,8 @@ class SyntaxParsingCache {
   /// reused for a new syntax tree.
   llvm::Optional<Syntax> lookUp(size_t NewPosition, SyntaxKind Kind);
 
-  const std::unordered_set<SyntaxNodeId> &getReusedNodeIds() const {
-    return ReusedNodeIds;
+  const std::unordered_set<const RawSyntax *> &getReusedNodes() const {
+    return ReusedNodes;
   }
 
   /// Get the source regions of the new source file, represented by

include/swift/Syntax/RawSyntax.h

@@ -151,25 +151,16 @@ struct SyntaxPrintOptions {
   bool PrintTrivialNodeKind = false;
 };
 
-typedef unsigned SyntaxNodeId;
-
 /// RawSyntax - the strictly immutable, shared backing nodes for all syntax.
 ///
 /// This is implementation detail - do not expose it in public API.
 class RawSyntax final
     : private llvm::TrailingObjects<RawSyntax, const RawSyntax *> {
   friend TrailingObjects;
 
-  /// The ID that shall be used for the next node that is created and does not
-  /// have a manually specified id
-  static SyntaxNodeId NextFreeNodeId;
-
   /// The \c SyntaxArena in which this node was allocated.
   SyntaxArena *Arena;
 
-  /// An ID of this node that is stable across incremental parses
-  SyntaxNodeId NodeId;
-
   /// Number of bytes this node takes up spelled out in the source code.
   /// Always 0 if the node is missing.
   uint32_t TextLength;
@@ -227,7 +218,7 @@ class RawSyntax final
   template <typename ChildrenIteratorType>
   RawSyntax(SyntaxKind Kind, ChildrenIteratorType ChildrenIt,
             uint32_t NumChildren, SourcePresence Presence,
-            const RC<SyntaxArena> &Arena, llvm::Optional<SyntaxNodeId> NodeId)
+            const RC<SyntaxArena> &Arena)
       : Arena(Arena.get()), TextLength(0 /*computed in body*/),
         Presence(Presence), IsToken(false),
         Bits(LayoutData{NumChildren,
@@ -254,13 +245,6 @@ class RawSyntax final
 
       *TrailingChildren = Child;
     }
-
-    if (NodeId.hasValue()) {
-      this->NodeId = NodeId.getValue();
-      NextFreeNodeId = std::max(this->NodeId + 1, NextFreeNodeId);
-    } else {
-      this->NodeId = NextFreeNodeId++;
-    }
   }
 
   /// Constructor for creating token nodes
@@ -270,8 +254,7 @@ class RawSyntax final
   /// the caller needs to assure that the NodeId has not been used yet.
   RawSyntax(tok TokKind, StringRef Text, size_t TextLength,
             StringRef LeadingTrivia, StringRef TrailingTrivia,
-            SourcePresence Presence, const RC<SyntaxArena> &Arena,
-            llvm::Optional<SyntaxNodeId> NodeId)
+            SourcePresence Presence, const RC<SyntaxArena> &Arena)
       : Arena(Arena.get()), TextLength(uint32_t(TextLength)),
         Presence(Presence), IsToken(true),
         Bits(TokenData{LeadingTrivia.data(), Text.data(), TrailingTrivia.data(),
@@ -285,13 +268,6 @@ class RawSyntax final
       assert(TextLength ==
              LeadingTrivia.size() + Text.size() + TrailingTrivia.size());
     }
-
-    if (NodeId.hasValue()) {
-      this->NodeId = NodeId.getValue();
-      NextFreeNodeId = std::max(this->NodeId + 1, NextFreeNodeId);
-    } else {
-      this->NodeId = NextFreeNodeId++;
-    }
     Arena->copyStringToArenaIfNecessary(Bits.Token.LeadingTrivia,
                                         Bits.Token.LeadingTriviaLength);
     Arena->copyStringToArenaIfNecessary(Bits.Token.TokenText,
@@ -320,52 +296,47 @@ class RawSyntax final
   template <typename ChildrenIteratorType>
   static const RawSyntax *
   make(SyntaxKind Kind, ChildrenIteratorType ChildrenIt, size_t NumChildren,
-       SourcePresence Presence, const RC<SyntaxArena> &Arena,
-       llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
+       SourcePresence Presence, const RC<SyntaxArena> &Arena) {
     assert(Arena && "RawSyntax nodes must always be allocated in an arena");
     auto size = totalSizeToAlloc<const RawSyntax *>(NumChildren);
     void *data = Arena->Allocate(size, alignof(RawSyntax));
     return new (data)
-        RawSyntax(Kind, ChildrenIt, NumChildren, Presence, Arena, NodeId);
+        RawSyntax(Kind, ChildrenIt, NumChildren, Presence, Arena);
   }
 
   /// Convenience constructor to create a raw "layout" syntax node from an
   /// \c llvm::ArrayRef containing the children.
   static const RawSyntax *
   make(SyntaxKind Kind, llvm::ArrayRef<const RawSyntax *> Children,
-       SourcePresence Presence, const RC<SyntaxArena> &Arena,
-       llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
-    return make(Kind, Children.begin(), Children.size(), Presence, Arena,
-                NodeId);
+       SourcePresence Presence, const RC<SyntaxArena> &Arena) {
+    return make(Kind, Children.begin(), Children.size(), Presence, Arena);
   }
 
   /// Make a raw "token" syntax node.
   static const RawSyntax *
   make(tok TokKind, StringRef Text, size_t TextLength, StringRef LeadingTrivia,
        StringRef TrailingTrivia, SourcePresence Presence,
-       const RC<SyntaxArena> &Arena,
-       llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
+       const RC<SyntaxArena> &Arena) {
     assert(Arena && "RawSyntax nodes must always be allocated in an arena");
     auto size = totalSizeToAlloc<const RawSyntax *>(0);
     void *data = Arena->Allocate(size, alignof(RawSyntax));
     return new (data) RawSyntax(TokKind, Text, TextLength, LeadingTrivia,
-                                TrailingTrivia, Presence, Arena, NodeId);
+                                TrailingTrivia, Presence, Arena);
   }
 
   /// Make a raw "token" syntax node that was allocated in \p Arena.
   static const RawSyntax *
   makeAndCalcLength(tok TokKind, StringRef Text, StringRef LeadingTrivia,
                     StringRef TrailingTrivia, SourcePresence Presence,
-                    const RC<SyntaxArena> &Arena,
-                    llvm::Optional<SyntaxNodeId> NodeId = llvm::None) {
+                    const RC<SyntaxArena> &Arena) {
     size_t TextLength = 0;
     if (Presence != SourcePresence::Missing) {
       TextLength += LeadingTrivia.size();
       TextLength += Text.size();
       TextLength += TrailingTrivia.size();
     }
     return make(TokKind, Text, TextLength, LeadingTrivia, TrailingTrivia,
-                Presence, Arena, NodeId);
+                Presence, Arena);
   }
 
   /// Make a missing raw "layout" syntax node.
@@ -413,9 +384,6 @@ class RawSyntax final
     }
   }
 
-  /// Get an ID for this node that is stable across incremental parses
-  SyntaxNodeId getId() const { return NodeId; }
-
   /// Returns true if the node is "missing" in the source (i.e. it was
   /// expected (or optional) but not written.
   bool isMissing() const { return getPresence() == SourcePresence::Missing; }

include/swift/Syntax/Serialization/SyntaxDeserialization.h

@@ -177,30 +177,18 @@ template <> struct MappingTraits<const swift::RawSyntax *> {
       in.mapRequired("trailingTrivia", trailingTrivia);
       swift::SourcePresence presence;
       in.mapRequired("presence", presence);
-      /// FIXME: This is a workaround for existing bug from llvm yaml parser
-      /// which would raise error when deserializing number with trailing
-      /// character like "1\n". See https://bugs.llvm.org/show_bug.cgi?id=15505
-      StringRef nodeIdString;
-      in.mapRequired("id", nodeIdString);
-      unsigned nodeId = std::atoi(nodeIdString.data());
       value = swift::RawSyntax::makeAndCalcLength(
           tokenKind, text, leadingTrivia, trailingTrivia, presence,
-          input->Arena, nodeId);
+          input->Arena);
     } else {
       swift::SyntaxKind kind;
       in.mapRequired("kind", kind);
       std::vector<const swift::RawSyntax *> layout;
       in.mapRequired("layout", layout);
       swift::SourcePresence presence;
       in.mapRequired("presence", presence);
-      /// FIXME: This is a workaround for existing bug from llvm yaml parser
-      /// which would raise error when deserializing number with trailing
-      /// character like "1\n". See https://bugs.llvm.org/show_bug.cgi?id=15505
-      StringRef nodeIdString;
-      in.mapRequired("id", nodeIdString);
-      unsigned nodeId = std::atoi(nodeIdString.data());
       value =
-          swift::RawSyntax::make(kind, layout, presence, input->Arena, nodeId);
+          swift::RawSyntax::make(kind, layout, presence, input->Arena);
     }
   }
 };

include/swift/Syntax/Serialization/SyntaxSerialization.h

@@ -28,14 +28,6 @@
 namespace swift {
 namespace json {
 
-/// The associated value will be interpreted as \c bool. If \c true the node IDs
-/// will not be included in the serialized JSON.
-static void *DontSerializeNodeIdsUserInfoKey = &DontSerializeNodeIdsUserInfoKey;
-
-/// The user info key pointing to a std::unordered_set of IDs of nodes that
-/// shall be omitted when the tree gets serialized
-static void *OmitNodesUserInfoKey = &OmitNodesUserInfoKey;
-
 /// Serialization traits for SourcePresence.
 template <>
 struct ScalarReferenceTraits<syntax::SourcePresence> {
@@ -150,22 +142,6 @@ struct ObjectTraits<TokenDescription> {
 template <>
 struct ObjectTraits<const syntax::RawSyntax> {
   static void mapping(Output &out, const syntax::RawSyntax &value) {
-    bool dontSerializeIds =
-        (bool)out.getUserInfo()[DontSerializeNodeIdsUserInfoKey];
-    if (!dontSerializeIds) {
-      auto nodeId = value.getId();
-      out.mapRequired("id", nodeId);
-    }
-
-    auto omitNodes =
-        (std::unordered_set<unsigned> *)out.getUserInfo()[OmitNodesUserInfoKey];
-
-    if (omitNodes && omitNodes->count(value.getId()) > 0) {
-      bool omitted = true;
-      out.mapRequired("omitted", omitted);
-      return;
-    }
-
     if (value.isToken()) {
       auto tokenKind = value.getTokenKind();
       auto text = value.getTokenText();

include/swift/Syntax/Syntax.h

@@ -201,9 +201,6 @@ class Syntax {
   /// Get the shared raw syntax.
   const RawSyntax *getRaw() const;
 
-  /// Get an ID for this node that is stable across incremental parses
-  SyntaxNodeId getId() const { return getRaw()->getId(); }
-
   /// Get the number of child nodes in this piece of syntax, not including
   /// tokens.
   size_t getNumChildren() const;
@@ -370,13 +367,6 @@ class SyntaxRef {
   /// Get the kind of syntax.
   SyntaxKind getKind() const { return getRaw()->getKind(); }
 
-  /// Get an ID for the \c RawSyntax node backing this \c Syntax which is
-  /// stable across incremental parses.
-  /// Note that this is different from the \c AbsoluteRawSyntax's \c NodeId,
-  /// which uniquely identifies this node in the tree, but is not stable across
-  /// incremental parses.
-  SyntaxNodeId getId() const { return getRaw()->getId(); }
-
   /// Return the number of bytes this node takes when spelled out in the source,
   /// including trivia.
   size_t getTextLength() const { return getRaw()->getTextLength(); }

lib/Parse/SyntaxParsingCache.cpp

@@ -111,7 +111,7 @@ llvm::Optional<Syntax> SyntaxParsingCache::lookUp(size_t NewPosition,
 
   auto Node = lookUpFrom(OldSyntaxTree, /*NodeStart=*/0, *OldPosition, Kind);
   if (Node.hasValue()) {
-    ReusedNodeIds.insert(Node->getId());
+    ReusedNodes.insert(Node->getRaw());
   }
   return Node;
 }
@@ -120,16 +120,16 @@ std::vector<SyntaxReuseRegion>
 SyntaxParsingCache::getReusedRegions(const SourceFileSyntax &SyntaxTree) const {
   /// Determines the reused source regions from reused syntax node IDs
   class ReusedRegionsCollector : public SyntaxVisitor {
-    std::unordered_set<SyntaxNodeId> ReusedNodeIds;
+    std::unordered_set<const RawSyntax *> ReusedNodes;
     std::vector<SyntaxReuseRegion> ReusedRegions;
 
-    bool didReuseNode(SyntaxNodeId NodeId) {
-      return ReusedNodeIds.count(NodeId) > 0;
+    bool didReuseNode(const RawSyntax *Node) {
+      return ReusedNodes.count(Node) > 0;
     }
 
   public:
-    ReusedRegionsCollector(std::unordered_set<SyntaxNodeId> ReusedNodeIds)
-        : ReusedNodeIds(ReusedNodeIds) {}
+    ReusedRegionsCollector(std::unordered_set<const RawSyntax *> ReusedNodes)
+        : ReusedNodes(ReusedNodes) {}
 
     const std::vector<SyntaxReuseRegion> &getReusedRegions() {
       std::sort(ReusedRegions.begin(), ReusedRegions.end(),
@@ -141,7 +141,7 @@ SyntaxParsingCache::getReusedRegions(const SourceFileSyntax &SyntaxTree) const {
     }
 
     void visit(Syntax Node) override {
-      if (didReuseNode(Node.getId())) {
+      if (didReuseNode(Node.getRaw())) {
         // Node has been reused, add it to the list
         auto Start = Node.getAbsolutePositionBeforeLeadingTrivia();
         auto End = Node.getAbsoluteEndPositionAfterTrailingTrivia();
@@ -158,7 +158,7 @@ SyntaxParsingCache::getReusedRegions(const SourceFileSyntax &SyntaxTree) const {
     }
   };
 
-  ReusedRegionsCollector ReuseRegionsCollector(getReusedNodeIds());
+  ReusedRegionsCollector ReuseRegionsCollector(getReusedNodes());
   ReuseRegionsCollector.collectReusedRegions(SyntaxTree);
   return ReuseRegionsCollector.getReusedRegions();
 }

lib/Syntax/RawSyntax.cpp

@@ -87,9 +87,6 @@ Trivia lexTrivia(StringRef TriviaStr) {
   return SyntaxTrivia;
 }
 
-// FIXME: If we want thread-safety for tree creation, this needs to be atomic.
-unsigned RawSyntax::NextFreeNodeId = 1;
-
 Trivia RawSyntax::getLeadingTriviaPieces() const {
   return lexTrivia(getLeadingTrivia());
 }