[PATCH] D109890: Flang OpenMP Report Plugin

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added a comment.
Herald added a project: Flang.

>From your summary:

> The plugin outputs a single file in the following format: summary-<source-file>.yaml

This is no longer accurate. Similarly here:

> Running the plugin:
> ./bin/flang-new -fc1 -load lib/flangOmpReport.so -plugin fomp-report-yaml -fopenmp <source_file.f90>

The plugin name is `flang-omp-report`.

In order to fix the clang-tidy check, try updating the header guard to: `FORTRAN_FLANG_OMP_REPORT_VISITOR_H`



================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report-visitor.h:21
+
+using namespace Fortran::parser;
+struct ClauseInfo {
----------------
We should avoid using `using namespace <namespace>` in header files. Otherwise,  all files that include this file will have their local namespaces polluted with `Fortran::parser`. That might be undesired.


================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report.cpp:11
+// line they're located on.
+// The report is generated in the same location as the source file.
+//
----------------
This will depend on how the plugin is invoked. For example,
```
./bin/flang-new -fc1 -load lib/flangOmpReport.so -plugin fomp-report-yaml -o - <source_file.f90>
```
prints to stdout. I suggest deleting this sentence and using ^^^ for your example (so that people see the output immediately when they run the plugin for the first time).


================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report.cpp:15
+// ./bin/flang-new -fc1 -load lib/flangOmpReport.so -plugin
+// fomp-report-yaml -fopen <source_file.f90>
+//
----------------
`-fopen`?


================
Comment at: flang/examples/flang-omp-report-plugin/flang-omp-report.cpp:57-73
+class FlangOmpReport : public PluginParseTreeAction {
+  void ExecuteAction() override {
+    CompilerInstance &ci = this->instance();
+
+    OpenMPCounterVisitor visitor;
+    Fortran::parser::Parsing &parsing = instance().parsing();
+    visitor.parsing = &parsing;
----------------
Small suggestion as how to split this into "functional" blocks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109890