[flang] Update the plugin API

[awarzynski]

Summary

This patch adds a new member method in the PluginParseTreeAction
frontend action base class: getParsing. As plugins are meant as a
convenient interface for analysing the parse tree, this method is key.
With this new method, plugins no longer need to include
CompilerInstance.h to retrieve e.g. parse trees. As a result, the
plugins are more independent of the driver's implementation's details
(e.g. CompilerInstance).

[kiranchandramohan]

Can you check why the patch application failed?

[awarzynski]

Rebase

@kiranchandramohan My bad, I forgot to squash before sending to Phab!

[awarzynski]

Fix formatting

[Leporacanthicus]

flang/examples/FlangOmpReport/FlangOmpReport.cpp

56

Just out of curiosity, why is the parsing not a constructor argument?

[kiranchandramohan]

flang/examples/FlangOmpReport/FlangOmpReport.cpp

64–65

I was thinking that flang-omp-report will run on all the source files and generate a lot of YAML files whose name is derived from the name of the source file. Then the summary python script collects a summary from all these info.

This code looks like it dumps everything to output, wouldn't it affect the running of the summarize python script?

[awarzynski]

flang/examples/FlangOmpReport/FlangOmpReport.cpp

56

Of PluginParseTreeAction? Parsing is not available until BeginSourceFileAction is run. That's after this object is constructed.

64–65

I was thinking that flang-omp-report will run on all the source files and generate a lot of YAML files whose name is derived from the name of the source file.

Not quite. You will have to run the plugin manually for every file. But you can automate this by "hacking" a Makefile of the project that you are interested in. Currently, you will need these extra flags:

FFLAGS=-fsyntax-only -I /.../llvm-project/flang/module -Xflang -load -Xflang /.../llvm-project/build/Release/lib/flangOmpReport.so -Xflang -plugin -Xflang flang-omp-report -fopenmp

With this change, you will also have to pipe the output to a file (so that it does not end up in stdout):

FFLAGS=-fsyntax-only -I /.../llvm-project/flang/module -Xflang -load -Xflang /.../llvm-project/build/Release/lib/flangOmpReport.so -Xflang -plugin -Xflang flang-omp-report -fopenmp -o output.yaml

This does make using the plugin on larger projects a bit more tricky, but I feel that the API should be independent of the internals of the driver. In particular, the driver depends on various TableGen files from Clang that define diagnostics. So, in general, so does this plugin because it includes "CompilerInstance.h" from the driver. I'm trying to remove this dependency.

As for the script, it's independent of the plugin and all it needs are some *.yaml files (i.e. it does not care how you generate them).

[kiranchandramohan]

flang/examples/FlangOmpReport/FlangOmpReport.cpp

64–65

but I feel that the API should be independent of the internals of the driver

That is a good goal for plugins. But at the same time the advantage of running flang-omp-report as a plugin is the fact that we can run it on large projects. Most drivers default to using the file name as the default. Why can't we ask the driver to provide the output stream just like you did for Parsing?

[awarzynski]

flang/examples/FlangOmpReport/FlangOmpReport.cpp

64–65

That is a good goal for plugins. But at the same time the advantage of running flang-omp-report as a plugin is the fact that we can run it on large projects.

This change does not affect this.

Recall that in order to run FlangOmpReporton a project (note that we changed the name from flang-omp-report for consistency with other libs), you need to modify the corresponding build script. This is the case _with_ and _without_ this change. However, this update does mean that you will need one additional change in the build script though. For example, for SNAP I needed this:

@@ -187,7 +187,7 @@ translv.o : global.o plib.o geom.o sn.o data.o control.o utils.o \
        $(FORTRAN) $(FFLAGS) -c $*.f90

 %.o:   %.f90
-       $(FORTRAN) $(FFLAGS) -c $<
+       $(FORTRAN) $(FFLAGS) -c $< >> $(basename $@).yaml

 #

But still, I'm not removing the possibility of running this plugin on a project (i.e. for multiple files).

Most drivers default to using the file name as the default.

Yes, that's a good point. But I felt that that's not "mission critical". First, I want to get rid of the dependency on "CompilerInstance.h". That has caused build failures for people (CC @DavidSpickett ).

I do agree that plugins should inherit the drivers' behavior. I just didn't have the time to look into this. Anyway, let me fix that (update coming shortly).

[awarzynski]

Make plugins respect drivers' -o <output-file>

[kiranchandramohan]

Thanks @awarzynski. LGTM.

[DavidSpickett]

@awarzynski Did you forget about this?

(I'm not waiting on it for any reason just happened to be looking through my phab queues)

[awarzynski]

In D120999#3466847, @DavidSpickett wrote:

@awarzynski Did you forget about this?

I did :) Thanks for the ping, merging now!

flang/docs/FlangDriver.md

 ```
 
 ### Implementation of `ExecuteAction`
 Like in other frontend actions, the driver looks for an `ExecuteAction` function
 to run, so in order for your plugin to do something, you will need to implement
 the `ExecuteAction` method in your plugin class. This method will contain the
 implementation of what the plugin actually does, for example:
 ```cpp
