[flang][driver] Add debug measure-parse-tree and pre-fir-tree options

[FarisRehman]

Summary

Add the following options:

• -fdebug-measure-parse-tree
• -fdebug-pre-fir-tree

Summary of changes:

• Add 2 new frontend actions: DebugMeasureParseTreeAction and DebugPreFIRTreeAction
• Add MeasurementVisitor to FrontendActions.h
• Make reportFatalSemanticErrors return true if there are any fatal errors
• Port most of the -fdebug-pre-fir-tree tests to use the new driver if built, otherwise use f18

[FarisRehman]

Rebase off main

Rebase off main and add a comment to MeasurementVisitor.

[awarzynski]

Thank you for working on this @FarisRehman !

This semantics implemented here are identical to what f18 does. -fdebug-instrumented-parse from f18 becomes -fdebug-parsing-log in flang-new -fc1. This was proposed (and accepted) in the RFC.

I've left a few comments inline, but nothing major and otherwise this is ready to land.

flang/include/flang/Frontend/FrontendActions.h

18

This should be shared with f18.cpp, but currently we don't have a good place for that (and we probably want to understand how much is going to be shared first). So IMO it is fine to keep it here.

Could you add a comment similar to this so that it is clear that this should be moved at some point?

flang/lib/Frontend/FrontendActions.cpp

313

Could this use diagnostics instead? For example:

unsigned diagID = ci.diagnostics().getCustomDiagID(clang::DiagnosticsEngine::Error, "Pre FIR Tree is NULL.");
ci.diagnostics().Report(diagID);

flang/test/Flang-Driver/debug-measure-parse-tree.f90

24–25

The output for this test is:

Parse tree comprises 18 objects and occupies 1496 total bytes.

I think that the actual size (i.e. 18 and 1496) is not that relevant here and also prone to change (which may cause the test to become fragile).

Instead, I suggest that we verify that -fdebug-measure-parse-tree indeed instructs the driver to measure the parse and print the result. So, perhaps this instead:

Parse tree comprises {{[0-9]+}} objects and occupies {{[0-9]+}} total bytes.

flang/test/Flang-Driver/debug-parsing-log.f90

23 ↗

(On Diff #324362)

IIUC, this output is not really specific to -fdebug-parsing-log (-fsyntax-only prints this too). I think that this would be more specific to this flag:

! FRONTEN: fail 1
! FRONTEN: fail 2
! FRONTEN: fail 3

This is still not great, but looking at the log itself I struggle to notice anything that would stand out.

[awarzynski]

I've just realized that for -fdebug-parsing-log we need to set Fortran::parser::instrumentedParse. I don't see a good place for that just yet.

It's probably best extracting that option into a separate patch.

[FarisRehman]

Remove -fdebug-parsing-log

Remove -fdebug-parsing-log from this patch as it requires additional work that can be implemented in a separate patch.
Also address review comments by @awarzynski

[awarzynski]

Thank you for updating this @FarisRehman !

The original implementation of `-fdebug-parsing-log submitted here was not 100% compatible with -fdebug-instrumented-parse from f18 as we originally thought. I think that that's fine, but we need something that gives us such compatibility. We probably need another flag for that, e.g. -finstrumented-parse, that would toggle Fortran::parser::instrumentedParse. This way we will have this:

• -fdebug-parsing-log (flang-new -fc1) != -fdebug-instrumented-parse (f18)
• -fdebug-parsing-log -finstrumented-parse (flang-new -fc1) == -fdebug-instrumented-parse (f18)

But that's quite a few new scenarios and it's worth submitting as a separate patch.

You've also changed the order of flags in tests. This is also fine and guarantees that tests work with both f18 and flang-new -fc1:

• in flang-new -fc1 only the last _action_ option is take into account, so -fsyntax-only is effectively ignored (and not needed)
• in f18 we still need -fsyntax-only to make sure that code-generation is not run (e.g. to make f18 exit _after_ running the semantic checks), but the order is irrelevant

All this makes a lot of sense, thank you for adding these! LGTM!