[clang][TableGen] Change ASTTableGen to use const Record pointers

[jurahul]

Change ASTTableGen to use const Record pointers.

This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089

[llvmbot]

@llvm/pr-subscribers-clang

Author: Rahul Joshi (jurahul)

Changes

Change ASTTableGen to use const Record pointers.

This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089

---

Full diff: https://github.com/llvm/llvm-project/pull/108193.diff

2 Files Affected:

• (modified) clang/utils/TableGen/ASTTableGen.cpp (+8-11)
• (modified) clang/utils/TableGen/ASTTableGen.h (+19-18)

diff --git a/clang/utils/TableGen/ASTTableGen.cpp b/clang/utils/TableGen/ASTTableGen.cpp
index 54288ff6a03be3..47344777e9311a 100644
--- a/clang/utils/TableGen/ASTTableGen.cpp
+++ b/clang/utils/TableGen/ASTTableGen.cpp
@@ -31,7 +31,8 @@ llvm::StringRef clang::tblgen::HasProperties::getName() const {
   }
 }
 
-static StringRef removeExpectedNodeNameSuffix(Record *node, StringRef suffix) {
+static StringRef removeExpectedNodeNameSuffix(const Record *node,
+                                              StringRef suffix) {
   StringRef nodeName = node->getName();
   if (!nodeName.ends_with(suffix)) {
     PrintFatalError(node->getLoc(),
@@ -105,8 +106,7 @@ static void visitASTNodeRecursive(ASTNode node, ASTNode base,
   }
 }
 
-static void visitHierarchy(RecordKeeper &records,
-                           StringRef nodeClassName,
+static void visitHierarchy(const RecordKeeper &records, StringRef nodeClassName,
                            ASTNodeHierarchyVisitor<ASTNode> visit) {
   // Check for the node class, just as a basic correctness check.
   if (!records.getClass(nodeClassName)) {
@@ -114,13 +114,10 @@ static void visitHierarchy(RecordKeeper &records,
                       + nodeClassName);
   }
 
-  // Find all the nodes in the hierarchy.
-  auto nodes = records.getAllDerivedDefinitions(nodeClassName);
-
-  // Derive the child map.
+  // Derive the child map for all nodes in the hierarchy.
   ChildMap hierarchy;
   ASTNode root;
-  for (ASTNode node : nodes) {
+  for (ASTNode node : records.getAllDerivedDefinitions(nodeClassName)) {
     if (auto base = node.getBase())
       hierarchy.insert(std::make_pair(base, node));
     else if (root)
@@ -136,8 +133,8 @@ static void visitHierarchy(RecordKeeper &records,
   visitASTNodeRecursive(root, ASTNode(), hierarchy, visit);
 }
 
-void clang::tblgen::visitASTNodeHierarchyImpl(RecordKeeper &records,
-                                              StringRef nodeClassName,
-                                      ASTNodeHierarchyVisitor<ASTNode> visit) {
+void clang::tblgen::visitASTNodeHierarchyImpl(
+    const RecordKeeper &records, StringRef nodeClassName,
+    ASTNodeHierarchyVisitor<ASTNode> visit) {
   visitHierarchy(records, nodeClassName, visit);
 }
diff --git a/clang/utils/TableGen/ASTTableGen.h b/clang/utils/TableGen/ASTTableGen.h
index 41f78a6a3bbcdd..143d779a8a64f8 100644
--- a/clang/utils/TableGen/ASTTableGen.h
+++ b/clang/utils/TableGen/ASTTableGen.h
@@ -87,18 +87,18 @@ namespace clang {
 namespace tblgen {
 
 class WrappedRecord {
-  llvm::Record *Record;
+  const llvm::Record *Record;
 
 protected:
-  WrappedRecord(llvm::Record *record = nullptr) : Record(record) {}
+  WrappedRecord(const llvm::Record *record = nullptr) : Record(record) {}
 
-  llvm::Record *get() const {
+  const llvm::Record *get() const {
     assert(Record && "accessing null record");
     return Record;
   }
 
 public:
-  llvm::Record *getRecord() const { return Record; }
+  const llvm::Record *getRecord() const { return Record; }
 
   explicit operator bool() const { return Record != nullptr; }
 
@@ -144,7 +144,7 @@ class HasProperties : public WrappedRecord {
 public:
   static constexpr llvm::StringRef ClassName = HasPropertiesClassName;
 
-  HasProperties(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  HasProperties(const llvm::Record *record = nullptr) : WrappedRecord(record) {}
 
   llvm::StringRef getName() const;
 
@@ -157,7 +157,7 @@ class HasProperties : public WrappedRecord {
 /// in one of Clang's AST hierarchies.
 class ASTNode : public HasProperties {
 public:
-  ASTNode(llvm::Record *record = nullptr) : HasProperties(record) {}
+  ASTNode(const llvm::Record *record = nullptr) : HasProperties(record) {}
 
   llvm::StringRef getName() const {
     return get()->getName();
@@ -180,7 +180,7 @@ class ASTNode : public HasProperties {
 
 class DeclNode : public ASTNode {
 public:
-  DeclNode(llvm::Record *record = nullptr) : ASTNode(record) {}
+  DeclNode(const llvm::Record *record = nullptr) : ASTNode(record) {}
 
   llvm::StringRef getId() const;
   std::string getClassName() const;
@@ -202,7 +202,7 @@ class DeclNode : public ASTNode {
 
 class TypeNode : public ASTNode {
 public:
-  TypeNode(llvm::Record *record = nullptr) : ASTNode(record) {}
+  TypeNode(const llvm::Record *record = nullptr) : ASTNode(record) {}
 
   llvm::StringRef getId() const;
   llvm::StringRef getClassName() const;
@@ -224,7 +224,7 @@ class TypeNode : public ASTNode {
 
 class StmtNode : public ASTNode {
 public:
-  StmtNode(llvm::Record *record = nullptr) : ASTNode(record) {}
+  StmtNode(const llvm::Record *record = nullptr) : ASTNode(record) {}
 
   std::string getId() const;
   llvm::StringRef getClassName() const;
@@ -247,7 +247,7 @@ class StmtNode : public ASTNode {
 /// The type of a property.
 class PropertyType : public WrappedRecord {
 public:
-  PropertyType(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  PropertyType(const llvm::Record *record = nullptr) : WrappedRecord(record) {}
 
   /// Is this a generic specialization (i.e. `Array<T>` or `Optional<T>`)?
   bool isGenericSpecialization() const {
@@ -331,7 +331,7 @@ class PropertyType : public WrappedRecord {
 /// A rule for returning the kind of a type.
 class TypeKindRule : public WrappedRecord {
 public:
-  TypeKindRule(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  TypeKindRule(const llvm::Record *record = nullptr) : WrappedRecord(record) {}
 
   /// Return the type to which this applies.
   PropertyType getParentType() const {
@@ -361,7 +361,7 @@ class TypeKindRule : public WrappedRecord {
 /// An implementation case of a property type.
 class TypeCase : public HasProperties {
 public:
-  TypeCase(llvm::Record *record = nullptr) : HasProperties(record) {}
+  TypeCase(const llvm::Record *record = nullptr) : HasProperties(record) {}
 
   /// Return the name of this case.
   llvm::StringRef getCaseName() const {
@@ -381,7 +381,7 @@ class TypeCase : public HasProperties {
 /// A property of an AST node.
 class Property : public WrappedRecord {
 public:
-  Property(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  Property(const llvm::Record *record = nullptr) : WrappedRecord(record) {}
 
   /// Return the name of this property.
   llvm::StringRef getName() const {
@@ -417,7 +417,8 @@ class Property : public WrappedRecord {
 /// a value (which is actually done when writing the value out).
 class ReadHelperRule : public WrappedRecord {
 public:
-  ReadHelperRule(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  ReadHelperRule(const llvm::Record *record = nullptr)
+      : WrappedRecord(record) {}
 
   /// Return the class for which this is a creation rule.
   /// Should never be abstract.
@@ -437,7 +438,7 @@ class ReadHelperRule : public WrappedRecord {
 /// A rule for how to create an AST node from its properties.
 class CreationRule : public WrappedRecord {
 public:
-  CreationRule(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  CreationRule(const llvm::Record *record = nullptr) : WrappedRecord(record) {}
 
   /// Return the class for which this is a creation rule.
   /// Should never be abstract.
@@ -457,7 +458,7 @@ class CreationRule : public WrappedRecord {
 /// A rule which overrides the standard rules for serializing an AST node.
 class OverrideRule : public WrappedRecord {
 public:
-  OverrideRule(llvm::Record *record = nullptr) : WrappedRecord(record) {}
+  OverrideRule(const llvm::Record *record = nullptr) : WrappedRecord(record) {}
 
   /// Return the class for which this is an override rule.
   /// Should never be abstract.
@@ -483,12 +484,12 @@ template <class NodeClass>
 using ASTNodeHierarchyVisitor =
   llvm::function_ref<void(NodeClass node, NodeClass base)>;
 
-void visitASTNodeHierarchyImpl(llvm::RecordKeeper &records,
+void visitASTNodeHierarchyImpl(const llvm::RecordKeeper &records,
                                llvm::StringRef nodeClassName,
                                ASTNodeHierarchyVisitor<ASTNode> visit);
 
 template <class NodeClass>
-void visitASTNodeHierarchy(llvm::RecordKeeper &records,
+void visitASTNodeHierarchy(const llvm::RecordKeeper &records,
                            ASTNodeHierarchyVisitor<NodeClass> visit) {
   visitASTNodeHierarchyImpl(records, NodeClass::getTableGenNodeClassName(),
                             [visit](ASTNode node, ASTNode base) {

[jurahul]

@AaronBallman I am adding you as a reviewer, but if there is someone else who can help review these, please let me know.

[AaronBallman]

LGTM!