+// Forward declaration
+struct ParseTreeVisitor;
+
 void ExecuteAction() override {
-  auto &parseTree{instance().parsing().parseTree()};
   ParseTreeVisitor visitor;
-  Fortran::parser::Walk(parseTree, visitor);
+  Fortran::parser::Walk(getParsing().parseTree(), visitor);
 }
 ```
-In the example plugin, the `ExecuteAction` method first gets a reference to the
-parse tree, `instance().parsing().parseTree()`, then declares a `visitor`
-struct, before passing both of these to the `Fortran::parser::Walk` function
-that will traverse the parse tree. Implementation and details of the `Walk`
-function can be found in `flang/include/flang/Parser/parse-tree-visitor.h`.
-
-A `visitor` struct should define different `Pre` and `Post` functions that take
-the type of a specific `ParseTree` node as an argument. When the `Walk` function
-is traversing the parse tree, these functions will be run before/after a node
-of that type is visited. Template functions for `Pre`/`Post` are defined so that
-when a node is visited that you have not defined a function for, it will still
-be able to continue. `Pre` returns a `bool` indicating whether to visit that
-node's children or not. For example:
+In the example plugin, the `ExecuteAction` method first creates an instance of
+`visitor` struct, before passing it together with the parse tree to the
+`Fortran::parser::Walk` function that will traverse the parse tree. The parse
+tree will normally be generated by the frontend driver and can be retrieved in
+your plugin through the `getParsing()` member method. Implementation and
+details of the `Walk` function can be found in
+`flang/include/flang/Parser/parse-tree-visitor.h`.
+
+You will have to define your own `visitor` struct. It should define different
+`Pre` and `Post` functions that take the type of a specific `ParseTree` node as
+an argument. When the `Walk` function is traversing the parse tree, these
+functions will be run before/after a node of that type is visited. Template
+functions for `Pre`/`Post` are defined so that when a node is visited that you
+have not defined a function for, it will still be able to continue. `Pre`
+returns a `bool` indicating whether to visit that node's children or not. For
+example:
 ```cpp
 struct ParseTreeVisitor {
   template <typename A> bool Pre(const A&) { return true; }
   template <typename A> void Post(const A&) {}
   void Post(const Fortran::parser::FunctionStmt &f) {
     llvm::outs() << std::get<Fortran::parser::Name>(f.t).ToString() << "\n" ;
   }
 }

flang/examples/FlangOmpReport/FlangOmpReport.cpp

 //===-- examples/flang-omp-report-plugin/flang-omp-report.cpp -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-// This plugin parses a Fortran source file and generates a YAML
-// report with all the OpenMP constructs and clauses and which
-// line they're located on.
+// This plugin parses a Fortran source file and generates a YAML report with
+// all the OpenMP constructs and clauses and which line they're located on.
 //
 // The plugin may be invoked as:
-// ./bin/flang-new -fc1 -load lib/flangOmpReport.so -plugin
-// flang-omp-report -fopenmp -o - <source_file.f90>
+// ./bin/flang-new -fc1 -load lib/flangOmpReport.so -plugin flang-omp-report
+// -fopenmp
 //
 //===----------------------------------------------------------------------===//
 
 #include "FlangOmpReportVisitor.h"
 
-#include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/FrontendActions.h"
 #include "flang/Frontend/FrontendPluginRegistry.h"
 #include "flang/Parser/dump-parse-tree.h"
 #include "llvm/Support/YAMLParser.h"
 #include "llvm/Support/YAMLTraits.h"
 
 using namespace Fortran::frontend;
 using namespace Fortran::parser;
template <> struct MappingTraits<LogRecord> {
   }
 };
 } // namespace yaml
 } // namespace llvm
 
 class FlangOmpReport : public PluginParseTreeAction {
   void ExecuteAction() override {
     // Prepare the parse tree and the visitor
-    CompilerInstance &ci = this->instance();
-    Parsing &parsing = ci.parsing();
-    const Program &parseTree = *parsing.parseTree();
+    Parsing &parsing = getParsing();
     OpenMPCounterVisitor visitor;
     visitor.parsing = &parsing;
 
     // Walk the parse tree
-    Walk(parseTree, visitor);
+    Walk(parsing.parseTree(), visitor);
 
     // Dump the output
-    std::unique_ptr<llvm::raw_pwrite_stream> OS{ci.CreateDefaultOutputFile(
-        /*Binary=*/true, /*InFile=*/GetCurrentFileOrBufferName(),
-        /*Extension=*/".yaml")};
+    std::unique_ptr<llvm::raw_pwrite_stream> OS{
+        createOutputFile(/*extension=*/"yaml")};
     llvm::yaml::Output yout(*OS);
+
     yout << visitor.constructClauses;
   }
 };
 
 static FrontendPluginRegistry::Add<FlangOmpReport> X("flang-omp-report",
     "Generate a YAML summary of OpenMP constructs and clauses");

