[flang][driver] Add `-fno-analyzed-objects-for-unparse`

[awarzynski]

Summary

This patch adds a new option for the new Flang driver:
-fno-analyzed-objects-for-unparse. The semantics are similar to
-funparse-typed-exprs-to-f18-fc from f18. For consistency, the
latter is replaced with -fno-analyzed-objects-for-unparse.

The new option controls the behaviour of the unparser (i.e. the action
corresponding to -fdebug-unparse). The default behaviour is to use the
analyzed objects when unparsing. The new flag can be used to turn this
off, so that the original parse-tree objects are used. The analyzed
objects are generated during the semantic checks [1].

This patch also updates the semantics of
-fno-analyzed-objects-for-unparse/-funparse-typed-exprs-to-f18-fc
in f18, so that this flag is always taken into account when Unparse
is used (this way the semantics in f18 and flang-new are identical).

The added test file is based on example from Peter Steinfeld.

[1] https://github.com/llvm/llvm-project/blob/main/flang/docs/Semantics.md

[awarzynski]

Rebased on top of D103613

[awarzynski]

Rebase on top of main, revert one small change uploaded accidentally

[awarzynski]

Fix build and test failure

[awarzynski]

Rebase on top of main

[awarzynski]

Refine the semantics after some discussion offline

[klausler]

The default behaviour is to always decorate unparsed typed expression with e.g. their KIND. The new flag can be used to turn this off, so that the generated output uses valid Fortran syntax and can be fed to another Fortran compiler.

The output of Expr<T>::AsFortran() should be valid Fortran, and it's a bug if it's not.

EDIT: To be more clear: the output of Expr<T>::AsFortran() should be syntactically valid Fortran, but it may contain references to symbols that might cause errors when recompiled; for example, references to generic interfaces will have been resolved to specific subprograms, and it's possible that those identifiers might be PRIVATE or not in scope. But the syntax of the dump of the analyzed expression should be valid Fortran.

[awarzynski]

In D103612#2827444, @klausler wrote:

The default behaviour is to always decorate unparsed typed expression with e.g. their KIND. The new flag can be used to turn this off, so that the generated output uses valid Fortran syntax and can be fed to another Fortran compiler.

The output of Expr<T>::AsFortran() should be valid Fortran, and it's a bug if it's not.

This is the output that I get from the unparser (input file: flang/test/Driver/unparse-typed-exprs.f95):

PROGRAM test_allocated
 INTEGER :: i = 13_4
 REAL(KIND=2_4), ALLOCATABLE :: x(:)
 IF (.NOT.allocated(x)) ALLOCATE(x(i))
END PROGRAM test_allocated

This is not valid, is it? Or am I missing something?

[klausler]

In D103612#2827458, @awarzynski wrote:

In D103612#2827444, @klausler wrote:

The default behaviour is to always decorate unparsed typed expression with e.g. their KIND. The new flag can be used to turn this off, so that the generated output uses valid Fortran syntax and can be fed to another Fortran compiler.

The output of Expr<T>::AsFortran() should be valid Fortran, and it's a bug if it's not.

This is the output that I get from the unparser (input file: flang/test/Driver/unparse-typed-exprs.f95):

PROGRAM test_allocated
 INTEGER :: i = 13_4
 REAL(KIND=2_4), ALLOCATABLE :: x(:)
 IF (.NOT.allocated(x)) ALLOCATE(x(i))
END PROGRAM test_allocated

This is not valid, is it? Or am I missing something?

Kind suffixes are described in subclause 7.4.3 of Fortran 2018, e.g. R708 on p. 58.

[awarzynski]

In D103612#2827468, @klausler wrote:

Kind suffixes are described in subclause 7.4.3 of Fortran 2018, e.g. R708 on p. 58.

Many thanks for this reference.

So when calling Unparse, one specifes AnalyzedObjectsAsFortran (i.e. the optional last argument) to get more semantic context in the dump, right?

[klausler]

In D103612#2830111, @awarzynski wrote:

In D103612#2827468, @klausler wrote:

Kind suffixes are described in subclause 7.4.3 of Fortran 2018, e.g. R708 on p. 58.

Many thanks for this reference.

So when calling Unparse, one specifes AnalyzedObjectsAsFortran (i.e. the optional last argument) to get more semantic context in the dump, right?

It's for better code coverage when testing. When the internal representations of expressions & variables are dumped by unparsing rather than the original parse trees, it allows semantic analysis (esp. folding) to be exercised. If an unparsed program compiles and runs fine when unparsed into gfortran without this flag, but fails when the flag is present, there's a bug. This feature allows much of the frontend to undergo some testing and debugging in the absence of a code generator.

