Adjust uAST serializer to store children after subclass fields (chape…

…l-lang#23708)

Continuation of PR #chapel-lang#23697.

This PR adjusts the uAST serializer/deserializers to store the children
after the subclass fields implementing a uAST node. Storing the children
after the subclass fields is important to preserve the property that
each uAST node is contiguous (other than its children). In my view, that
is important for the file format to be possible for a person to debug.

However, the way that the `serialize` functions were implemented
prevented this reordering because 1) `serialize` was the public
interface into these and 2) the only chance `AstNode` has to serialize
the children is in `AstNode::serialize` and that's necessarily called
before serializing the fields of a subclass.

To address that, this PR changes the strategy to differentiate between
methods a subclass must provide and the methods for the public
interface:
* `AstNode::serialize` and `AstNode::deserialize` form the public
interface
* subclasses need to override `serializeInner(Serializer& ser)` and
implement a constructor like `SubClass(Deserializer& des)`
* `AstNode::serialize` calls `serializeInner` at the appropriate time.
Similarly, `AstNode::deserialize` calls the constructor and deserializes
the children at an appropriate time.

While there, I noticed that we can remove `DECLARE_STATIC_DESERIALIZE`
by having `AstNode::deserialize` simply call `new NAME(des)` for each
subclass (according to the tag). Also, since this constructor should not
be part of the public API, I enabled it to be `private` / `protected` by
making each uAST subclass include `friend class AstNode`. That reduces
some of the switching between `public:` and `private:` in these uAST
class definitions which appeared in PR chapel-lang#23697.

This PR also changes the abstract uAST types to include their name in
the method to be called by subclasses in `serializeInner`: e.g.,
`declSerializeInner`. This matches the pattern already used in
`contentsMatchInner` / `markUniqueStringsInner` with e.g.
`declContentsMatchInner` / `declMarkUniqueStringsInner`. While I think
it's important that these 3 use a consistent style, an alternative would
be to use the `::` form in all 3: e.g., `Decl::serializeInner` /
`Decl::contentsMatchInner` / `Decl::markUniqueStringsInner`.

Additionally, this PR updates the deserializing constructors to use
`explicit` to avoid implicit conversion from a deserializer.

Lastly, this PR adds a C++ test of the uAST serialization and
deserialization. This adds to the existing testing in
`test/separate_compilation/serialization`.

Reviewed by @benharsh - thanks!

- [x] full comm=none testing

frontend/include/chpl/uast/AggregateDecl.h