flang/examples/FlangOmpReport/yaml_summarizer.py

 """YAML Summariser
 
-The flang plugin ``flang-omp-report`` takes one fortran
+The flang plugin ``flang-omp-report`` takes one Fortran
 file in and returns a YAML report file of the input file.
 This becomes an issue when you want to analyse an entire project
 into one final report.
 The purpose of this Python script is to generate a final YAML
 summary from all of the files generated by ``flang-omp-report``.
 
 Currently, it requires ``ruamel.yaml``,
 which can be installed with:

flang/examples/PrintFlangFunctionNames/PrintFlangFunctionNames.cpp

 // SubroutineSubprogram so a Bool can be set to show that it is the definition
 // of a function/subroutine, and not print those that are in an Interface.
 // This plugin does not recognise Statement Functions or Module Procedures,
 // which could be dealt with through StmtFunctionStmt and MpSubprogramStmt nodes
 // respectively.
 //
 //===----------------------------------------------------------------------===//
 
-#include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/FrontendActions.h"
 #include "flang/Frontend/FrontendPluginRegistry.h"
 #include "flang/Parser/dump-parse-tree.h"
 #include "flang/Parser/parsing.h"
 
 using namespace Fortran::frontend;
 
 class PrintFunctionNamesAction : public PluginParseTreeAction {
struct ParseTreeVisitor {
     int fcounter{0};
     int scounter{0};
 
   private:
     bool isInSubprogram_{false};
   };
 
   void ExecuteAction() override {
-    auto &parseTree{instance().parsing().parseTree()};
-
     ParseTreeVisitor visitor;
-    Fortran::parser::Walk(parseTree, visitor);
+    Fortran::parser::Walk(getParsing().parseTree(), visitor);
 
     llvm::outs() << "\n====   Functions: " << visitor.fcounter << " ====\n";
     llvm::outs() << "==== Subroutines: " << visitor.scounter << " ====\n";
   }
 };
 
 static FrontendPluginRegistry::Add<PrintFunctionNamesAction> X(
     "print-fns", "Print Function names");

flang/include/flang/Frontend/FrontendActions.h

 //===- FrontendActions.h -----------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_FLANG_FRONTEND_FRONTENDACTIONS_H
 #define LLVM_FLANG_FRONTEND_FRONTENDACTIONS_H
 
 #include "flang/Frontend/FrontendAction.h"
+#include "flang/Parser/parsing.h"
 #include "flang/Semantics/semantics.h"
 
 #include "mlir/IR/BuiltinOps.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Module.h"
 #include <memory>
 
 namespace Fortran::frontend {
 
 // TODO: This is a copy from f18.cpp. It doesn't really belong here and should
 // be moved to a more suitable place in future.
 struct MeasurementVisitor {

 };
 
 class ParseSyntaxOnlyAction : public PrescanAndSemaAction {
   void ExecuteAction() override;
 };
 
 class PluginParseTreeAction : public PrescanAndSemaAction {
   void ExecuteAction() override = 0;
+
+public:
+  Fortran::parser::Parsing &getParsing();
+  /// Creates an output file. This is just a wrapper for calling
+  /// CreateDefaultOutputFile from CompilerInstance. Use it to make sure that
+  /// your plugin respects driver's `-o` flag.
+  /// \param extension  The extension to use for the output file (ignored when
+  ///                   the user decides to print to stdout via `-o -`)
+  /// \return           Null on error, ostream for the output file otherwise
+  std::unique_ptr<llvm::raw_pwrite_stream> createOutputFile(
+      llvm::StringRef extension);
 };
 
 //===----------------------------------------------------------------------===//
 // PrescanAndSemaDebug Actions
 //
 // These actions will parse the input, run the semantic checks and execute
 // their actions _regardless of_ whether any semantic errors have been found.
 // This can be useful when adding new languge feature and when you wish to

flang/lib/Frontend/FrontendActions.cpp

if (auto ast = Fortran::lower::createPFT(
     Fortran::lower::dumpPFT(llvm::outs(), *ast);
     return;
   }
 
   unsigned DiagID = ci.diagnostics().getCustomDiagID(
       clang::DiagnosticsEngine::Error, "Pre FIR Tree is NULL.");
   ci.diagnostics().Report(DiagID);
 }
+
+Fortran::parser::Parsing &PluginParseTreeAction::getParsing() {
+  return instance().parsing();
+}
+
+std::unique_ptr<llvm::raw_pwrite_stream>
+PluginParseTreeAction::createOutputFile(llvm::StringRef extension = "") {
+
+  std::unique_ptr<llvm::raw_pwrite_stream> OS{
+      instance().CreateDefaultOutputFile(
+          /*Binary=*/false, /*InFile=*/GetCurrentFileOrBufferName(),
+          extension)};
+  return OS;
+}