It's also useful for self-testing via a "parse/unparse/reparse/unparse" mode, in which we recompile unparsed Fortran and check that it unparses to the same output.

[awarzynski]

Rename the new flag as -fno-analyzed-objects-for-unparse

Based on the discussion, I've renamed the flag to better reflect the intention. Does it make more sense now?

Note, the default behaviour of the drivers does not change with this patch (i.e. {f18|flang-new} -fdebug-unparse will generate identical results _before_ and _after_).

@klousler & @PeteSteinfeld, many thanks for all the feedback!

[PeteSteinfeld]

All builds, tests, and looks good.

[klausler]

This patch may have broken the shared library buildbots.

[awarzynski]

In D103612#2840886, @klausler wrote:

This patch may have broken the shared library buildbots.

Sorry about that. A fix is on its way.

[awarzynski]

I pushed a fix without a review: https://reviews.llvm.org/rGc3ebb53eabb7f851687f66ada88aa16f768d76ce. Please let me know if you prefer such changes to go through a regular review in the future!

clang/include/clang/Driver/Options.td

def fdebug_measure_parse_tree : Flag<["-"], "fdebug-measure-parse-tree">, Group<Action_Group>,
   HelpText<"Measure the parse tree">;
 def fdebug_pre_fir_tree : Flag<["-"], "fdebug-pre-fir-tree">, Group<Action_Group>,
   HelpText<"Dump the pre-FIR tree">;
 def fdebug_module_writer : Flag<["-"],"fdebug-module-writer">, 
   HelpText<"Enable debug messages while writing module files">;
 def fget_symbols_sources : Flag<["-"], "fget-symbols-sources">, Group<Action_Group>,
   HelpText<"Dump symbols and their source code locations">;
 
