Address PR feedback

* Add and improve documentation comments
* Adjust formatting
* Rename recordExactRawSyntax -> rec.recordRawSyntax

Author: akyrtzi

include/swift/Parse/ParsedRawSyntaxNode.h

@@ -30,7 +30,11 @@ typedef void *OpaqueSyntaxNode;
 /// invocation, or 'deferred' which captures the data for a
 /// \c SyntaxParseActions invocation to occur later.
 ///
-/// The deferred form is used for when the parser is backtracking and when
+/// An \c OpaqueSyntaxNode can represent both the result of 'recording' a token
+/// as well as 'recording' a syntax layout, so there's only one
+/// \c RecordedSyntaxNode structure that can represent both.
+///
+/// The 'deferred' form is used for when the parser is backtracking and when
 /// there are instances that it's not clear what will be the final syntax node
 /// in the current parsing context.
 class ParsedRawSyntaxNode {
@@ -101,11 +105,14 @@ class ParsedRawSyntaxNode {
   ParsedRawSyntaxNode(const ParsedRawSyntaxNode &other) {
     switch (other.DK) {
     case DataKind::Recorded:
-      new(&this->RecordedData)RecordedSyntaxNode(other.RecordedData); break;
+      new(&this->RecordedData)RecordedSyntaxNode(other.RecordedData);
+      break;
     case DataKind::DeferredLayout:
-      new(&this->DeferredLayout)DeferredLayoutNode(other.DeferredLayout); break;
+      new(&this->DeferredLayout)DeferredLayoutNode(other.DeferredLayout);
+      break;
     case DataKind::DeferredToken:
-      new(&this->DeferredToken)DeferredTokenNode(other.DeferredToken); break;
+      new(&this->DeferredToken)DeferredTokenNode(other.DeferredToken);
+      break;
     }
     this->SynKind = other.SynKind;
     this->TokKind = other.TokKind;
@@ -116,13 +123,16 @@ class ParsedRawSyntaxNode {
     switch (other.DK) {
     case DataKind::Recorded:
       new(&this->RecordedData)RecordedSyntaxNode(
-          std::move(other.RecordedData)); break;
+          std::move(other.RecordedData));
+      break;
     case DataKind::DeferredLayout:
       new(&this->DeferredLayout)DeferredLayoutNode(
-          std::move(other.DeferredLayout)); break;
+          std::move(other.DeferredLayout));
+      break;
     case DataKind::DeferredToken:
       new(&this->DeferredToken)DeferredTokenNode(
-          std::move(other.DeferredToken)); break;
+          std::move(other.DeferredToken));
+      break;
     }
     this->SynKind = other.SynKind;
     this->TokKind = other.TokKind;

include/swift/Parse/ParsedRawSyntaxRecorder.h

@@ -46,17 +46,17 @@ class ParsedRawSyntaxRecorder {
                                   const syntax::Trivia &leadingTrivia,
                                   const syntax::Trivia &trailingTrivia);
 
-  /// Record a missing token. \c loc can be invalid or an approximate location
+  /// Record a missing token. \p loc can be invalid or an approximate location
   /// of where the token would be if not missing.
   ParsedRawSyntaxNode recordMissingToken(tok tokenKind, SourceLoc loc);
 
-  /// The provided \c elements are an exact layout appropriate for the syntax
-  /// \c kind. Missing optional elements are represented with a null
+  /// The provided \p elements are an exact layout appropriate for the syntax
+  /// \p kind. Missing optional elements are represented with a null
   /// ParsedRawSyntaxNode object.
-  ParsedRawSyntaxNode recordExactRawSyntax(syntax::SyntaxKind kind,
+  ParsedRawSyntaxNode recordRawSyntax(syntax::SyntaxKind kind,
                                      ArrayRef<ParsedRawSyntaxNode> elements);
 
-  /// Record a raw syntax collecton without eny elements. \c loc can be invalid
+  /// Record a raw syntax collecton without eny elements. \p loc can be invalid
   /// or an approximate location of where an element of the collection would be
   /// if not missing.
   ParsedRawSyntaxNode recordEmptyRawSyntaxCollection(syntax::SyntaxKind kind,

include/swift/Parse/SyntaxParseActions.h

@@ -58,7 +58,7 @@ class SyntaxParseActions {
 
   /// Used for incremental re-parsing.
   virtual std::pair<size_t, OpaqueSyntaxNode>
-      lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) {
+  lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) {
     return std::make_pair(0, nullptr);
   }
 };

include/swift/Parse/SyntaxParsingContext.h

@@ -43,7 +43,16 @@ enum class SyntaxContextKind {
 };
 
 enum class SyntaxNodeCreationKind {
+  /// This is for \c SyntaxParsingContext to collect the syntax data and create
+  /// a 'recorded' ParsedRawSyntaxNode object, which would be a result of
+  /// passing the index data to the \c SyntaxParseActions implementation.
   Recorded,
+  /// This is for \c SyntaxParsingContext to collect the syntax data and create
+  /// a 'deferred' ParsedRawSyntaxNode object, which captures the data for a
+  /// \c SyntaxParseActions invocation to occur later.
+  ///
+  /// This is intended to be used for when it's not clear what will be the final
+  /// syntax node in the current parsing context.
   Deferred,
 };
 

include/swift/SyntaxParse/SyntaxTreeCreator.h

@@ -62,7 +62,7 @@ class SyntaxTreeCreator: public SyntaxParseActions {
                                    CharSourceRange range) override;
 
   std::pair<size_t, OpaqueSyntaxNode>
-      lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) override;
+  lookupNode(size_t lexerOffset, syntax::SyntaxKind kind) override;
 };
 
 } // end namespace swift

