[PATCH] D72542: [Flang] add a band-aid to support the creation of mutually recursive types when lowering to LLVM IR

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: mlir/test/Dialect/LLVMIR/roundtrip.mlir:222
+
+// XXXCHECK: llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
+//llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
----------------
schweitz wrote:
> jdoerfert wrote:
> > schweitz wrote:
> > > ftynse wrote:
> > > > jdoerfert wrote:
> > > > > A "FIXME/TODO" and an actual `CHECK` might be better here. The FIXME explains the problem and the `CHECK` makes sure we see changes to whatever we are doing.
> > > > +1
> > > Can someone provide a pointer to something like this? I'm having a hard time envisioning a test that tests for a TODO item.
> > ```
> > TODO: This should be llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
> > CHECK: <here check what it is right now>
> > ```
> Running this test as `mlir-opt %s | mlir-opt | FileCheck %s` would result in the following output.
> 
> ```
> mlir/test/Dialect/LLVMIR/roundtrip.mlir:223:34: error: expected end of string
> llvm.func @recursive_type(!llvm<"%a = type { %a* }">)
>                                  ^
> mlir/test/Dialect/LLVMIR/roundtrip.mlir:3:17: error: CHECK-LABEL: expected string not found in input
> // CHECK-LABEL: func @ops(%arg0: !llvm.i32, %arg1: !llvm.float)
>                 ^
> <stdin>:3:1: note: scanning from here
> module {
> ^
> ```
> 
> That seems an undesirable option.
> 
> Another approach would be to add a separate test altogether, a test that expects the tool to error and terminate.
I'm confused. You have XXXCHECK check right now, why can't we make that into, or augment, a TODO with some information?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72542/new/

https://reviews.llvm.org/D72542