[flang][f18] Add an additional check for semantic errors

[awarzynski]

Summary

Semantics::AnyFatalError may return true after
Fortran::semantics::BuildRuntimeDerivedTypeTables is run. That's
despite Semantics::perform having been run earlier and not reporting
any issues. This is demonstrated below:

semantics.perform()
// No errors here, i.e. `semantics.AnyFatalError()` returns `false`
BuildRuntimeDerivedTypeTables()
// New semantic errors are found, i.e. `semantics.AnyFatalError()` returns `true`

This patch udpates the "throwaway" driver so that the semantic errors
identified after BuildRuntimeDerivedTypeTables are correctly reported.

This change affects some tests - the corresponding input leads to
semantic errors that are only identified after
BuildRuntimeDerivedTypeTables is run. The generated output (which is
fed into FileCheck) still matches the expected output, but f18 returns
a non-zero code. As a temporary solution workaround for this, %f18 is
replaced with not %f18.

[awarzynski]

I've noticed that there might be new semantic errors generated after BuildRuntimeDerivedTypeTables is run. Should we ignore or report them? The tests that trigger these errors look fine to me, so I'm not sure.

[awarzynski]

Revert %flang_fc1 --> %f18 (leftover from experimenting)

[klausler]

semantics.perform()
// No errors here, i.e. `semantics.AnyFatalError()` returns `false`
BuildRuntimeDerivedTypeTables()
// New semantic errors are found, i.e. `semantics.AnyFatalError()` returns `false`

How do you know that more errors were found if AnyFatalError() is still false?

[awarzynski]

In D100648#2695093, @klausler wrote:

semantics.perform()
// No errors here, i.e. `semantics.AnyFatalError()` returns `false`
BuildRuntimeDerivedTypeTables()
// New semantic errors are found, i.e. `semantics.AnyFatalError()` returns `false`

How do you know that more errors were found if AnyFatalError() is still false?

That's a typo, apologies.

[klausler]

So AnyFatalError() is true after the derived type tables are built for the runtime? Were there additional Messages?

[awarzynski]

In D100648#2695283, @klausler wrote:

So AnyFatalError() is true after the derived type tables are built for the runtime? Were there additional Messages?

Yes. You can reproduce it with this file (extracted from offset01.f90):

subroutine s5(n)
   integer :: n
   type :: t1(l)
     integer, len :: l
     real :: a(l)
   end type
   type :: t2(l1, l2)
     integer, len :: l1
     integer, len :: l2
     real :: b(l1, l2)
   end type
   type(t1(n))   :: x1 !CHECK: x1 size=48 offset=
   type(t2(n,n)) :: x2 !CHECK: x2 size=56 offset=
 end

And here are the errors:

$ f18 -fdebug-dump-symbols offsets01.f90 > dump.txt
f18: Semantic errors in offsets01.f90
offsets01.f90:5:13: error: Specification expression 'int(int(int(n,kind=8),kind=4),kind=8)' is neither constant nor a length type parameter
      real :: a(l)
              ^
offsets01.f90:10:13: error: Specification expression 'int(int(int(n,kind=8),kind=4),kind=8)' is neither constant nor a length type parameter
      real :: b(l1, l2)
              ^
offsets01.f90:10:13: error: Specification expression 'int(int(int(n,kind=8),kind=4),kind=8)' is neither constant nor a length type parameter
      real :: b(l1, l2)
              ^

[PeteSteinfeld]

All builds, tests, and looks good.

But I'm concerned that we may be sweeping problems under the rug with this change. @klausler, shouldn't all semantic errors be caught by the end of execution of semantics.perform()? If so, we should track down all of the errors revealed by data05.f90, offsets01.f90, offsets02.f90, and typeinfo01.f90 and make sure that they're handled in semantics.perform() and write tests for them.

[klausler]

In D100648#2698449, @PeteSteinfeld wrote:

All builds, tests, and looks good.

But I'm concerned that we may be sweeping problems under the rug with this change. @klausler, shouldn't all semantic errors be caught by the end of execution of semantics.perform()? If so, we should track down all of the errors revealed by data05.f90, offsets01.f90, offsets02.f90, and typeinfo01.f90 and make sure that they're handled in semantics.perform() and write tests for them.

All user-caused errors must be caught by the end of semantics (specifically, before lowering) and all messages created by then. It's okay to have the derived type table builder catch errors so long as it's always run.

In this case, though, the errors are bogus, so it's kind of a moot point.

[awarzynski]