+def fno_unparse_typed_exprs : Flag<["-"],
+  "fno-unparse-typed-exprs">,  Group<f_Group>,
+  HelpText<"Don't unparse typed expressions">;
+
 }
 
 //===----------------------------------------------------------------------===//
 // CC1 Options
 //===----------------------------------------------------------------------===//
 
 let Flags = [CC1Option, NoDriverOption] in {
 

flang/include/flang/Frontend/CompilerInvocation.h

class CompilerInvocation : public CompilerInvocationBase {
   // TODO: Merge with or translate to frontendOpts_. We shouldn't need two sets
   // of options.
   std::string moduleDir_ = ".";
 
   bool debugModuleDir_ = false;
 
   bool warnAsErr_ = false;
 
+  bool unparseTypedExprs_ = true;
+
   // Fortran Dialect options
   Fortran::common::IntrinsicTypeDefaultKinds defaultKinds_;
 
   bool EnableConformanceChecks_ = false;
 
+  Fortran::parser::AnalyzedObjectsAsFortran AsFortran_{
+      [](llvm::raw_ostream &o, const Fortran::evaluate::GenericExprWrapper &x) {
+        if (x.v) {
+          x.v->AsFortran(o);
+        } else {
+          o << "(bad expression)";
+        }
+      },
+      [](llvm::raw_ostream &o,
+          const Fortran::evaluate::GenericAssignmentWrapper &x) {
+        if (x.v) {
+          x.v->AsFortran(o);
+        } else {
+          o << "(bad assignment)";
+        }
+      },
+      [](llvm::raw_ostream &o, const Fortran::evaluate::ProcedureRef &x) {
+        x.AsFortran(o << "CALL ");
+      },
+  };
+
 public:
   CompilerInvocation() = default;
 
   FrontendOptions &frontendOpts() { return frontendOpts_; }
   const FrontendOptions &frontendOpts() const { return frontendOpts_; }
 
   Fortran::parser::Options &fortranOpts() { return parserOpts_; }
   const Fortran::parser::Options &fortranOpts() const { return parserOpts_; }
public:
   const std::string &moduleDir() const { return moduleDir_; }
 
   bool &debugModuleDir() { return debugModuleDir_; }
   const bool &debugModuleDir() const { return debugModuleDir_; }
 
   bool &warnAsErr() { return warnAsErr_; }
   const bool &warnAsErr() const { return warnAsErr_; }
 
+  bool &unparseTypedExprs() { return unparseTypedExprs_; }
+  const bool &unaprseTypedExprsAsFortran() const { return unparseTypedExprs_; }
+
   bool &enableConformanceChecks() { return EnableConformanceChecks_; }
   const bool &enableConformanceChecks() const {
     return EnableConformanceChecks_;
   }
 
+  Fortran::parser::AnalyzedObjectsAsFortran &asFortran() { return AsFortran_; }
+  const Fortran::parser::AnalyzedObjectsAsFortran &asFortran() const {
+    return AsFortran_;
+  }
+
   Fortran::common::IntrinsicTypeDefaultKinds &defaultKinds() {
     return defaultKinds_;
   }
   const Fortran::common::IntrinsicTypeDefaultKinds &defaultKinds() const {
     return defaultKinds_;
   }
 
   /// Create a compiler invocation from a list of input options.
public:
 
   /// Useful setters
   void SetModuleDir(std::string &moduleDir) { moduleDir_ = moduleDir; }
 
   void SetDebugModuleDir(bool flag) { debugModuleDir_ = flag; }
 
   void SetWarnAsErr(bool flag) { warnAsErr_ = flag; }
 
+  void SetUnparseTypedExprs(bool flag) { unparseTypedExprs_ = flag; }
+
   /// Set the Fortran options to predifined defaults. These defaults are
   /// consistend with f18/f18.cpp.
   // TODO: We should map frontendOpts_ to parserOpts_ instead. For that, we
   // need to extend frontendOpts_ first. Next, we need to add the corresponding
   // compiler driver options in libclangDriver.
   void SetDefaultFortranOpts();
 
   /// Set the default predefinitions.

flang/lib/Frontend/CompilerInvocation.cpp

static bool parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
   if (moduleDirList.size() == 1)
     res.SetModuleDir(moduleDirList[0]);
 
   // -fdebug-module-writer option
   if (args.hasArg(clang::driver::options::OPT_fdebug_module_writer)) {
     res.SetDebugModuleDir(true);
   }
 
+  // -fno-unparse-typed-exprs
+  if (args.hasArg(clang::driver::options::OPT_fno_unparse_typed_exprs)) {
+    res.SetUnparseTypedExprs(false);
+  }
+
   return diags.getNumErrors() == numErrorsBefore;
 }
 
 /// Parses all diagnostics related arguments and populates the variables
 /// options accordingly. Returns false if new errors are generated.
 static bool parseDiagArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
     clang::DiagnosticsEngine &diags) {
   unsigned numErrorsBefore = diags.getNumErrors();

flang/lib/Frontend/FrontendActions.cpp

 }
 
 void ParseSyntaxOnlyAction::ExecuteAction() {
   reportFatalSemanticErrors(semantics(), this->instance().diagnostics(),
       GetCurrentFileOrBufferName());
 }
 
 void DebugUnparseNoSemaAction::ExecuteAction() {
+  auto &invoc = this->instance().invocation();
   auto &parseTree{instance().parsing().parseTree()};
 
-  Fortran::parser::AnalyzedObjectsAsFortran asFortran =
-      Fortran::frontend::getBasicAsFortran();
-
   // TODO: Options should come from CompilerInvocation
   Unparse(llvm::outs(), *parseTree,
       /*encoding=*/Fortran::parser::Encoding::UTF_8,
       /*capitalizeKeywords=*/true, /*backslashEscapes=*/false,
-      /*preStatement=*/nullptr, &asFortran);
+      /*preStatement=*/nullptr,
+      invoc.unparseTypedExprs() ? &invoc.asFortran() : nullptr);
 }
 
 void DebugUnparseAction::ExecuteAction() {
+  auto &invoc = this->instance().invocation();
   auto &parseTree{instance().parsing().parseTree()};
-  Fortran::parser::AnalyzedObjectsAsFortran asFortran =
-      Fortran::frontend::getBasicAsFortran();
 
   // TODO: Options should come from CompilerInvocation
   Unparse(llvm::outs(), *parseTree,
       /*encoding=*/Fortran::parser::Encoding::UTF_8,
       /*capitalizeKeywords=*/true, /*backslashEscapes=*/false,
-      /*preStatement=*/nullptr, &asFortran);
+      /*preStatement=*/nullptr,
+      invoc.unparseTypedExprs() ? &invoc.asFortran() : nullptr);
 
   // Report fatal semantic errors
   reportFatalSemanticErrors(semantics(), this->instance().diagnostics(),
       GetCurrentFileOrBufferName());
 }
 
 void DebugUnparseWithSymbolsAction::ExecuteAction() {
   auto &parseTree{*instance().parsing().parseTree()};
void DebugDumpSymbolsAction::ExecuteAction() {
   }
 
   // Dump symbols
   semantics.DumpSymbols(llvm::outs());
 }
 
 void DebugDumpParseTreeNoSemaAction::ExecuteAction() {
   auto &parseTree{instance().parsing().parseTree()};
-  Fortran::parser::AnalyzedObjectsAsFortran asFortran =
-      Fortran::frontend::getBasicAsFortran();
 
   // Dump parse tree
-  Fortran::parser::DumpTree(llvm::outs(), parseTree, &asFortran);
+  Fortran::parser::DumpTree(
+      llvm::outs(), parseTree, &this->instance().invocation().asFortran());
 }
 
 void DebugDumpParseTreeAction::ExecuteAction() {
   auto &parseTree{instance().parsing().parseTree()};
-  Fortran::parser::AnalyzedObjectsAsFortran asFortran =
-      Fortran::frontend::getBasicAsFortran();
 
   // Dump parse tree
-  Fortran::parser::DumpTree(llvm::outs(), parseTree, &asFortran);
+  Fortran::parser::DumpTree(
+      llvm::outs(), parseTree, &this->instance().invocation().asFortran());
+
   // Report fatal semantic errors
   reportFatalSemanticErrors(semantics(), this->instance().diagnostics(),
       GetCurrentFileOrBufferName());
 }
 
 void DebugMeasureParseTreeAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
 

