[CIR][IR] Relax get_member verifier for incomplete types (#269)

gitoleg · lanza · commit a2bfdbbc84a0 · 2024-03-22T16:19:53.000-07:00
This is a suggestion to relax the existing verification even more than we did it in PR #257. Here we also skip verification if a field on the given index is also of incomplete type - and we can not compare it with the result type of the operation. Now the next code fails with type mismatch error: ``` typedef struct Node { struct Node* next; } NodeStru; void foo(NodeStru* a) { a->next = 0; } ``` because the result type is kind of full and the type of field is not (for the reasons discussed in #256). Basically, the problem is in the `GetMemberOp` result type generated as following (via `CIRGenTypes::convertType`) `!cir.ptr<!cir.struct<struct "Node" {!cir.ptr<!cir.struct<struct "Node" incomplete #cir.record.decl.ast>>} #cir.record.decl.ast>>` where the field type at index differs from the record type - compare with `!cir.ptr<!cir.struct<struct "Node" incomplete #cir.record.decl.ast>>` We just slightly relax the previous solution in #257 - and the compilation won't fail in the case of recursive types. Well, if there are some other thoughts?
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -2412,6 +2412,14 @@ LogicalResult MemCpyOp::verify() {
   return mlir::success();
 }
 
+static bool isIncompleteType(mlir::Type typ) {
+  if (auto ptr = typ.dyn_cast<PointerType>())
+    return isIncompleteType(ptr.getPointee());
+  else if (auto rec = typ.dyn_cast<StructType>())
+    return !rec.getBody();
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 // GetMemberOp Definitions
 //===----------------------------------------------------------------------===//
@@ -2424,16 +2432,20 @@ LogicalResult GetMemberOp::verify() {
 
   // FIXME: currently we bypass typechecking of incomplete types due to errors
   // in the codegen process. This should be removed once the codegen is fixed.
-  if (!recordTy.getBody())
+  if (isIncompleteType(recordTy))
     return mlir::success();
 
   if (recordTy.getMembers().size() <= getIndex())
     return emitError() << "member index out of bounds";
 
   // FIXME(cir): member type check is disabled for classes as the codegen for
   // these still need to be patched.
-  if (!recordTy.isClass() &&
-      recordTy.getMembers()[getIndex()] != getResultTy().getPointee())
+  // Also we bypass the typechecking for the fields of incomplete types.
+  bool shouldSkipMemberTypeMismatch =
+    recordTy.isClass() || isIncompleteType(recordTy.getMembers()[getIndex()]);
+
+  if (!shouldSkipMemberTypeMismatch
+      && recordTy.getMembers()[getIndex()] != getResultTy().getPointee())
     return emitError() << "member type mismatch";
 
   return mlir::success();
diff --git a/clang/test/CIR/CodeGen/struct.c b/clang/test/CIR/CodeGen/struct.c
@@ -12,11 +12,18 @@ struct Foo {
   struct Bar z;
 };
 
+// Recursive type
+typedef struct Node {
+  struct Node* next;
+} NodeStru;
+
 void baz(void) {
   struct Bar b;
   struct Foo f;
 }
 
+// CHECK-DAG: !ty_22Node22 = !cir.struct<struct "Node" incomplete #cir.record.decl.ast>
+// CHECK-DAG: !ty_22Node221 = !cir.struct<struct "Node" {!cir.ptr<!ty_22Node22>} #cir.record.decl.ast>
 // CHECK-DAG: !ty_22Bar22 = !cir.struct<struct "Bar" {!s32i, !s8i}>
 // CHECK-DAG: !ty_22Foo22 = !cir.struct<struct "Foo" {!s32i, !s8i, !ty_22Bar22}>
 //  CHECK-DAG: module {{.*}} {
@@ -78,3 +85,9 @@ struct Bar shouldGenerateAndAccessStructArrays(void) {
 // CHECK-DAG: %[[#DARR:]] = cir.cast(array_to_ptrdecay, %{{.+}} : !cir.ptr<!cir.array<!ty_22Bar22 x 1>>), !cir.ptr<!ty_22Bar22>
 // CHECK-DAG: %[[#ELT:]] = cir.ptr_stride(%[[#DARR]] : !cir.ptr<!ty_22Bar22>, %[[#STRIDE]] : !s32i), !cir.ptr<!ty_22Bar22>
 // CHECK-DAG: cir.copy %[[#ELT]] to %{{.+}} : !cir.ptr<!ty_22Bar22>
+
+// CHECK-DAG: cir.func @useRecursiveType
+// CHECK-DAG: cir.get_member {{%.}}[0] {name = "next"} : !cir.ptr<!ty_22Node221> -> !cir.ptr<!cir.ptr<!ty_22Node221>>
+void useRecursiveType(NodeStru* a) {
+  a->next = 0;
+}
