[flang] Don't perform macro replacement unless *.F, *.F90, &c.

[klausler]

Summary

Avoid spurious and confusing macro replacements from things like
-DPIC on Fortran source files whose suffixes indicate that preprocessing
is not expected.

Add gfortran-like "-cpp" and "-nocpp" flags to f18 to force predefinition
of macros independent of the source file suffix.

[PeteSteinfeld]

f18.cpp needs to have clang-format run on it. Otherwise, all builds, tests, and looks good.

[awarzynski]

Hello @klausler, thank you for working on this!

@PeteSteinfeld and @klausler, could you please clarify the following before this is merged. I want to make sure that the two Flang compiler drivers don't diverge too much.

Q1:
According Parsing.md:

The preprocessor always runs.

Doesn't this patch change this? Or indicate that such change is coming? If that's the case, could you please update Parsing.md?

Q2:
This patch adds -cpp/-nocpp flags. Are these meant to be used to enable/disable preprocessing? (like in e.g. gfortran). If I understand correctly, currently even with -nocpp the preprocessor is still run.

Q3:
Could the following condition be extracted into a separate method (e.g. bool shouldRunPP):

if (suffix == "F" || suffix == "F90" || suffix == "F95" ||
    suffix == "CUF" || suffix == "F18") {
  options.predefinitions = predefinitions;

? It would be very helpful if this could be shared between the drivers. Alternatively, could you suggest _where_ such method could be added?

The new driver assumes that the preprocessor always runs and adds standard predefinitions unconditionally here. From what I can see, this patch shouldn't cause any of the driver tests to fail, but it does mean that the currently implemented behavior becomes incorrect. I'm happy to update the new driver in a separate patch, but could you clarify Q1, Q2 and Q3 first? Your input is much appreciated!

[klausler]

The behavior of a preprocessor is always present, even with "-nocpp"; preprocessing is an integrated part of prescanning, not a separate pass or process. The preprocessor instances used to be unconditionally initialized with some macro definitions from -D options and built-in names like FILE; those initializations are now conditional, so that unexpected macro replacement will not occur. The motivation for this change is from an application that defined -DPIC on the command line and had a variable named PIC in a *.f90 file.

The list of file suffixes that will observe predefined macros in f18 is probably incomplete; I don't know what to do with *.ff files, for example. I would not view it as being authoritative if I were you.

[awarzynski]

Thank you for clarifying, this helps a lot!

As there is no intention for f18 -nocpp (or flang-new -nocpp) to match the behavior of gfortran -nocpp, could we use different spellings? Also, I doubt that we would be able to re-use -cpp/-nocpp in flang-new for something other than enabling/disabling the preprocessor (these options are already present in Options.td).

I'm guessing that want something with semantics similar to -ffree-form/-ffixed-form. Naming is hard, but let me try suggesting something:

• -fadd-pp-predefs/-fno-pp-predefs (add preprocessor predefinitions)
• -fno-macro-predefs/-fmacro-predefs (add macro predefinitions)
• -fno-std-macro-predefs/-fstd-macro-predefs (add standard macro predefinitions)

Would any of these make sense to you?

[klausler]

In D96464#2557497, @awarzynski wrote:

Thank you for clarifying, this helps a lot!

As there is no intention for f18 -nocpp (or flang-new -nocpp) to match the behavior of gfortran -nocpp, could we use different spellings? Also, I doubt that we would be able to re-use -cpp/-nocpp in flang-new for something other than enabling/disabling the preprocessor (these options are already present in Options.td).

I'm guessing that want something with semantics similar to -ffree-form/-ffixed-form. Naming is hard, but let me try suggesting something:

• -fadd-pp-predefs/-fno-pp-predefs (add preprocessor predefinitions)
• -fno-macro-predefs/-fmacro-predefs (add macro predefinitions)
• -fno-std-macro-predefs/-fstd-macro-predefs (add standard macro predefinitions)

Would any of these make sense to you?

I would not enjoy explaining to a user why they have to modify their decades-old makefiles to build their existing applications with your new driver; I advise that you avoid gratuitous incompatibility.

flang/lib/Parser/parsing.cpp

   }
 
   Preprocessor preprocessor{allSources};