flang/lib/Frontend/FrontendOptions.cpp

 }
 
 bool Fortran::frontend::mustBePreprocessed(llvm::StringRef suffix) {
   return suffix == "F" || suffix == "FOR" || suffix == "fpp" ||
       suffix == "FPP" || suffix == "F90" || suffix == "F95" ||
       suffix == "F03" || suffix == "F08" || suffix == "F18";
 }
 
-// TODO: This is a copy of `asFortran` from f18.cpp and is added here for
-// compatiblity. It doesn't really belong here, but I couldn't find a better
-// place. We should decide whether to add it to the Evaluate or Parse/Unparse
-// APIs or some dedicated utility library in the driver.
-Fortran::parser::AnalyzedObjectsAsFortran
-Fortran::frontend::getBasicAsFortran() {
-  return Fortran::parser::AnalyzedObjectsAsFortran{
-      [](llvm::raw_ostream &o, const Fortran::evaluate::GenericExprWrapper &x) {
-        if (x.v) {
-          x.v->AsFortran(o);
-        } else {
-          o << "(bad expression)";
-        }
-      },
-      [](llvm::raw_ostream &o,
-          const Fortran::evaluate::GenericAssignmentWrapper &x) {
-        if (x.v) {
-          x.v->AsFortran(o);
-        } else {
-          o << "(bad assignment)";
-        }
-      },
-      [](llvm::raw_ostream &o, const Fortran::evaluate::ProcedureRef &x) {
-        x.AsFortran(o << "CALL ");
-      },
-  };
-}
-
 InputKind FrontendOptions::GetInputKindForExtension(llvm::StringRef extension) {
   if (isFixedFormSuffix(extension) || isFreeFormSuffix(extension)) {
     return Language::Fortran;
   }
   return Language::Unknown;
 }

flang/test/Driver/unparse-typed-exprs.f95

+! Tests `-fno-unparse-typed-exprs-to-f18-fc` frontend option
+
+!--------------------------
+! RUN lines
+!--------------------------
+! RUN: %flang_fc1 -fdebug-unparse  %s | FileCheck %s --check-prefix=UNPARSED_TYPED_EXPR
+! RUN: %flang_fc1 -fdebug-unparse -fno-unparse-typed-exprs %s | FileCheck %s --check-prefix=AS_FORTRAN
+
+!--------------------------
+! EXPECTED OUTPUT: default
+!--------------------------
+! UNPARSED_TYPED_EXPR: PROGRAM test_allocated
+! UNPARSED_TYPED_EXPR-NEXT:  INTEGER :: i = 4_4
+! UNPARSED_TYPED_EXPR-NEXT:  REAL(KIND=4_4), ALLOCATABLE :: x(:)
+! UNPARSED_TYPED_EXPR-NEXT:  IF (.NOT.allocated(x)) ALLOCATE(x(i))
+! UNPARSED_TYPED_EXPR-NEXT: END PROGRAM test_allocated
+
+!-------------------------------------
+! EXPECTED OUTPUT: unparsed as Fortran
+!--------------------------------------
+! AS_FORTRAN: PROGRAM test_allocated
+! AS_FORTRAN-NEXT:  INTEGER :: i = 4
+! AS_FORTRAN-NEXT:  REAL(KIND=4), ALLOCATABLE :: x(:)
+! AS_FORTRAN-NEXT:  IF (.NOT.allocated(x)) ALLOCATE(x(i))
+! AS_FORTRAN-NEXT: END PROGRAM test_allocated
+
+!--------------------------
+! INPUT
+!--------------------------
+program test_allocated
+  integer :: i = 4
+  real(4), allocatable :: x(:)
+  if (.not. allocated(x)) allocate(x(i))
+end program test_allocated

