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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
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:
> kiranchandramohan wrote:
> > 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"() : () -> ()
> > ```
> > I am assuming you mean to prefix with not, redirect error to stdout and check with FileCheck after removing XFAIL, like given below.
> 
> The `not` prefix vs XFAIL is orthogonal to checking the message, my comment here is only about checking the cause of the failure.
> 
> 
> >  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. 
> 
> I'm puzzled: wouldn't the message show up anyway? 
> Why wouldn't the `// CHECK: not yet implemented fir.end codegen` work in both mode?
> The not prefix vs XFAIL is orthogonal to checking the message, my comment here is only about checking the cause of the failure.

Actually, no it isn't orthogonal. But the question remains: I'm not sure why the message isn't emitted in both cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114371