Move Datatype management to ExprManager (#3568)

This is further work towards decoupling the Expr layer from the Node layer.

This commit makes it so that ExprManager does memory management for Datatype while NodeManager maintains a list of DType.

As a reminder, the ownership policy (and level of indirection through DatatypeIndex) is necessary due to not being able to store Datatype within Node since this leads to circular dependencies in the Node AST.

1 files changed, 12 insertions, 4 deletions

diff --git a/src/expr/node_manager.h b/src/expr/node_manager.h
index 61ef1d39d..eced00c48 100644
--- a/src/expr/node_manager.h
+++ b/src/expr/node_manager.h
@@ -178,7 +178,7 @@ class NodeManager {
   std::vector<NodeManagerListener*> d_listeners;
 
   /** A list of datatypes owned by this node manager. */
-  std::vector<Datatype*> d_ownedDatatypes;
+  std::vector<std::shared_ptr<DType> > d_ownedDTypes;
 
   /**
    * A map of tuple and record types to their corresponding datatype.
@@ -428,9 +428,17 @@ public:
   }
   
   /** register datatype */
-  unsigned registerDatatype(Datatype* dt);
-  /** get datatype for index */
-  const Datatype & getDatatypeForIndex( unsigned index ) const;
+  size_t registerDatatype(std::shared_ptr<DType> dt);
+  /**
+   * Return the datatype at the given index owned by this class. Type nodes are
+   * associated with datatypes through the DatatypeIndexConstant class. The
+   * argument index is intended to be a value taken from that class.
+   *
+   * Type nodes must access their DTypes through a level of indirection to
+   * prevent cycles in the Node AST (as DTypes themselves contain Nodes), which
+   * would lead to memory leaks. Thus TypeNode are given a DatatypeIndexConstant
+   * which is used as an index to retrieve the DType via this call.
+   */
   const DType& getDTypeForIndex(unsigned index) const;
 
   /** Get a Kind from an operator expression */