flang/tools/f18/f18.cpp

struct DriverOptions {
   bool dumpUnparseWithSymbols{false};
   bool dumpParseTree{false};
   bool dumpPreFirTree{false};
   bool dumpSymbols{false};
   bool debugNoSemantics{false};
   bool debugModuleWriter{false};
   bool defaultReal8{false};
   bool measureTree{false};
-  bool unparseTypedExprsToF18_FC{false};
+  bool unparseTypedExprs{true};
   std::vector<std::string> F18_FCArgs;
   const char *prefix{nullptr};
   bool getDefinition{false};
   GetDefinitionArgs getDefinitionArgs{0, 0, 0};
   bool getSymbolsSources{false};
   std::optional<bool> forcePreprocessing; // -cpp & -nocpp
 };
 
std::string CompileFortran(std::string path, Fortran::parser::Options options,
   }
   if (driver.dumpParseTree) {
     Fortran::parser::DumpTree(llvm::outs(), parseTree, &asFortran);
   }
   if (driver.dumpUnparse) {
     Unparse(llvm::outs(), parseTree, driver.encoding, true /*capitalize*/,
         options.features.IsEnabled(
             Fortran::common::LanguageFeature::BackslashEscapes),
-        nullptr /* action before each statement */, &asFortran);
+        nullptr /* action before each statement */,
+        driver.unparseTypedExprs ? &asFortran : nullptr);
     return {};
   }
   if (driver.dumpPreFirTree) {
     if (auto ast{Fortran::lower::createPFT(parseTree, semanticsContext)}) {
       Fortran::lower::dumpPFT(llvm::outs(), *ast);
     } else {
       llvm::errs() << "Pre FIR Tree is NULL.\n";
       exitStatus = EXIT_FAILURE;
if (EC) {
       llvm::errs() << EC.message() << "\n";
       std::exit(EXIT_FAILURE);
     }
     llvm::raw_fd_ostream tmpSource(fd, /*shouldClose*/ true);
     Unparse(tmpSource, parseTree, driver.encoding, true /*capitalize*/,
         options.features.IsEnabled(
             Fortran::common::LanguageFeature::BackslashEscapes),
         nullptr /* action before each statement */,
-        driver.unparseTypedExprsToF18_FC ? &asFortran : nullptr);
+        driver.unparseTypedExprs ? &asFortran : nullptr);
   }
 
   RunOtherCompiler(driver, tmpSourcePath.data(), relo.data());
-  filesToDelete.emplace_back(tmpSourcePath);
+  // filesToDelete.emplace_back(tmpSourcePath);
   if (!driver.compileOnly && driver.outputPath.empty()) {
     filesToDelete.push_back(relo);
   }
   return relo;
 }
 
 std::string CompileOtherLanguage(std::string path, DriverOptions &driver) {
   std::string relo{RelocatableName(driver, path)};
if (arg.empty()) {
       driver.debugNoSemantics = true;
       driver.dumpParseTree = true;
       driver.syntaxOnly = true;
     } else if (arg == "-funparse" || arg == "-fdebug-unparse") {
       driver.dumpUnparse = true;
     } else if (arg == "-funparse-with-symbols" ||
         arg == "-fdebug-unparse-with-symbols") {
       driver.dumpUnparseWithSymbols = true;
-    } else if (arg == "-funparse-typed-exprs-to-f18-fc") {
-      driver.unparseTypedExprsToF18_FC = true;
+    } else if (arg == "-fno-unparse-typed-exprs") {
+      driver.unparseTypedExprs = false;
     } else if (arg == "-fparse-only" || arg == "-fsyntax-only") {
       driver.syntaxOnly = true;
     } else if (arg == "-c") {
       driver.compileOnly = true;
     } else if (arg == "-o") {
       driver.outputPath = args.front();
       args.pop_front();
     } else if (arg.substr(0, 2) == "-D") {

flang/tools/f18/flang

 #!/bin/bash
 #===-- tools/f18/flang.sh -----------------------------------------*- sh -*-===#
 #
 # 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
 #
 #===------------------------------------------------------------------------===#
 
 wd=$(cd $(dirname "$0")/.. && pwd)
-opts="-module-suffix .f18.mod "
+opts="-fno-unparse-typed-exprs -module-suffix .f18.mod "
 if ! $wd/bin/f18 $opts "$@"
 then status=$?
      echo flang: in $PWD, f18 failed with exit status $status: $wd/bin/f18 $opts "$@" >&2
      exit $status
 fi