[CIR] Refactor StructType printing/parsing

This change simplifies the StructType printing/parsing methods. It also
updates the representations to group members between braces and to
remove commas between type attributes (e.g. id, packed, etc.).

ghstack-source-id: 094d506
Pull Request resolved: #226

Author: sitio-couto

clang/include/clang/CIR/Dialect/IR/CIRTypes.td

@@ -113,18 +113,6 @@ def CIR_StructType : CIR_Type<"Struct", "struct",
     "std::optional<::mlir::cir::ASTRecordDeclAttr>":$ast
   );
 
-  let builders = [
-    TypeBuilder<(ins
-      "ArrayRef<mlir::Type>":$members, "StringRef":$typeName,
-      "bool":$body
-    ), [{
-      auto id = mlir::StringAttr::get(context, typeName);
-      auto sTy = StructType::get(context, members, id, body,
-                                 /*packed=*/false, std::nullopt);
-      return sTy;
-    }]>
-  ];
-
   let hasCustomAssemblyFormat = 1;
 
   let extraClassDeclaration = [{

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -14,6 +14,7 @@
 #include "clang/CIR/Dialect/IR/CIRAttrs.h"
 #include "clang/CIR/Dialect/IR/CIROpsEnums.h"
 #include "clang/CIR/Dialect/IR/CIRTypes.h"
+#include <optional>
 
 #include "mlir/Dialect/Func/IR/FuncOps.h"
 #include "mlir/Dialect/LLVMIR/LLVMTypes.h"
@@ -2104,9 +2105,10 @@ void SignedOverflowBehaviorAttr::print(::mlir::AsmPrinter &printer) const {
 
 ::mlir::Attribute ASTFunctionDeclAttr::parse(::mlir::AsmParser &parser,
                                              ::mlir::Type type) {
-  // We cannot really parse anything AST related at this point
-  // since we have no serialization/JSON story.
-  return ASTFunctionDeclAttr::get(parser.getContext(), nullptr);
+  // We cannot really parse anything AST related at this point since we have no
+  // serialization/JSON story. Even if the attr is parsed, it just holds nullptr
+  // instead of the AST node.
+  return get(parser.getContext(), nullptr);
 }
 
 void ASTFunctionDeclAttr::print(::mlir::AsmPrinter &printer) const {
@@ -2121,9 +2123,10 @@ LogicalResult ASTFunctionDeclAttr::verify(
 
 ::mlir::Attribute ASTVarDeclAttr::parse(::mlir::AsmParser &parser,
                                         ::mlir::Type type) {
-  // We cannot really parse anything AST related at this point
-  // since we have no serialization/JSON story.
-  return ASTVarDeclAttr::get(parser.getContext(), nullptr);
+  // We cannot really parse anything AST related at this point since we have no
+  // serialization/JSON story. Even if the attr is parsed, it just holds nullptr
+  // instead of the AST node.
+  return get(parser.getContext(), nullptr);
 }
 
 void ASTVarDeclAttr::print(::mlir::AsmPrinter &printer) const {
@@ -2138,9 +2141,10 @@ LogicalResult ASTVarDeclAttr::verify(
 
 ::mlir::Attribute ASTRecordDeclAttr::parse(::mlir::AsmParser &parser,
                                            ::mlir::Type type) {
-  // We cannot really parse anything AST related at this point
-  // since we have no serialization/JSON story.
-  return ASTRecordDeclAttr::get(parser.getContext(), nullptr);
+  // We cannot really parse anything AST related at this point since we have no
+  // serialization/JSON story. Even if the attr is parsed, it just holds nullptr
+  // instead of the AST node.
+  return get(parser.getContext(), nullptr);
 }
 
 void ASTRecordDeclAttr::print(::mlir::AsmPrinter &printer) const {

clang/lib/CIR/Dialect/IR/CIRTypes.cpp

@@ -11,9 +11,11 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/CIR/Dialect/IR/CIRTypes.h"
+#include "clang/CIR/Dialect/IR/CIRAttrs.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 
 #include "mlir/IR/Attributes.h"
+#include "mlir/IR/BuiltinAttributes.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/DialectImplementation.h"
 #include "mlir/Support/LogicalResult.h"
@@ -22,6 +24,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
+#include <optional>
 
 //===----------------------------------------------------------------------===//
 // CIR Custom Parser/Printer Signatures
@@ -87,72 +90,69 @@ Type BoolType::parse(mlir::AsmParser &parser) {
 
 void BoolType::print(mlir::AsmPrinter &printer) const {}
 
+//===----------------------------------------------------------------------===//
+// StructType Definitions
+//===----------------------------------------------------------------------===//
+
 Type StructType::parse(mlir::AsmParser &parser) {
+  llvm::SmallVector<mlir::Type> members;
+  mlir::StringAttr id;
+  bool body = false;
+  bool packed = false;
+  mlir::cir::ASTRecordDeclAttr ast = nullptr;
+
   if (parser.parseLess())
-    return Type();
-  std::string typeName;
-  if (parser.parseString(&typeName))
-    return Type();
+    return {};
 
-  llvm::SmallVector<Type> members;
-  bool parsedBody = false;
-
-  auto parseASTAttribute = [&](Attribute &attr) {
-    auto optAttr = parser.parseOptionalAttribute(attr);
-    if (optAttr.has_value()) {
-      if (failed(*optAttr))
-        return false;
-      if (attr.isa<ASTFunctionDeclAttr>() || attr.isa<ASTRecordDeclAttr>() ||
-          attr.isa<ASTVarDeclAttr>())
-        return true;
-      parser.emitError(parser.getCurrentLocation(),
-                       "Unknown cir.struct attribute");
-      return false;
-    }
-    return false;
-  };
-
-  while (mlir::succeeded(parser.parseOptionalComma())) {
-    if (mlir::succeeded(parser.parseOptionalKeyword("incomplete")))
-      continue;
-
-    parsedBody = true;
-    Type nextMember;
-    auto optTy = parser.parseOptionalType(nextMember);
-    if (optTy.has_value()) {
-      if (failed(*optTy))
-        return Type();
-      members.push_back(nextMember);
-      continue;
-    }
+  if (parser.parseAttribute(id))
+    return {};
 
-    // Maybe it's an AST attribute: always last member, break.
-    Attribute astAttr;
-    if (parseASTAttribute(astAttr))
-      break;
+  if (parser.parseOptionalKeyword("packed").succeeded())
+    packed = true;
+
+  if (parser.parseOptionalKeyword("incomplete").failed()) {
+    body = true;
+    const auto delim = AsmParser::Delimiter::Braces;
+    auto result = parser.parseCommaSeparatedList(delim, [&]() -> ParseResult {
+      mlir::Type ty;
+      if (parser.parseType(ty))
+        return mlir::failure();
+      members.push_back(ty);
+      return mlir::success();
+    });
+
+    if (result.failed())
+      return {};
   }
 
+  parser.parseOptionalAttribute(ast);
+
   if (parser.parseGreater())
-    return Type();
-  auto sTy = get(parser.getContext(), members, typeName, parsedBody);
-  return sTy;
+    return {};
+
+  return StructType::get(parser.getContext(), members, id, body, packed,
+                         std::nullopt);
 }
 
 void StructType::print(mlir::AsmPrinter &printer) const {
-  printer << '<' << getTypeName();
+  printer << '<' << getTypeName() << " ";
+
+  if (getPacked())
+    printer << "packed ";
+
   if (!getBody()) {
-    printer << ", incomplete";
+    printer << "incomplete";
   } else {
-    auto members = getMembers();
-    if (!members.empty()) {
-      printer << ", ";
-      llvm::interleaveComma(getMembers(), printer);
-    }
+    printer << "{";
+    llvm::interleaveComma(getMembers(), printer);
+    printer << "}";
   }
-  if (getAst()) {
-    printer << ", ";
-    printer.printAttributeWithoutType(*getAst());
+
+  if (getAst().has_value()) {
+    printer << " ";
+    printer.printAttribute(getAst().value());
   }
+
   printer << '>';
 }
 

clang/test/CIR/CodeGen/agg-init.cpp

@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++17 -fclangir -Wno-unused-value -emit-cir %s -o %t.cir
 // RUN: FileCheck --input-file=%t.cir %s
 
-// CHECK: !ty_22struct2EZero22 = !cir.struct<"struct.Zero", !u8i>
-// CHECK: !ty_22struct2Eyep_22 = !cir.struct<"struct.yep_", !u32i, !u32i>
+// CHECK: !ty_22struct2EZero22 = !cir.struct<"struct.Zero" {!u8i}>
+// CHECK: !ty_22struct2Eyep_22 = !cir.struct<"struct.yep_" {!u32i, !u32i}>
 
 struct Zero {
   void yolo();

clang/test/CIR/CodeGen/atomic.cpp

@@ -7,4 +7,4 @@ typedef struct _a {
 
 void m() { at y; }
 
-// CHECK: !ty_22struct2E_a22 = !cir.struct<"struct._a", !s32i>
+// CHECK: !ty_22struct2E_a22 = !cir.struct<"struct._a" {!s32i}>

clang/test/CIR/CodeGen/bitfields.cpp

@@ -14,5 +14,5 @@ void m() {
   __long l;
 }
 
-// CHECK: !ty_22struct2Eanon22 = !cir.struct<"struct.anon", !u32i, #cir.recdecl.ast>
-// CHECK: !ty_22struct2E__long22 = !cir.struct<"struct.__long", !ty_22struct2Eanon22, !u32i, !cir.ptr<!u32i>>
+// CHECK: !ty_22struct2Eanon22 = !cir.struct<"struct.anon" {!u32i} #cir.recdecl.ast>
+// CHECK: !ty_22struct2E__long22 = !cir.struct<"struct.__long" {!ty_22struct2Eanon22, !u32i, !cir.ptr<!u32i>}>

clang/test/CIR/CodeGen/coro-task.cpp

@@ -126,13 +126,13 @@ co_invoke_fn co_invoke;
 
 }} // namespace folly::coro
 
-// CHECK: ![[VoidTask:ty_.*]] = !cir.struct<"struct.folly::coro::Task", !u8i>
-// CHECK: ![[IntTask:ty_.*]] = !cir.struct<"struct.folly::coro::Task", !u8i>
-// CHECK: ![[VoidPromisse:ty_.*]] = !cir.struct<"struct.folly::coro::Task<void>::promise_type", !u8i>
-// CHECK: ![[CoroHandleVoid:ty_.*]] = !cir.struct<"struct.std::coroutine_handle", !u8i>
-// CHECK: ![[CoroHandlePromise:ty_.*]] = !cir.struct<"struct.std::coroutine_handle", !u8i>
-// CHECK: ![[StdString:ty_.*]] = !cir.struct<"struct.std::string", !u8i
-// CHECK: ![[SuspendAlways:ty_.*]] = !cir.struct<"struct.std::suspend_always", !u8i>
+// CHECK: ![[VoidTask:ty_.*]] = !cir.struct<"struct.folly::coro::Task" {!u8i}>
+// CHECK: ![[IntTask:ty_.*]] = !cir.struct<"struct.folly::coro::Task" {!u8i}>
+// CHECK: ![[VoidPromisse:ty_.*]] = !cir.struct<"struct.folly::coro::Task<void>::promise_type" {!u8i}>
+// CHECK: ![[CoroHandleVoid:ty_.*]] = !cir.struct<"struct.std::coroutine_handle" {!u8i}>
+// CHECK: ![[CoroHandlePromise:ty_.*]] = !cir.struct<"struct.std::coroutine_handle" {!u8i}>
+// CHECK: ![[StdString:ty_.*]] = !cir.struct<"struct.std::string" {!u8i}
+// CHECK: ![[SuspendAlways:ty_.*]] = !cir.struct<"struct.std::suspend_always" {!u8i}>
 
 // CHECK: module {{.*}} {
 // CHECK-NEXT: cir.global external @_ZN5folly4coro9co_invokeE = #cir.zero : !ty_22struct2Efolly3A3Acoro3A3Aco_invoke_fn22

clang/test/CIR/CodeGen/ctor.cpp

@@ -11,7 +11,7 @@ void baz() {
   Struk s;
 }
 
-// CHECK: !ty_22struct2EStruk22 = !cir.struct<"struct.Struk", !s32i>
+// CHECK: !ty_22struct2EStruk22 = !cir.struct<"struct.Struk" {!s32i}>
 
 // CHECK:   cir.func linkonce_odr @_ZN5StrukC2Ev(%arg0: !cir.ptr<!ty_22struct2EStruk22>
 // CHECK-NEXT:     %0 = cir.alloca !cir.ptr<!ty_22struct2EStruk22>, cir.ptr <!cir.ptr<!ty_22struct2EStruk22>>, ["this", init] {alignment = 8 : i64}

clang/test/CIR/CodeGen/derived-to-base.cpp

@@ -75,8 +75,8 @@ void C3::Layer::Initialize() {
   }
 }
 
-// CHECK: !ty_22class2EC23A3ALayer22 = !cir.struct<"class.C2::Layer", !ty_22class2EC13A3ALayer22, !cir.ptr<!ty_22class2EC222>
-// CHECK: !ty_22struct2EC33A3ALayer22 = !cir.struct<"struct.C3::Layer", !ty_22class2EC23A3ALayer22
+// CHECK: !ty_22class2EC23A3ALayer22 = !cir.struct<"class.C2::Layer" {!ty_22class2EC13A3ALayer22, !cir.ptr<!ty_22class2EC222>
+// CHECK: !ty_22struct2EC33A3ALayer22 = !cir.struct<"struct.C3::Layer" {!ty_22class2EC23A3ALayer22
 
 // CHECK: cir.func @_ZN2C35Layer10InitializeEv
 

clang/test/CIR/CodeGen/dtors.cpp

@@ -38,10 +38,10 @@ class B : public A
 };
 
 // Class A
-// CHECK: ![[ClassA:ty_.*]] = !cir.struct<"class.A", !cir.ptr<!cir.ptr<!cir.func<!u32i ()>>>, #cir.recdecl.ast>
+// CHECK: ![[ClassA:ty_.*]] = !cir.struct<"class.A" {!cir.ptr<!cir.ptr<!cir.func<!u32i ()>>>} #cir.recdecl.ast>
 
 // Class B
-// CHECK: ![[ClassB:ty_.*]] = !cir.struct<"class.B", ![[ClassA]]>
+// CHECK: ![[ClassB:ty_.*]] = !cir.struct<"class.B" {![[ClassA]]}>
 
 // CHECK: cir.func @_Z4bluev()
 // CHECK:   %0 = cir.alloca !ty_22class2EPSEvent22, cir.ptr <!ty_22class2EPSEvent22>, ["p", init] {alignment = 8 : i64}