@@ -42,6 +42,8 @@ namespace uast {
 
  */
 class AggregateDecl : public TypeDecl {
+ friend class AstNode;
+
  private:
   int inheritExprChildNum_;
   int numInheritExprs_;
@@ -67,7 +69,6 @@ class AggregateDecl : public TypeDecl {
 
   std::string aggregateDeclDumpChildLabelInner(int i) const;
 
- public:
   AggregateDecl(AstTag tag, AstList children, int attributeGroupChildNum,
                 Decl::Visibility vis,
                 Decl::Linkage linkage,
@@ -99,8 +100,8 @@ class AggregateDecl : public TypeDecl {
     CHPL_ASSERT(validAggregateChildren(declOrComments()));
   }
 
-  void serialize(Serializer& ser) const override {
-    TypeDecl::serialize(ser);
+  void aggregateDeclSerializeInner(Serializer& ser) const {
+    typeDeclSerializeInner(ser);
     ser.writeVInt(inheritExprChildNum_);
     ser.writeVInt(numInheritExprs_);
     ser.writeVInt(elementsChildNum_);
@@ -117,6 +118,7 @@ class AggregateDecl : public TypeDecl {
 
   ~AggregateDecl() = 0; // this is an abstract base class
 
+ public:
   /**
     Return a way to iterate over the contained Decls and Comments.
    */

frontend/include/chpl/uast/AnonFormal.h

@@ -63,6 +63,8 @@ namespace uast {
   They also do not carry an initialization expression.
 */
 class AnonFormal final : public AstNode {
+ friend class AstNode;
+
  public:
   using Intent = Formal::Intent;
 
@@ -77,21 +79,16 @@ class AnonFormal final : public AstNode {
       typeExpressionChildNum_(typeExpressionChildNum) {
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    AstNode::serialize(ser);
+  void serializeInner(Serializer& ser) const override {
     ser.write(intent_);
     ser.write(typeExpressionChildNum_);
   }
 
-  DECLARE_STATIC_DESERIALIZE(AnonFormal);
-
- private:
-  AnonFormal(Deserializer& des)
+  explicit AnonFormal(Deserializer& des)
     : AstNode(asttags::AnonFormal, des) {
-      intent_ = des.read<Formal::Intent>();
-      typeExpressionChildNum_ = des.read<int8_t>();
-    }
+    intent_ = des.read<Formal::Intent>();
+    typeExpressionChildNum_ = des.read<int8_t>();
+  }
 
   bool contentsMatchInner(const AstNode* other) const override {
     const AnonFormal* lhs = this;
@@ -100,7 +97,7 @@ class AnonFormal final : public AstNode {
            lhs->typeExpressionChildNum_ == rhs->typeExpressionChildNum_;
   }
 
-  void markUniqueStringsInner(Context* context) const override {}
+  void markUniqueStringsInner(Context* context) const override { }
 
   void dumpInner(const DumpSettings& s) const;
 

frontend/include/chpl/uast/Array.h

@@ -41,8 +41,9 @@ namespace uast {
   An array expression will never contain comments.
  */
 class Array final : public AstNode {
- private:
+ friend class AstNode;
 
+ private:
   bool trailingComma_,
        associative_;
 
@@ -52,17 +53,12 @@ class Array final : public AstNode {
     associative_ = associative;
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    AstNode::serialize(ser);
+  void serializeInner(Serializer& ser) const override {
     ser.write(trailingComma_);
     ser.write(associative_);
   }
 
-  DECLARE_STATIC_DESERIALIZE(Array);
-
- private:
-  Array(Deserializer& des)
+  explicit Array(Deserializer& des)
     : AstNode(asttags::Array, des) {
     trailingComma_ = des.read<bool>();
     associative_ = des.read<bool>();

frontend/include/chpl/uast/As.h

@@ -41,25 +41,21 @@ namespace uast {
   \endrst
  */
 class As final : public AstNode {
+ friend class AstNode;
+
  private:
   As(AstList children) : AstNode(asttags::As, std::move(children)) { }
 
- public:
-  void serialize(Serializer& ser) const override {
-    AstNode::serialize(ser);
-  }
-  DECLARE_STATIC_DESERIALIZE(As);
+  void serializeInner(Serializer& ser) const override { }
 
- private:
-  As(Deserializer& des) : AstNode(asttags::As, des) { }
+  explicit As(Deserializer& des) : AstNode(asttags::As, des) { }
 
   // No need to match 'symbolChildNum_' or 'renameChildNum_', they are const.
   bool contentsMatchInner(const AstNode* other) const override {
     return true;
   }
 
-  void markUniqueStringsInner(Context* context) const override {
-  }
+  void markUniqueStringsInner(Context* context) const override { }
 
   std::string dumpChildLabelInner(int i) const override;
 

frontend/include/chpl/uast/AstNode.h

@@ -79,6 +79,15 @@ class AstNode {
    */
   virtual void markUniqueStringsInner(Context* context) const = 0;
 
+  /**
+   This function needs to be defined by subclasses.
+   It should serialize any fields in the subclasses.
+   It need not concern itself with AstNode's fields, including the children.
+   Note that subclasses also need to provide a constructor
+   accepting AstTag, Deserializer.
+   */
+  virtual void serializeInner(Serializer& os) const = 0;
+
   struct DumpSettings {
     chpl::StringifyKind kind = StringifyKind::DEBUG_DETAIL;
     bool printId = true;
@@ -125,8 +134,14 @@ class AstNode {
     }
   }
 
+  /** To be called by subclasses to set the tag and
+      deserialize the AstNode fields other than the children */
   AstNode(AstTag tag, Deserializer& des);
 
+  /** Completes the deserialization process for an AstNode
+      by deserializing the children. */
+  void deserializeChildren(Deserializer& des);
+
   // Quick way to return an already exhausted iterator.
   template <typename T>
   AstListIteratorPair<T> emptyIterator() const {
@@ -277,8 +292,7 @@ class AstNode {
   // compute the maximum width of all of the IDs
   int computeMaxIdStringWidth() const;
 
-  virtual void serialize(Serializer& os) const;
-
+  void serialize(Serializer& ser) const;
   static owned<AstNode> deserialize(Deserializer& des);
 
   /// \cond DO_NOT_DOCUMENT
@@ -533,7 +547,7 @@ class AstNode {
 
 template<> struct serialize<uast::AstList> {
   void operator()(Serializer& ser, const uast::AstList& list) {
-    ser.write((uint64_t)list.size());
+    ser.writeVU64(list.size());
     for (const auto& node : list) {
       node->serialize(ser);
     }
@@ -543,7 +557,7 @@ template<> struct serialize<uast::AstList> {
 template<> struct deserialize<uast::AstList> {
   uast::AstList operator()(Deserializer& des) {
     uast::AstList ret;
-    auto len = des.read<uint64_t>();
+    uint64_t len = des.readVU64();
     for (uint64_t i = 0; i < len; i++) {
       ret.push_back(uast::AstNode::deserialize(des));
     }
@@ -587,11 +601,6 @@ AST_LESS(AstNode)
 #undef AST_LESS
 /// \endcond
 
-#define DECLARE_STATIC_DESERIALIZE(NAME) \
-static owned<NAME> deserialize(Deserializer& des) { \
-  return owned<NAME>(new NAME(des)); \
-}
-
 } // end namespace std
 
 #endif

frontend/include/chpl/uast/Attribute.h

@@ -28,6 +28,7 @@ namespace chpl {
 namespace uast {
 
 class Attribute final: public AstNode {
+ friend class AstNode;
 
  private:
   // the attribute name - deprecated or unstable or chpldoc.nodoc, for example
@@ -43,19 +44,13 @@ class Attribute final: public AstNode {
       actualNames_(std::move(actualNames)) {
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    AstNode::serialize(ser);
-
+  void serializeInner(Serializer& ser) const override {
     ser.write(name_);
     ser.writeVInt(numActuals_);
     ser.write(actualNames_);
   }
 
-  DECLARE_STATIC_DESERIALIZE(Attribute);
-
- private:
-  Attribute(Deserializer& des) : AstNode(asttags::Attribute, des) {
+  explicit Attribute(Deserializer& des) : AstNode(asttags::Attribute, des) {
     name_ = des.read<UniqueString>();
     numActuals_ = des.readVInt();
     actualNames_ = des.read<std::vector<UniqueString>>();

frontend/include/chpl/uast/AttributeGroup.h

@@ -38,6 +38,8 @@ namespace uast {
   use only) are both examples of attributes.
 */
 class AttributeGroup final : public AstNode {
+ friend class AstNode;
+
  private:
   std::set<PragmaTag> pragmas_;
   bool isDeprecated_;
@@ -108,10 +110,7 @@ class AttributeGroup final : public AstNode {
     CHPL_ASSERT(pragmas_.size() <= NUM_KNOWN_PRAGMAS);
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    AstNode::serialize(ser);
-
+  void serializeInner(Serializer& ser) const override {
     ser.write(pragmas_);
     ser.write(isDeprecated_);
     ser.write(isUnstable_);
@@ -121,20 +120,16 @@ class AttributeGroup final : public AstNode {
     ser.write(parenfulDeprecationMessage_);
   }
 
-  DECLARE_STATIC_DESERIALIZE(AttributeGroup);
-
- private:
-  AttributeGroup(Deserializer& des)
+  explicit AttributeGroup(Deserializer& des)
     : AstNode(asttags::AttributeGroup, des) {
-      pragmas_ = des.read<std::set<PragmaTag>>();
-      isDeprecated_ = des.read<bool>();
-      isUnstable_ = des.read<bool>();
-      isParenfulDeprecated_ = des.read<bool>();
-      deprecationMessage_ = des.read<UniqueString>();
-      unstableMessage_ = des.read<UniqueString>();
-      parenfulDeprecationMessage_ = des.read<UniqueString>();
-    }
-
+    pragmas_ = des.read<std::set<PragmaTag>>();
+    isDeprecated_ = des.read<bool>();
+    isUnstable_ = des.read<bool>();
+    isParenfulDeprecated_ = des.read<bool>();
+    deprecationMessage_ = des.read<UniqueString>();
+    unstableMessage_ = des.read<UniqueString>();
+    parenfulDeprecationMessage_ = des.read<UniqueString>();
+  }
 
   bool contentsMatchInner(const AstNode* other) const override {
     const AttributeGroup* rhs = (const AttributeGroup*)other;

frontend/include/chpl/uast/Begin.h

@@ -46,6 +46,8 @@ namespace uast {
 
  */
 class Begin final : public SimpleBlockLike {
+ friend class AstNode;
+
  private:
   int8_t withClauseChildNum_;
 
@@ -58,17 +60,12 @@ class Begin final : public SimpleBlockLike {
       withClauseChildNum_(withClauseChildNum) {
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    SimpleBlockLike::serialize(ser);
+  void serializeInner(Serializer& ser) const override {
     ser.write(withClauseChildNum_);
   }
 
-  DECLARE_STATIC_DESERIALIZE(Begin);
-
- private:
-  Begin(Deserializer& des)
-  : SimpleBlockLike(asttags::Begin, des) {
+  explicit Begin(Deserializer& des)
+    : SimpleBlockLike(asttags::Begin, des) {
     withClauseChildNum_ = des.read<int8_t>();
   }
 

frontend/include/chpl/uast/Block.h

@@ -32,6 +32,8 @@ namespace uast {
   This class represents a { } block.
  */
 class Block final : public SimpleBlockLike {
+ friend class AstNode;
+
  private:
   Block(AstList stmts, int bodyChildNum, int numBodyStmts)
     : SimpleBlockLike(asttags::Block, std::move(stmts),
@@ -42,16 +44,11 @@ class Block final : public SimpleBlockLike {
     CHPL_ASSERT(bodyChildNum_ >= 0);
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    SimpleBlockLike::serialize(ser);
+  void serializeInner(Serializer& ser) const override {
+    simpleBlockLikeSerializeInner(ser);
   }
 
-  DECLARE_STATIC_DESERIALIZE(Block);
-
- private:
-  Block(Deserializer& des)
-    : SimpleBlockLike(asttags::Block, des) { }
+  explicit Block(Deserializer& des) : SimpleBlockLike(asttags::Block, des) { }
 
   bool contentsMatchInner(const AstNode* other) const override {
     return simpleBlockLikeContentsMatchInner(other);

frontend/include/chpl/uast/BoolLiteral.h

@@ -32,20 +32,18 @@ namespace uast {
   This class represents a boolean literal.
  */
 class BoolLiteral final : public Literal {
+ friend class AstNode;
+
  private:
   explicit BoolLiteral(const types::BoolParam* value)
     : Literal(asttags::BoolLiteral, value) {
   }
 
- public:
-  void serialize(Serializer& ser) const override {
-    Literal::serialize(ser);
+  void serializeInner(Serializer& ser) const override {
+    literalSerializeInner(ser);
   }
 
-  DECLARE_STATIC_DESERIALIZE(BoolLiteral);
-
- private:
-  BoolLiteral(Deserializer& des)
+  explicit BoolLiteral(Deserializer& des)
     : Literal(asttags::BoolLiteral, des) {
     assert(value_->isBoolParam());
   }