Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 4, 2025

Change NamespaceEmitter to emit nested namespace using C++17 nested namespace definitions.

@jurahul jurahul changed the title [NFC][TableGen] Use C++17 nested namespace definitions [NFC][TableGen] Emit C++17 nested namespace definitions in NamespaceEmitter Oct 4, 2025
@jurahul jurahul changed the title [NFC][TableGen] Emit C++17 nested namespace definitions in NamespaceEmitter [NFC][TableGen] Emit nested namespace definitions in NamespaceEmitter Oct 4, 2025
@jurahul jurahul marked this pull request as ready for review October 4, 2025 23:07
@llvmbot llvmbot added the mlir label Oct 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2025

@llvm/pr-subscribers-mlir

Author: Rahul Joshi (jurahul)

Changes

Change NamespaceEmitter to emit nested namespace using C++17 nested namespace definitions.


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

2 Files Affected:

  • (modified) llvm/include/llvm/TableGen/CodeGenHelpers.h (+12-11)
  • (modified) mlir/test/mlir-tblgen/dialect.td (+5)
diff --git a/llvm/include/llvm/TableGen/CodeGenHelpers.h b/llvm/include/llvm/TableGen/CodeGenHelpers.h
index 7dca6a051ba75..f9629236b9dfd 100644
--- a/llvm/include/llvm/TableGen/CodeGenHelpers.h
+++ b/llvm/include/llvm/TableGen/CodeGenHelpers.h
@@ -38,28 +38,29 @@ class IfDefEmitter {
 // namespace (empty for anonymous namespace) or nested namespace.
 class NamespaceEmitter {
 public:
-  NamespaceEmitter(raw_ostream &OS, StringRef Name) : OS(OS) {
-    emitNamespaceStarts(Name);
+  NamespaceEmitter(raw_ostream &OS, StringRef Name)
+      : Name(trim(Name).str()), OS(OS) {
+    OS << "namespace " << this->Name << " {\n";
   }
 
   ~NamespaceEmitter() { close(); }
 
   // Explicit function to close the namespace scopes.
   void close() {
-    for (StringRef NS : llvm::reverse(Namespaces))
-      OS << "} // namespace " << NS << "\n";
-    Namespaces.clear();
+    if (!Closed)
+      OS << "} // namespace " << Name << "\n";
+    Closed = true;
   }
 
 private:
-  void emitNamespaceStarts(StringRef Name) {
-    llvm::SplitString(Name, Namespaces, "::");
-    for (StringRef NS : Namespaces)
-      OS << "namespace " << NS << " {\n";
+  // Trim "::" prefix.
+  static StringRef trim(StringRef Name) {
+    Name.consume_front("::");
+    return Name;
   }
-
-  SmallVector<StringRef, 2> Namespaces;
+  std::string Name;
   raw_ostream &OS;
+  bool Closed = false;
 };
 
 } // end namespace llvm
diff --git a/mlir/test/mlir-tblgen/dialect.td b/mlir/test/mlir-tblgen/dialect.td
index f35ce345a90a8..9b454959a1da9 100644
--- a/mlir/test/mlir-tblgen/dialect.td
+++ b/mlir/test/mlir-tblgen/dialect.td
@@ -62,9 +62,14 @@ def E_SpecialNSOp : Op<E_Dialect, "special_ns_op", []> {
 // DEF: ::E::SPECIAL_NS::SpecialNSOp definitions
 
 // DECL-LABEL: GET_OP_CLASSES
+// DECL: namespace a {
 // DECL: a::SomeOp declarations
+// DECL: namespace BNS {
 // DECL: BNS::SomeOp declarations
+// DECL: namespace C::CC {
 // DECL: ::C::CC::SomeOp declarations
 // DECL: DSomeOp declarations
+// DECL: namespace ENS {
 // DECL: ENS::SomeOp declarations
+// DECL: namespace E::SPECIAL_NS {
 // DECL: ::E::SPECIAL_NS::SpecialNSOp declarations

@jurahul
Copy link
Contributor Author

jurahul commented Oct 10, 2025

@joker-eph or @matthias-springer gentle ping

Change NamespaceEmitter to emit nested namespace using C++17
nested namespace definitions.
@jurahul jurahul merged commit dc365b2 into llvm:main Oct 13, 2025
10 checks passed
@jurahul jurahul deleted the nfc_ns_emitter branch October 13, 2025 21:08
akadutta pushed a commit to akadutta/llvm-project that referenced this pull request Oct 14, 2025
…llvm#161958)

Change NamespaceEmitter to emit nested namespace using C++17 nested
namespace definitions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants