[flang-commits] [flang] 4308416 - [flang][Driver] Refine _when_ driver diagnostics are formatted

Peixin Qiao via flang-commits flang-commits at lists.llvm.org
Wed Jun 22 08:58:56 PDT 2022


Author: Peixin Qiao
Date: 2022-06-22T23:56:34+08:00
New Revision: 430841605d49f515b6a671ec772135cf67ca9506

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

LOG: [flang][Driver] Refine _when_ driver diagnostics are formatted

This patch refines //when// driver diagnostics are formatted so that
`flang-new` and `flang-new -fc1` behave consistently with `clang` and
`clang -cc1`, respectively. This change only applies to driver diagnostics.
Scanning, parsing and semantic diagnostics are separate and not covered here.

**NEW BEHAVIOUR**
To illustrate the new behaviour, consider the following input file:
```! file.f90
program m
  integer :: i = k
end
```
In the following invocations, "error: Semantic errors in file.f90" _will be_
formatted:
```
$ flang-new file.f90
error: Semantic errors in file.f90
./file.f90:2:18: error: Must be a constant value
    integer :: i = k
$ flang-new -fc1 -fcolor-diagnostics file.f90
error: Semantic errors in file.f90
./file.f90:2:18: error: Must be a constant value
    integer :: i = k
```

However, in the following invocations, "error: Semantic errors in file.f90"
_will not be_ formatted:
```
$ flang-new -fno-color-diagnostics file.f90
error: Semantic errors in file.f90
./file.f90:2:18: error: Must be a constant value
    integer :: i = k
$ flang-new -fc1 file.f90
error: Semantic errors in file.f90
./file.f90:2:18: error: Must be a constant value
    integer :: i = k
```

Before this change, none of the above would be formatted. Note also that the
default behaviour in `flang-new` is different to `flang-new -fc1` (this is
consistent with Clang).

**NOTES ON IMPLEMENTATION**
Note that the diagnostic options are parsed in `createAndPopulateDiagOpt`s in
driver.cpp. That's where the driver's `DiagnosticEngine` options are set. Like
most command-line compiler driver options, these flags are "claimed" in
Flang.cpp (i.e.  when creating a frontend driver invocation) by calling
`getLastArg` rather than in driver.cpp.

