[flang-commits] [PATCH] D106727: [flang] Produce proper "preprocessor output" for -E option

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/Frontend/CompilerInvocation.cpp:378-379
+  } else if (args.hasArg(clang::driver::options::OPT_P)) {
+    // If both -P and -fdebug-dump-cooked-chars appear, -P is
+    // unclaimed and will elicit a warning
+    opts.set_noLineDirectives(true);
----------------
I would replace this comment with a diagnostic, e.g. :
```lang=c++
const unsigned diagID = diags.getCustomDiagID(clang::DiagnosticsEngine::Warning, "'-P' will be ignored ('-P' and '-fno-reformat' are incompatible)");
diags.Report(diagID);
}
```
This way, the driver becomes a bit more friendly towards end-users. You will have to update this method a tiny bit. Here's a patch with a test (it's a small change):

```lang=bash
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 04905f103daf..3d38d6dba3ff 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -342,8 +342,10 @@ static std::string getIntrinsicDir() {
 ///
 /// \param [in] opts The preprocessor options instance
 /// \param [out] args The list of input arguments
-static void parsePreprocessorArgs(
-    Fortran::frontend::PreprocessorOptions &opts, llvm::opt::ArgList &args) {
+static bool parsePreprocessorArgs(Fortran::frontend::PreprocessorOptions &opts,
+    llvm::opt::ArgList &args, clang::DiagnosticsEngine &diags) {
+  unsigned numErrorsBefore = diags.getNumErrors();
+
   // Add macros from the command line.
   for (const auto *currentArg : args.filtered(
            clang::driver::options::OPT_D, clang::driver::options::OPT_U)) {
@@ -372,13 +374,24 @@ static void parsePreprocessorArgs(
         ? PPMacrosFlag::Include
         : PPMacrosFlag::Exclude;

+
   if (args.hasArg(clang::driver::options::OPT_fdebug_dump_cooked_chars)) {
     opts.set_dumpCookedChars(true);
-  } else if (args.hasArg(clang::driver::options::OPT_P)) {
-    // If both -P and -fdebug-dump-cooked-chars appear, -P is
-    // unclaimed and will elicit a warning
+  }
+
+  if (args.hasArg(clang::driver::options::OPT_P)) {
     opts.set_noLineDirectives(true);
   }
+
+  if (opts.noLineDirectives() && opts.dumpCookedChars()) {
+    const unsigned diagID =
+        diags.getCustomDiagID(clang::DiagnosticsEngine::Warning,
+            "'-P' will be ignored ('-P' and '-fno-reformat' are incompatible)");
+    diags.Report(diagID);
+    opts.set_noLineDirectives(false);
+  }
+
+  return diags.getNumErrors() == numErrorsBefore;
 }

 /// Parses all semantic related arguments and populates the variables
@@ -541,7 +554,7 @@ bool CompilerInvocation::CreateFromArgs(CompilerInvocation &res,
   }

   success &= ParseFrontendArgs(res.frontendOpts(), args, diags);
-  parsePreprocessorArgs(res.preprocessorOpts(), args);
+  success &= parsePreprocessorArgs(res.preprocessorOpts(), args, diags);
   success &= parseSemaArgs(res, args, diags);
   success &= parseDialectArgs(res, args, diags);
   success &= parseDiagArgs(res, args, diags);
diff --git a/flang/test/Driver/pp-flags-warning.f90 b/flang/test/Driver/pp-flags-warning.f90
new file mode 100644
index 000000000000..5fbf3a1574a5
--- /dev/null
+++ b/flang/test/Driver/pp-flags-warning.f90
@@ -0,0 +1,8 @@
+! REQUIRES: new-flang-driver
+
+! RUN: %flang_fc1 -E -P -fdebug-dump-cooked-chars %s 2>&1 | FileCheck %s
+
+! CHECK: warning: '-P' will be ignored ('-P' and '-fno-reformat' are incompatible)
+
+program hello
+end
```
Feel free to use this (with or without edits).


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

https://reviews.llvm.org/D106727