lib/Parse/ParsedRawSyntaxRecorder.cpp

@@ -52,7 +52,7 @@ getRecordedNode(const ParsedRawSyntaxNode &node, ParsedRawSyntaxRecorder &rec) {
   if (node.isRecorded())
     return node;
   if (node.isDeferredLayout())
-    return rec.recordExactRawSyntax(node.getKind(), node.getDeferredChildren());
+    return rec.recordRawSyntax(node.getKind(), node.getDeferredChildren());
   assert(node.isDeferredToken());
   const Token &tok = node.getToken();
   if (node.isMissing())
@@ -62,7 +62,7 @@ getRecordedNode(const ParsedRawSyntaxNode &node, ParsedRawSyntaxRecorder &rec) {
 }
 
 ParsedRawSyntaxNode
-ParsedRawSyntaxRecorder::recordExactRawSyntax(SyntaxKind kind,
+ParsedRawSyntaxRecorder::recordRawSyntax(SyntaxKind kind,
                                     ArrayRef<ParsedRawSyntaxNode> elements) {
   CharSourceRange range;
   SmallVector<OpaqueSyntaxNode, 16> subnodes;

lib/Parse/ParsedSyntaxBuilders.cpp.gyb

@@ -55,8 +55,7 @@ Parsed${node.name}Builder::add${child_elt}(Parsed${child_elt_type} ${child_elt})
 Parsed${node.name}
 Parsed${node.name}Builder::record() {
   finishLayout();
-  auto raw = Rec.recordExactRawSyntax(SyntaxKind::${node.syntax_kind},
-    Layout);
+  auto raw = Rec.recordRawSyntax(SyntaxKind::${node.syntax_kind}, Layout);
   return Parsed${node.name}(std::move(raw));
 }
 

lib/Parse/ParsedSyntaxRecorder.cpp.gyb

@@ -85,7 +85,7 @@ bool ParsedSyntaxRecorder::formExactLayoutFor(syntax::SyntaxKind Kind,
 Parsed${node.name}
 ParsedSyntaxRecorder::record${node.syntax_kind}(${child_params},
                                        ParsedRawSyntaxRecorder &rec) {
-  auto raw = rec.recordExactRawSyntax(SyntaxKind::${node.syntax_kind}, {
+  auto raw = rec.recordRawSyntax(SyntaxKind::${node.syntax_kind}, {
 %     for child in node.children:
 %       if child.is_optional:
     ${child.name}.hasValue() ? ${child.name}->getRaw() : ParsedRawSyntaxNode::null(),
@@ -129,8 +129,7 @@ ParsedSyntaxRecorder::record${node.syntax_kind}(
   for (auto &element : elements) {
     layout.push_back(element.getRaw());
   }
-  auto raw = rec.recordExactRawSyntax(SyntaxKind::${node.syntax_kind},
-                             layout);
+  auto raw = rec.recordRawSyntax(SyntaxKind::${node.syntax_kind}, layout);
   return Parsed${node.name}(std::move(raw));
 }
 

lib/Parse/SyntaxParsingContext.cpp

@@ -69,7 +69,7 @@ SyntaxParsingContext::makeUnknownSyntax(SyntaxKind Kind,
   if (IsBacktracking)
     return ParsedRawSyntaxNode::makeDeferred(Kind, Parts);
   else
-    return getRecorder().recordExactRawSyntax(Kind, Parts);
+    return getRecorder().recordRawSyntax(Kind, Parts);
 }
 
 ParsedRawSyntaxNode
@@ -83,7 +83,7 @@ SyntaxParsingContext::createSyntaxAs(SyntaxKind Kind,
     if (nodeCreateK == SyntaxNodeCreationKind::Deferred || IsBacktracking) {
       rawNode = ParsedRawSyntaxNode::makeDeferred(kind, layout);
     } else {
-      rawNode = rec.recordExactRawSyntax(kind, layout);
+      rawNode = rec.recordRawSyntax(kind, layout);
     }
   };
   if (ParsedSyntaxRecorder::formExactLayoutFor(Kind, Parts, formNode))
@@ -279,10 +279,10 @@ static ParsedRawSyntaxNode finalizeSourceFile(RootContextData &RootData,
     AllTopLevel.push_back(RawNode);
   }
 
-  auto itemList = Recorder.recordExactRawSyntax(SyntaxKind::CodeBlockItemList,
-                                                AllTopLevel);
-  auto root = Recorder.recordExactRawSyntax(SyntaxKind::SourceFile,
-                                            { itemList, EOFToken });
+  auto itemList = Recorder.recordRawSyntax(SyntaxKind::CodeBlockItemList,
+                                           AllTopLevel);
+  auto root = Recorder.recordRawSyntax(SyntaxKind::SourceFile,
+                                       { itemList, EOFToken });
   return root;
 }