In Clang's Options.td, `defm color_diagnostics` is replaced with two separate
definitions: `def fcolor_diagnostics` and def fno_color_diagnostics`. That's
because originally `color_diagnostics` derived from `OptInCC1FFlag`, which is a
multiclass for opt-in options in CC1. In order to preserve the current
behaviour in `clang -cc1` (i.e. to keep `-fno-color-diagnostics` unavailable in
`clang -cc1`) and to implement similar behaviour in `flang-new -fc1`, we can't
re-use `OptInCC1FFlag`.

Formatting is only available in consoles that support it and will normally mean that
the message is printed in bold + color.

Co-authored-by: Andrzej Warzynski <andrzej.warzynski at arm.com>

Reviewed By: rovka

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

Added: 
    flang/test/Driver/color-diagnostics-forwarding.f90
    flang/test/Driver/color-diagnostics.f90

Modified: 
    clang/include/clang/Driver/Options.td
    clang/lib/Driver/ToolChains/Flang.cpp
    flang/include/flang/Frontend/CompilerInvocation.h
    flang/include/flang/Frontend/FrontendOptions.h
    flang/lib/Frontend/CompilerInvocation.cpp
    flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
    flang/test/Driver/driver-help.f90

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 7aea7324cf241..4c0f0ada36547 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1355,8 +1355,11 @@ def fclang_abi_compat_EQ : Joined<["-"], "fclang-abi-compat=">, Group<f_clang_Gr
   Flags<[CC1Option]>, MetaVarName<"<version>">, Values<"<major>.<minor>,latest">,
   HelpText<"Attempt to match the ABI of Clang <version>">;
 def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group<f_Group>;
-defm color_diagnostics : OptInCC1FFlag<"color-diagnostics", "Enable", "Disable", " colors in diagnostics",
-  [CoreOption, FlangOption]>;
+def fcolor_diagnostics : Flag<["-"], "fcolor-diagnostics">, Group<f_Group>,
+  Flags<[CoreOption, CC1Option, FlangOption, FC1Option]>,
+  HelpText<"Enable colors in diagnostics">;
+def fno_color_diagnostics : Flag<["-"], "fno-color-diagnostics">, Group<f_Group>,
+  Flags<[CoreOption, FlangOption]>, HelpText<"Disable colors in diagnostics">;
 def : Flag<["-"], "fdiagnostics-color">, Group<f_Group>, Flags<[CoreOption]>, Alias<fcolor_diagnostics>;
 def : Flag<["-"], "fno-diagnostics-color">, Group<f_Group>, Flags<[CoreOption]>, Alias<fno_color_diagnostics>;
 def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, Group<f_Group>;

diff  --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index ab9287d880a95..aeab9d5fc417f 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -65,6 +65,7 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
   const llvm::Triple &Triple = TC.getEffectiveTriple();
   const std::string &TripleStr = Triple.getTriple();
 
+  const Driver &D = TC.getDriver();
   ArgStringList CmdArgs;
 
   // Invoke ourselves in -fc1 mode.
@@ -108,6 +109,14 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
 
   AddFortranDialectOptions(Args, CmdArgs);
 
+  // Color diagnostics are parsed by the driver directly from argv and later
+  // re-parsed to construct this job; claim any possible color diagnostic here
+  // to avoid warn_drv_unused_argument.
+  Args.getLastArg(options::OPT_fcolor_diagnostics,
+                  options::OPT_fno_color_diagnostics);
+  if (D.getDiags().getDiagnosticOptions().ShowColors)
+    CmdArgs.push_back("-fcolor-diagnostics");
+
   // Add other compile options
   AddOtherOptions(Args, CmdArgs);
 
@@ -139,7 +148,6 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
 
   CmdArgs.push_back(Input.getFilename());
 
-  const auto& D = C.getDriver();
   // TODO: Replace flang-new with flang once the new driver replaces the
   // throwaway driver
   const char *Exec = Args.MakeArgString(D.GetProgramPath("flang-new", TC));

diff  --git a/flang/include/flang/Frontend/CompilerInvocation.h b/flang/include/flang/Frontend/CompilerInvocation.h
index 8686c6e1162b7..55fd5a039b475 100644
--- a/flang/include/flang/Frontend/CompilerInvocation.h
+++ b/flang/include/flang/Frontend/CompilerInvocation.h
@@ -30,8 +30,7 @@ namespace Fortran::frontend {
 /// When errors are encountered, return false and, if Diags is non-null,
 /// report the error(s).
 bool parseDiagnosticArgs(clang::DiagnosticOptions &opts,
-                         llvm::opt::ArgList &args,
-                         bool defaultDiagColor = true);
+                         llvm::opt::ArgList &args);
 
 class CompilerInvocationBase {
 public:

diff  --git a/flang/include/flang/Frontend/FrontendOptions.h b/flang/include/flang/Frontend/FrontendOptions.h
index 3be87b6f5218d..96c4b67fbfe1c 100644
--- a/flang/include/flang/Frontend/FrontendOptions.h
+++ b/flang/include/flang/Frontend/FrontendOptions.h
@@ -219,7 +219,7 @@ class FrontendInputFile {
 struct FrontendOptions {
   FrontendOptions()
       : showHelp(false), showVersion(false), instrumentedParse(false),
-        needProvenanceRangeToCharBlockMappings(false) {}
+        showColors(false), needProvenanceRangeToCharBlockMappings(false) {}
 
   /// Show the -help text.
   unsigned showHelp : 1;
@@ -230,6 +230,9 @@ struct FrontendOptions {
   /// Instrument the parse to get a more verbose log
   unsigned instrumentedParse : 1;
 
+  /// Enable color diagnostics.
+  unsigned showColors : 1;
+
   /// Enable Provenance to character-stream mapping. Allows e.g. IDEs to find
   /// symbols based on source-code location. This is not needed in regular
   /// compilation.

diff  --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 8f2167b14f0e6..f665daf7a9b10 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -53,10 +53,11 @@ CompilerInvocationBase::~CompilerInvocationBase() = default;
 //===----------------------------------------------------------------------===//
 // Deserialization (from args)
 //===----------------------------------------------------------------------===//
-static bool parseShowColorsArgs(
-    const llvm::opt::ArgList &args, bool defaultColor) {
-  // Color diagnostics default to auto ("on" if terminal supports) in the driver
-  // but default to off in cc1, needing an explicit OPT_fdiagnostics_color.
+static bool parseShowColorsArgs(const llvm::opt::ArgList &args,
+                                bool defaultColor = true) {
+  // Color diagnostics default to auto ("on" if terminal supports) in the
+  // compiler driver `flang-new` but default to off in the frontend driver
+  // `flang-new -fc1`, needing an explicit OPT_fdiagnostics_color.
   // Support both clang's -f[no-]color-diagnostics and gcc's
   // -f[no-]diagnostics-colors[=never|always|auto].
   enum {
@@ -88,9 +89,8 @@ static bool parseShowColorsArgs(
 }
 
 bool Fortran::frontend::parseDiagnosticArgs(clang::DiagnosticOptions &opts,
-                                            llvm::opt::ArgList &args,
-                                            bool defaultDiagColor) {
-  opts.ShowColors = parseShowColorsArgs(args, defaultDiagColor);
+                                            llvm::opt::ArgList &args) {
+  opts.ShowColors = parseShowColorsArgs(args);
 
   return true;
 }
@@ -502,6 +502,10 @@ static bool parseDiagArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
     }
   }
 
+  // Default to off for `flang-new -fc1`.
+  res.getFrontendOpts().showColors =
+      parseShowColorsArgs(args, /*defaultDiagColor=*/false);
+
   return diags.getNumErrors() == numErrorsBefore;
 }
 

diff  --git a/flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
index a797fd5998594..f719530a772ed 100644
--- a/flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ b/flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -158,6 +158,9 @@ bool executeCompilerInvocation(CompilerInstance *flang) {
     return false;
   }
 
+  // Honor color diagnostics.
+  flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
+
   // Create and execute the frontend action.
   std::unique_ptr<FrontendAction> act(createFrontendAction(*flang));
   if (!act)

diff  --git a/flang/test/Driver/color-diagnostics-forwarding.f90 b/flang/test/Driver/color-diagnostics-forwarding.f90
new file mode 100644
index 0000000000000..daef17cb75787
--- /dev/null
+++ b/flang/test/Driver/color-diagnostics-forwarding.f90
@@ -0,0 +1,21 @@
+! Test that flang-new forwards -f{no-}color-diagnostics options to
+! flang-new -fc1 as expected.
+
+! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 -fcolor-diagnostics \
+! RUN:   | FileCheck %s --check-prefix=CHECK-CD
+! CHECK-CD: "-fc1"{{.*}} "-fcolor-diagnostics"
+
+! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 -fno-color-diagnostics \
+! RUN:   | FileCheck %s --check-prefix=CHECK-NCD
+! CHECK-NCD-NOT: "-fc1"{{.*}} "-fcolor-diagnostics"
+
+! Check that the last flag wins.
+! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 \
+! RUN:     -fno-color-diagnostics -fcolor-diagnostics \
+! RUN:   | FileCheck %s --check-prefix=CHECK-NCD_CD_S
+! CHECK-NCD_CD_S: "-fc1"{{.*}} "-fcolor-diagnostics"
+
+! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 \
+! RUN:     -fcolor-diagnostics -fno-color-diagnostics \
+! RUN:   | FileCheck %s --check-prefix=CHECK-CD_NCD_S
+! CHECK-CD_NCD_S-NOT: "-fc1"{{.*}} "-fcolor-diagnostics"

diff  --git a/flang/test/Driver/color-diagnostics.f90 b/flang/test/Driver/color-diagnostics.f90
new file mode 100644
index 0000000000000..2d18196d0af73
--- /dev/null
+++ b/flang/test/Driver/color-diagnostics.f90
@@ -0,0 +1,23 @@
+! Test the behaviors of -f{no-}color-diagnostics.
+! Windows command prompt doesn't support ANSI escape sequences.
+! REQUIRES: shell
+
+! RUN: not %flang %s -fcolor-diagnostics 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=CHECK_CD
+! RUN: not %flang %s -fno-color-diagnostics 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=CHECK_NCD
+! RUN: not %flang_fc1 %s -fcolor-diagnostics 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=CHECK_CD
+! RUN: not %flang_fc1 %s -fno-color-diagnostics 2>&1 \
+! RUN:     | FileCheck %s --check-prefix=UNSUPPORTED_OPTION
+! RUN: not %flang_fc1 %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD
+
+! CHECK_CD: {{.*}}[0;1;31merror: {{.*}}[0m{{.*}}[1mSemantic errors in {{.*}}color-diagnostics.f90{{.*}}[0m
+
+! CHECK_NCD: Semantic errors in {{.*}}color-diagnostics.f90
+
+! UNSUPPORTED_OPTION: error: unknown argument: '-fno-color-diagnostics'
+
+program m
+  integer :: i = k
+end

diff  --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90
index 70cf1f514a6fe..fade5abc3b2a5 100644
--- a/flang/test/Driver/driver-help.f90
+++ b/flang/test/Driver/driver-help.f90
@@ -83,6 +83,7 @@
 ! HELP-FC1-NEXT: -falternative-parameter-statement
 ! HELP-FC1-NEXT: Enable the old style PARAMETER statement
 ! HELP-FC1-NEXT: -fbackslash            Specify that backslash in string introduces an escape character
+! HELP-FC1-NEXT: -fcolor-diagnostics     Enable colors in diagnostics
 ! HELP-FC1-NEXT: -fdebug-dump-all       Dump symbols and the parse tree after the semantic checks
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree-no-sema
 ! HELP-FC1-NEXT:                        Dump the parse tree (skips the semantic checks)


        


More information about the flang-commits mailing list