In D100648#2698677, @klausler wrote:

It's okay to have the derived type table builder catch errors so long as it's always run.

Currently it's only run for -fdebug-dump-symbols. Should it be always run instead?

In this case, though, the errors are bogus, so it's kind of a moot point.

Do you recommend that we don't check these errors and that this change is not merged?

[klausler]

In D100648#2698786, @awarzynski wrote:

In D100648#2698677, @klausler wrote:

It's okay to have the derived type table builder catch errors so long as it's always run.

Currently it's only run for -fdebug-dump-symbols. Should it be always run instead?

That subroutine will have to always run when code is going to be lowered. So it will soon always have to run unless a syntax-only option is in effect.

In this case, though, the errors are bogus, so it's kind of a moot point.

Do you recommend that we don't check these errors and that this change is not merged?

These checks are valid, but they obviously can't be applied until you fix the bogus errors.

[awarzynski]

@PeteSteinfeld & @klausler , thank you for your comments - the context surrounding BuildRuntimeDerivedTypeTables is much clearer now.

I agree with both of you - it would be much better to fix the invalid errors generated by BuildRuntimeDerivedTypeTables first. As I'm unfamiliar with this part of the frontend, I would rather wait for somebody more experienced in this area to look into it.

This is now reported as 50040.

[klausler]

A fix for the bogus errors is coming soon.

flang/test/Semantics/data05.f90

-!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: not %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 module m
   interface
     integer function ifunc(n)
       integer, intent(in) :: n
     end function
     real function rfunc(x)
       real, intent(in) :: x
     end function

flang/test/Semantics/offsets01.f90

-!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: not %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 
 ! Size and alignment of intrinsic types
 subroutine s1
   integer(1) :: a_i1  !CHECK: a_i1 size=1 offset=0:
   integer(8) :: b_i8  !CHECK: b_i8 size=8 offset=8:
   real(2)    :: c_r2  !CHECK: c_r2 size=2 offset=16:
   real(2)    :: d_r2  !CHECK: d_r2 size=2 offset=18:
   real(8)    :: e_r8  !CHECK: e_r8 size=8 offset=24:

flang/test/Semantics/offsets02.f90

-!RUN: %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: not %flang_fc1 -fdebug-dump-symbols %s | FileCheck %s
 
 ! Size and alignment of derived types
 
 ! Array of derived type with 64-bit alignment
 subroutine s1
   type t1
     real(8) :: a
     real(4) :: b

flang/test/Semantics/typeinfo01.f90

-!RUN: %f18 -fdebug-dump-symbols %s | FileCheck %s
+!RUN: not %f18 -fdebug-dump-symbols %s | FileCheck %s
 ! Tests for derived type runtime descriptions
 
 module m01
   type :: t1
     integer :: n
   end type
 !CHECK: .c.t1, SAVE, TARGET: ObjectEntity type: TYPE(component) shape: 0_8:0_8 init:[component::component(name=.n.n,genre=1_1,category=0_1,kind=4_1,rank=0_1,offset=0_8,characterlen=value(genre=1_1,value=0_8),derived=NULL(),lenvalue=NULL(),bounds=NULL(),initialization=NULL())]
 !CHECK: .dt.t1, SAVE, TARGET: ObjectEntity type: TYPE(derivedtype) init:derivedtype(binding=NULL(),name=.n.t1,sizeinbytes=4_8,parent=NULL(),uninstantiated=NULL(),kindparameter=NULL(),lenparameterkind=NULL(),component=.c.t1,procptr=NULL(),special=NULL())

flang/tools/f18/f18.cpp

auto tables{
         Fortran::semantics::BuildRuntimeDerivedTypeTables(semanticsContext)};
     if (!tables.schemata) {
       llvm::errs() << driver.prefix
                    << "could not find module file for __fortran_type_info\n";
     }
     if (driver.dumpSymbols) {
       semantics.DumpSymbols(llvm::outs());
     }
+    if (semantics.AnyFatalError()) {
+      llvm::errs() << driver.prefix << "Semantic errors in " << path << '\n';
+      semantics.EmitMessages(llvm::errs());
+      exitStatus = EXIT_FAILURE;
+      return {};
+    }
     if (driver.dumpUnparseWithSymbols) {
       Fortran::semantics::UnparseWithSymbols(
           llvm::outs(), parseTree, driver.encoding);
       return {};
     }
     if (driver.getSymbolsSources) {
       semantics.DumpSymbolsSources(llvm::outs());
       return {};