[llvm-branch-commits] [flang] aba24c1 - [flang][driver] Move fixed/free from detection out of FrontendAction API

[Andrzej Warzynski via llvm-branch-commits]

Author: Andrzej Warzynski
Date: 2021-01-21T16:57:22Z
New Revision: aba24c1580cfafc387d94add3855cb4a1e90db3c

URL: https://github.com/llvm/llvm-project/commit/aba24c1580cfafc387d94add3855cb4a1e90db3c
DIFF: https://github.com/llvm/llvm-project/commit/aba24c1580cfafc387d94add3855cb4a1e90db3c.diff

LOG: [flang][driver] Move fixed/free from detection out of FrontendAction API

All Fortran options should be set in `CompilerInstance` (via its
`CompilerInvocation`) before any of `FrontendAction` is entered -
that's one of the tasks of the driver. However, this is a bit tricky
with fixed and free from detection introduced in
https://reviews.llvm.org/D94228.

Fixed-free form detection needs to happen:
  * before any frontend action (we need to specify `isFixedForm` in
    `Fortran::parser::Options` before running any actions)
  * separately for every input file (we might be compiling multiple
    Fortran files, some in free form, some in fixed form)
In other words, we need this to happen early (before any
`FrontendAction`), but not too early (we need to know what the current
input file is). In practice, `isFixedForm` can only be set later
than other options (other options are inferred from compiler flags). So
we can't really set all of them in one place, which is not ideal.

All changes in this patch are NFCs (hence no new tests). Quick summary:
  * move fixed/free form detection from `FrontendAction::ExecuteAction` to
    `CompilerInstance::ExecuteAction`
  * add a bool flag in `FrontendInputFile` to mark a file as fixed/free
    form
  * updated a few comments

Differential Revision: https://reviews.llvm.org/D95042

Added: 
    

Modified: 
    flang/include/flang/Frontend/FrontendOptions.h
    flang/lib/Frontend/CompilerInstance.cpp
    flang/lib/Frontend/FrontendAction.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Frontend/FrontendOptions.h b/flang/include/flang/Frontend/FrontendOptions.h
index 9bebf6c8f35d..44fac37cd322 100644
--- a/flang/include/flang/Frontend/FrontendOptions.h
+++ b/flang/include/flang/Frontend/FrontendOptions.h
@@ -104,10 +104,22 @@ class FrontendInputFile {
   /// The kind of input, atm it contains language
   InputKind kind_;
 
+  /// Is this input file in fixed-form format? This is simply derived from the
+  /// file extension and should not be altered by consumers. For input from
+  /// stdin this is never modified.
+  bool isFixedForm_ = false;
+
 public:
   FrontendInputFile() = default;
   FrontendInputFile(llvm::StringRef file, InputKind kind)
-      : file_(file.str()), kind_(kind) {}
+      : file_(file.str()), kind_(kind) {
+
+    // Based on the extension, decide whether this is a fixed or free form
+    // file.
+    auto pathDotIndex{file.rfind(".")};
+    std::string pathSuffix{file.substr(pathDotIndex + 1)};
+    isFixedForm_ = isFixedFormSuffix(pathSuffix);
+  }
   FrontendInputFile(const llvm::MemoryBuffer *buffer, InputKind kind)
       : buffer_(buffer), kind_(kind) {}
 
@@ -116,6 +128,7 @@ class FrontendInputFile {
   bool IsEmpty() const { return file_.empty() && buffer_ == nullptr; }
   bool IsFile() const { return !IsBuffer(); }
   bool IsBuffer() const { return buffer_ != nullptr; }
+  bool IsFixedForm() const { return isFixedForm_; }
 
   llvm::StringRef file() const {
     assert(IsFile());

diff  --git a/flang/lib/Frontend/CompilerInstance.cpp b/flang/lib/Frontend/CompilerInstance.cpp
index 589f8204c60f..8e9ee0ec671c 100644
--- a/flang/lib/Frontend/CompilerInstance.cpp
+++ b/flang/lib/Frontend/CompilerInstance.cpp
@@ -138,16 +138,22 @@ void CompilerInstance::ClearOutputFiles(bool eraseFiles) {
 }
 
 bool CompilerInstance::ExecuteAction(FrontendAction &act) {
+  auto &invoc = this->invocation();
+
   // Set some sane defaults for the frontend.
-  // TODO: Instead of defaults we should be setting these options based on the
-  // user input.
-  this->invocation().SetDefaultFortranOpts();
-  // Set the fortran options to user-based input.
-  this->invocation().setFortranOpts();
+  invoc.SetDefaultFortranOpts();
+  // Update the fortran options based on user-based input.
+  invoc.setFortranOpts();
 
-  // Connect Input to a CompileInstance
+  // Run the frontend action `act` for every input file.
   for (const FrontendInputFile &fif : frontendOpts().inputs_) {
     if (act.BeginSourceFile(*this, fif)) {
+      // Switch between fixed and free form format based on the input file
+      // extension. Ideally we should have all Fortran options set before
+      // entering this loop (i.e. processing any input files). However, we
+      // can't decide between fixed and free form based on the file extension
+      // earlier than this.
+      invoc.fortranOpts().isFixedForm = fif.IsFixedForm();
       if (llvm::Error err = act.Execute()) {
         consumeError(std::move(err));
       }

diff  --git a/flang/lib/Frontend/FrontendAction.cpp b/flang/lib/Frontend/FrontendAction.cpp
index dad2da683860..1a403bb56a26 100644
--- a/flang/lib/Frontend/FrontendAction.cpp
+++ b/flang/lib/Frontend/FrontendAction.cpp
@@ -53,12 +53,6 @@ llvm::Error FrontendAction::Execute() {
 
   Fortran::parser::Options parserOptions =
       this->instance().invocation().fortranOpts();
-  // Set the fixed form flag based on the file extension
-  auto pathDotIndex{currentInputPath.rfind(".")};
-  if (pathDotIndex != std::string::npos) {
-    std::string pathSuffix{currentInputPath.substr(pathDotIndex + 1)};
-    parserOptions.isFixedForm = isFixedFormSuffix(pathSuffix);
-  }
 
   // Prescan. In case of failure, report and return.
   ci.parsing().Prescan(currentInputPath, parserOptions);