[PATCH] D114371: [Flang] Replace notifyMatchFailure with TODO hard failures

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:1563
+    if (isDerivedTypeWithLenParams(boxTy)) {
+      TODO(embox.getLoc(), "fir.embox codegen of derived with length parameters");
+      return failure();
----------------
awarzynski wrote:
> clementval wrote:
> > awarzynski wrote:
> > > Test?
> > This is one of the case where the type conversion will failed first before reaching this point. 
> Ta! Could this be documented in the summary?
will do.


================
Comment at: flang/test/Fir/Todo/end.fir:1
+// RUN: fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s
+// XFAIL:*
----------------
mehdi_amini wrote:
> Can you add  `2>&1 | FileCheck %s` and check that you actually capture the expected reason to fail?
I am assuming you mean to prefix with not, redirect error to stdout and check with FileCheck after removing XFAIL, like given below. 

As I mentioned in the other thread, this does not work in a debug build. In a debug build, fir::emitFatalError is called which calls mlir::emitError and then aborts with llvm_report_fatal_error. I think the abort makes it difficult to test. 
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Lower/Todo.h#L65
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Optimizer/Support/FatalError.h#L23




```
// RUN: not fir-opt --fir-to-llvm-ir="target=x86_64-unknown-linux-gnu" %s 2>&1 | FileCheck %s
  
// Test `fir.end` conversion to llvm.
// Not implemented yet.

// CHECK: not yet implemented fir.end codegen

"fir.end"() : () -> ()
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114371