-  for (const auto &predef : options.predefinitions) {
-    if (predef.second) {
-      preprocessor.Define(predef.first, *predef.second);
-    } else {
-      preprocessor.Undefine(predef.first);
+  if (!options.predefinitions.empty()) {
+    preprocessor.DefineStandardMacros();
+    for (const auto &predef : options.predefinitions) {
+      if (predef.second) {
+        preprocessor.Define(predef.first, *predef.second);
+      } else {
+        preprocessor.Undefine(predef.first);
+      }
     }
   }
   currentCooked_ = &allCooked_.NewCookedSource();

flang/lib/Parser/preprocessor.h

   const AllSources &allSources() const { return allSources_; }
   AllSources &allSources() { return allSources_; }
 
+  void DefineStandardMacros();
   void Define(std::string macro, std::string value);
   void Undefine(std::string macro);
   bool IsNameDefined(const CharBlock &);

flang/lib/Parser/preprocessor.cpp

       std::strftime(buffer, sizeof buffer, format, std::localtime(&now))};
 }
 
-Preprocessor::Preprocessor(AllSources &allSources) : allSources_{allSources} {
+Preprocessor::Preprocessor(AllSources &allSources) : allSources_{allSources} {}
+
+void Preprocessor::DefineStandardMacros() {
   // Capture current local date & time once now to avoid having the values
   // of __DATE__ or __TIME__ change during compilation.
   std::time_t now;
   std::time(&now);
-  definitions_.emplace(SaveTokenAsName("__DATE__"s), // e.g., "Jun 16 1904"
-      Definition{FormatTime(now, "\"%h %e %Y\""), allSources});
-  definitions_.emplace(SaveTokenAsName("__TIME__"s), // e.g., "23:59:60"
-      Definition{FormatTime(now, "\"%T\""), allSources});
+  Define("__DATE__"s, FormatTime(now, "\"%h %e %Y\"")); // e.g., "Jun 16 1904"
+  Define("__TIME__"s, FormatTime(now, "\"%T\"")); // e.g., "23:59:60"
   // The values of these predefined macros depend on their invocation sites.
-  definitions_.emplace(
-      SaveTokenAsName("__FILE__"s), Definition{"__FILE__"s, allSources});
-  definitions_.emplace(
-      SaveTokenAsName("__LINE__"s), Definition{"__LINE__"s, allSources});
+  Define("__FILE__"s, "__FILE__"s);
+  Define("__LINE__"s, "__LINE__"s);
 }
 
 void Preprocessor::Define(std::string macro, std::string value) {

 std::optional<TokenSequence> Preprocessor::MacroReplacement(
     const TokenSequence &input, Prescanner &prescanner) {
   // Do quick scan for any use of a defined name.
+  if (definitions_.empty()) {
+    return std::nullopt;
+  }
   std::size_t tokens{input.SizeInTokens()};
   std::size_t j;
   for (j = 0; j < tokens; ++j) {

flang/tools/f18/f18.cpp

   bool getDefinition{false};
   GetDefinitionArgs getDefinitionArgs{0, 0, 0};
   bool getSymbolsSources{false};
+  std::optional<bool> forcePreprocessing; // -cpp & -nocpp
 };
 
 void Exec(std::vector<llvm::StringRef> &argv, bool verbose = false) {

   driver.prefix = prefix.data();
 
   Fortran::parser::Options options;
-  options.predefinitions.emplace_back("__F18", "1");
-  options.predefinitions.emplace_back("__F18_MAJOR__", "1");
-  options.predefinitions.emplace_back("__F18_MINOR__", "1");
-  options.predefinitions.emplace_back("__F18_PATCHLEVEL__", "1");
-  options.predefinitions.emplace_back("__flang__", FLANG_VERSION_STRING);
-  options.predefinitions.emplace_back(
-      "__flang_major__", FLANG_VERSION_MAJOR_STRING);
-  options.predefinitions.emplace_back(
-      "__flang_minor__", FLANG_VERSION_MINOR_STRING);
-  options.predefinitions.emplace_back(
+  std::vector<Fortran::parser::Options::Predefinition> predefinitions;
+  predefinitions.emplace_back("__F18", "1");
+  predefinitions.emplace_back("__F18_MAJOR__", "1");
+  predefinitions.emplace_back("__F18_MINOR__", "1");
+  predefinitions.emplace_back("__F18_PATCHLEVEL__", "1");
+  predefinitions.emplace_back("__flang__", FLANG_VERSION_STRING);
+  predefinitions.emplace_back("__flang_major__", FLANG_VERSION_MAJOR_STRING);
+  predefinitions.emplace_back("__flang_minor__", FLANG_VERSION_MINOR_STRING);
+  predefinitions.emplace_back(
       "__flang_patchlevel__", FLANG_VERSION_PATCHLEVEL_STRING);
 #if __x86_64__
-  options.predefinitions.emplace_back("__x86_64__", "1");
+  predefinitions.emplace_back("__x86_64__", "1");
 #endif
 
   Fortran::common::IntrinsicTypeDefaultKinds defaultKinds;

       driver.warnOnNonstandardUsage = true;
     } else if (arg == "-fopenacc") {
       options.features.Enable(Fortran::common::LanguageFeature::OpenACC);
-      options.predefinitions.emplace_back("_OPENACC", "202011");
+      predefinitions.emplace_back("_OPENACC", "202011");
     } else if (arg == "-fopenmp") {
       options.features.Enable(Fortran::common::LanguageFeature::OpenMP);
-      options.predefinitions.emplace_back("_OPENMP", "201511");
+      predefinitions.emplace_back("_OPENMP", "201511");
     } else if (arg == "-Werror") {
       driver.warningsAreErrors = true;
     } else if (arg == "-ed") {

     } else if (arg.substr(0, 2) == "-D") {
       auto eq{arg.find('=')};
       if (eq == std::string::npos) {
-        options.predefinitions.emplace_back(arg.substr(2), "1");
+        predefinitions.emplace_back(arg.substr(2), "1");
       } else {
-        options.predefinitions.emplace_back(
-            arg.substr(2, eq - 2), arg.substr(eq + 1));
+        predefinitions.emplace_back(arg.substr(2, eq - 2), arg.substr(eq + 1));
       }
     } else if (arg.substr(0, 2) == "-U") {
-      options.predefinitions.emplace_back(
-          arg.substr(2), std::optional<std::string>{});
+      predefinitions.emplace_back(arg.substr(2), std::optional<std::string>{});
     } else if (arg == "-fdefault-double-8") {
       defaultKinds.set_defaultRealKind(4);
     } else if (arg == "-r8" || arg == "-fdefault-real-8") {

       driver.getSymbolsSources = true;
     } else if (arg == "-byteswapio") {
       driver.byteswapio = true; // TODO: Pass to lowering, generate call
+    } else if (arg == "-cpp") {
+      driver.forcePreprocessing = true;
+    } else if (arg == "-nocpp") {
+      driver.forcePreprocessing = false;
     } else if (arg == "-h" || arg == "-help" || arg == "--help" ||
         arg == "-?") {
       llvm::errs()

           << "  -fget-definition\n"
           << "  -fget-symbols-sources\n"
           << "  -v -c -o -I -D -U    have their usual meanings\n"
+          << "  -cpp / -nocpp        force / inhibit macro replacement\n"
           << "  -help                print this again\n"
           << "Unrecognised options are passed through to the external "
              "compiler\n"

     return exitStatus;
   }
   for (const auto &path : fortranSources) {
+    options.predefinitions.clear();
+    if (driver.forcePreprocessing) {
+      if (*driver.forcePreprocessing) {
+        options.predefinitions = predefinitions;
+      }
+    } else {
+      auto dot{path.rfind(".")};
+      if (dot != std::string::npos) {
+        std::string suffix{path.substr(dot + 1)};
+        if (suffix == "F" || suffix == "F90" || suffix == "F95" ||
+            suffix == "CUF" || suffix == "F18") {
+          options.predefinitions = predefinitions;
+        }
+      }
+    }
     std::string relo{CompileFortran(path, options, driver, defaultKinds)};
     if (!driver.compileOnly && !relo.empty()) {
       objlist.push_back(relo);