[PATCH] D96884: [flang][driver] Add more -fdebug options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski accepted this revision.
awarzynski added a comment.
This revision is now accepted and ready to land.

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 <https://lists.llvm.org/pipermail/flang-dev/2020-November/000588.html>.

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



================
Comment at: flang/include/flang/Frontend/FrontendActions.h:18
 
+struct MeasurementVisitor {
+  template <typename A> bool Pre(const A &) { return true; }
----------------
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 [[ https://github.com/llvm/llvm-project/blob/adfd3c7083f9808d145239153c10f72eece485d8/flang/lib/Frontend/FrontendOptions.cpp#L29-L32 | this ]] so that it is clear that this should be moved at some point?


================
Comment at: flang/lib/Frontend/FrontendActions.cpp:313
+  } else {
+    llvm::errs() << "Pre FIR Tree is NULL.\n";
+  }
----------------
Could this use diagnostics instead? For example:
```
    unsigned diagID = ci.diagnostics().getCustomDiagID(clang::DiagnosticsEngine::Error, "Pre FIR Tree is NULL.");
    ci.diagnostics().Report(diagID);
```


================
Comment at: flang/test/Flang-Driver/debug-measure-parse-tree.f90:23-24
+!---------------------------------------
+! FRONTEND:18 objects
+! FRONTEND:1496 total bytes
+
----------------
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.
```


================
Comment at: flang/test/Flang-Driver/debug-parsing-log.f90:23
+!---------------------------------------
+! FRONTEND:in the context: END PROGRAM statement
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96884