[libSyntax] Remove getData method from ParsedRawSyntaxNode

The getData and getRecordedOrDeferredNode methods broke the move-only semantics of ParsedRawSyntaxNode because it allowed retrieving the data without resetting it.

Change most uses to use takeData or takeRecordedOrDeferredNode instead of the get methods.

To retrieve the children of a ParsedRawSyntaxNode, we still need a way to access the OpaqueSyntaxNode for inspection by the SyntaxParseActions without resetting it. For this, we still maintain a method with the semantics of getData, but it’s now called getUnsafeOpaqueData to make sure it’s not accidentally used.

Author: ahoppen

include/swift/Parse/ParsedRawSyntaxNode.h

@@ -106,16 +106,25 @@ class ParsedRawSyntaxNode {
   /// null).
   DataKind getDataKind() const { return Data.getKind(); }
 
-  /// Returns the opaque data of this node. This must be interpreted by the
-  /// \c SyntaxParseAction, which likely also needs the node type to know
-  /// what type of node the data represents.
-  OpaqueSyntaxNode getData() const { return Data.getOpaque(); }
+  /// Returns the opaque data of this node, assuming that it is deferred. This
+  /// must be interpreted by the \c SyntaxParseAction, which likely also needs
+  /// the node type (layout or token) to interpret the data.
+  /// The data opaque data returned by this function *must not* be used to
+  /// record the node, only to insepect it.
+  OpaqueSyntaxNode getUnsafeDeferredOpaqueData() const {
+    assert(isDeferredLayout() || isDeferredToken());
+    return Data.getOpaque();
+  }
 
-  RecordedOrDeferredNode getRecordedOrDeferredNode() const { return Data; }
+  RecordedOrDeferredNode takeRecordedOrDeferredNode() {
+    RecordedOrDeferredNode Data = this->Data;
+    reset();
+    return Data;
+  }
 
   /// Return the opaque data of this node and reset it.
   OpaqueSyntaxNode takeData() {
-    OpaqueSyntaxNode Data = this->getData();
+    OpaqueSyntaxNode Data = this->Data.getOpaque();
     reset();
     return Data;
   }

include/swift/Parse/ParsedRawSyntaxRecorder.h

@@ -43,7 +43,9 @@ class ParsedRawSyntaxRecorder final {
 
   /// Assuming that \p node is a deferred layout or token node, record it and
   /// return the recorded node.
-  ParsedRawSyntaxNode recordDeferredNode(const ParsedRawSyntaxNode &node);
+  /// This consumes the data from \c node, which is unusable after it has been
+  /// recorded. The returned node should be used afterwards instead.
+  ParsedRawSyntaxNode recordDeferredNode(ParsedRawSyntaxNode &node);
 
 public:
   explicit ParsedRawSyntaxRecorder(std::shared_ptr<SyntaxParseActions> spActions)

lib/Parse/ParsedRawSyntaxRecorder.cpp

@@ -28,19 +28,19 @@ using namespace swift;
 using namespace swift::syntax;
 
 ParsedRawSyntaxNode
-ParsedRawSyntaxRecorder::recordDeferredNode(const ParsedRawSyntaxNode &node) {
+ParsedRawSyntaxRecorder::recordDeferredNode(ParsedRawSyntaxNode &node) {
   switch (node.getDataKind()) {
   case RecordedOrDeferredNode::Kind::Null:
   case RecordedOrDeferredNode::Kind::Recorded:
     llvm_unreachable("Not deferred");
   case RecordedOrDeferredNode::Kind::DeferredLayout: {
-    OpaqueSyntaxNode Data = SPActions->recordDeferredLayout(node.getData());
+    OpaqueSyntaxNode Data = SPActions->recordDeferredLayout(node.takeData());
     return ParsedRawSyntaxNode(
         Data, node.getRange(), node.getKind(), node.getTokenKind(),
         ParsedRawSyntaxNode::DataKind::Recorded, node.isMissing());
   }
   case RecordedOrDeferredNode::Kind::DeferredToken: {
-    OpaqueSyntaxNode Data = SPActions->recordDeferredToken(node.getData());
+    OpaqueSyntaxNode Data = SPActions->recordDeferredToken(node.takeData());
     return ParsedRawSyntaxNode(
         Data, node.getRange(), node.getKind(), node.getTokenKind(),
         ParsedRawSyntaxNode::DataKind::Recorded, node.isMissing());
@@ -158,7 +158,7 @@ ParsedRawSyntaxNode ParsedRawSyntaxRecorder::makeDeferred(
       }
     }
 
-    children[i] = node.getRecordedOrDeferredNode();
+    children[i] = node.takeRecordedOrDeferredNode();
   }
   auto data = SPActions->makeDeferredLayout(k, /*IsMissing=*/false, children);
   return ParsedRawSyntaxNode(data, range, k, tok::NUM_TOKENS,
@@ -213,8 +213,9 @@ ParsedRawSyntaxNode
 ParsedRawSyntaxRecorder::getDeferredChild(const ParsedRawSyntaxNode &parent,
                                           size_t childIndex) const {
   assert(parent.isDeferredLayout());
-  auto childInfo = SPActions->getDeferredChild(parent.getData(), childIndex,
-                                               parent.getRange().getStart());
+  auto childInfo = SPActions->getDeferredChild(
+      parent.getUnsafeDeferredOpaqueData(),
+      childIndex, parent.getRange().getStart());
   return ParsedRawSyntaxNode(childInfo.Data, childInfo.Range,
                              childInfo.SyntaxKind, childInfo.TokenKind,
                              childInfo.SyntaxKind == syntax::SyntaxKind::Token
@@ -226,7 +227,7 @@ ParsedRawSyntaxRecorder::getDeferredChild(const ParsedRawSyntaxNode &parent,
 size_t ParsedRawSyntaxRecorder::getDeferredNumChildren(
     const ParsedRawSyntaxNode &node) const {
   assert(node.isDeferredLayout());
-  return SPActions->getDeferredNumChildren(node.getData());
+  return SPActions->getDeferredNumChildren(node.getUnsafeDeferredOpaqueData());
 }
 
 #ifndef NDEBUG