Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Sep 13, 2024

Change SyntaxEmitter to use const RecordKeeper.

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

@jurahul jurahul marked this pull request as ready for review September 13, 2024 03:47
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Sep 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 13, 2024

@llvm/pr-subscribers-clang

Author: Rahul Joshi (jurahul)

Changes

Change SyntaxEmitter to use const RecordKeeper.

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/108478.diff

2 Files Affected:

  • (modified) clang/utils/TableGen/ClangSyntaxEmitter.cpp (+7-6)
  • (modified) clang/utils/TableGen/TableGenBackends.h (+2-2)
diff --git a/clang/utils/TableGen/ClangSyntaxEmitter.cpp b/clang/utils/TableGen/ClangSyntaxEmitter.cpp
index 2a69e4c353b6b4..66b27be88f56f5 100644
--- a/clang/utils/TableGen/ClangSyntaxEmitter.cpp
+++ b/clang/utils/TableGen/ClangSyntaxEmitter.cpp
@@ -41,11 +41,12 @@ using llvm::formatv;
 // stable and useful way, where abstract Node subclasses correspond to ranges.
 class Hierarchy {
 public:
-  Hierarchy(llvm::RecordKeeper &Records) {
-    for (llvm::Record *T : Records.getAllDerivedDefinitions("NodeType"))
+  Hierarchy(const llvm::RecordKeeper &Records) {
+    for (const llvm::Record *T : Records.getAllDerivedDefinitions("NodeType"))
       add(T);
-    for (llvm::Record *Derived : Records.getAllDerivedDefinitions("NodeType"))
-      if (llvm::Record *Base = Derived->getValueAsOptionalDef("base"))
+    for (const llvm::Record *Derived :
+         Records.getAllDerivedDefinitions("NodeType"))
+      if (const llvm::Record *Base = Derived->getValueAsOptionalDef("base"))
         link(Derived, Base);
     for (NodeType &N : AllTypes) {
       llvm::sort(N.Derived, [](const NodeType *L, const NodeType *R) {
@@ -127,7 +128,7 @@ struct SyntaxConstraint {
 
 } // namespace
 
-void clang::EmitClangSyntaxNodeList(llvm::RecordKeeper &Records,
+void clang::EmitClangSyntaxNodeList(const llvm::RecordKeeper &Records,
                                     llvm::raw_ostream &OS) {
   llvm::emitSourceFileHeader("Syntax tree node list", OS, Records);
   Hierarchy H(Records);
@@ -186,7 +187,7 @@ static void printDoc(llvm::StringRef Doc, llvm::raw_ostream &OS) {
   }
 }
 
-void clang::EmitClangSyntaxNodeClasses(llvm::RecordKeeper &Records,
+void clang::EmitClangSyntaxNodeClasses(const llvm::RecordKeeper &Records,
                                        llvm::raw_ostream &OS) {
   llvm::emitSourceFileHeader("Syntax tree node list", OS, Records);
   Hierarchy H(Records);
diff --git a/clang/utils/TableGen/TableGenBackends.h b/clang/utils/TableGen/TableGenBackends.h
index 01d16d2dc3e5f1..881e9a8c5eec01 100644
--- a/clang/utils/TableGen/TableGenBackends.h
+++ b/clang/utils/TableGen/TableGenBackends.h
@@ -108,9 +108,9 @@ void EmitClangCommentCommandList(const llvm::RecordKeeper &Records,
                                  llvm::raw_ostream &OS);
 void EmitClangOpcodes(const llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
-void EmitClangSyntaxNodeList(llvm::RecordKeeper &Records,
+void EmitClangSyntaxNodeList(const llvm::RecordKeeper &Records,
                              llvm::raw_ostream &OS);
-void EmitClangSyntaxNodeClasses(llvm::RecordKeeper &Records,
+void EmitClangSyntaxNodeClasses(const llvm::RecordKeeper &Records,
                                 llvm::raw_ostream &OS);
 
 void EmitNeon(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jurahul jurahul merged commit d757bbf into llvm:main Sep 13, 2024
@jurahul jurahul deleted the clang_syntax_emitter_const_record branch September 13, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants