[clang] 4308416 - [flang][Driver] Refine _when_ driver diagnostics are formatted
Peixin Qiao via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 22 08:58:58 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 cfe-commits
mailing list