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.warzyn...@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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
