Skip to content

Commit 4308416

Browse files
committed
[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 <[email protected]> Reviewed By: rovka Differential Revision: https://reviews.llvm.org/D126164
1 parent f31ec68 commit 4308416

File tree

9 files changed

+78
-13
lines changed

9 files changed

+78
-13
lines changed

clang/include/clang/Driver/Options.td

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,8 +1355,11 @@ def fclang_abi_compat_EQ : Joined<["-"], "fclang-abi-compat=">, Group<f_clang_Gr
13551355
Flags<[CC1Option]>, MetaVarName<"<version>">, Values<"<major>.<minor>,latest">,
13561356
HelpText<"Attempt to match the ABI of Clang <version>">;
13571357
def fclasspath_EQ : Joined<["-"], "fclasspath=">, Group<f_Group>;
1358-
defm color_diagnostics : OptInCC1FFlag<"color-diagnostics", "Enable", "Disable", " colors in diagnostics",
1359-
[CoreOption, FlangOption]>;
1358+
def fcolor_diagnostics : Flag<["-"], "fcolor-diagnostics">, Group<f_Group>,
1359+
Flags<[CoreOption, CC1Option, FlangOption, FC1Option]>,
1360+
HelpText<"Enable colors in diagnostics">;
1361+
def fno_color_diagnostics : Flag<["-"], "fno-color-diagnostics">, Group<f_Group>,
1362+
Flags<[CoreOption, FlangOption]>, HelpText<"Disable colors in diagnostics">;
13601363
def : Flag<["-"], "fdiagnostics-color">, Group<f_Group>, Flags<[CoreOption]>, Alias<fcolor_diagnostics>;
13611364
def : Flag<["-"], "fno-diagnostics-color">, Group<f_Group>, Flags<[CoreOption]>, Alias<fno_color_diagnostics>;
13621365
def fdiagnostics_color_EQ : Joined<["-"], "fdiagnostics-color=">, Group<f_Group>;

clang/lib/Driver/ToolChains/Flang.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
6565
const llvm::Triple &Triple = TC.getEffectiveTriple();
6666
const std::string &TripleStr = Triple.getTriple();
6767

68+
const Driver &D = TC.getDriver();
6869
ArgStringList CmdArgs;
6970

7071
// Invoke ourselves in -fc1 mode.
@@ -108,6 +109,14 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
108109

109110
AddFortranDialectOptions(Args, CmdArgs);
110111

112+
// Color diagnostics are parsed by the driver directly from argv and later
113+
// re-parsed to construct this job; claim any possible color diagnostic here
114+
// to avoid warn_drv_unused_argument.
115+
Args.getLastArg(options::OPT_fcolor_diagnostics,
116+
options::OPT_fno_color_diagnostics);
117+
if (D.getDiags().getDiagnosticOptions().ShowColors)
118+
CmdArgs.push_back("-fcolor-diagnostics");
119+
111120
// Add other compile options
112121
AddOtherOptions(Args, CmdArgs);
113122

@@ -139,7 +148,6 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
139148

140149
CmdArgs.push_back(Input.getFilename());
141150

142-
const auto& D = C.getDriver();
143151
// TODO: Replace flang-new with flang once the new driver replaces the
144152
// throwaway driver
145153
const char *Exec = Args.MakeArgString(D.GetProgramPath("flang-new", TC));

flang/include/flang/Frontend/CompilerInvocation.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ namespace Fortran::frontend {
3030
/// When errors are encountered, return false and, if Diags is non-null,
3131
/// report the error(s).
3232
bool parseDiagnosticArgs(clang::DiagnosticOptions &opts,
33-
llvm::opt::ArgList &args,
34-
bool defaultDiagColor = true);
33+
llvm::opt::ArgList &args);
3534

3635
class CompilerInvocationBase {
3736
public:

flang/include/flang/Frontend/FrontendOptions.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ class FrontendInputFile {
219219
struct FrontendOptions {
220220
FrontendOptions()
221221
: showHelp(false), showVersion(false), instrumentedParse(false),
222-
needProvenanceRangeToCharBlockMappings(false) {}
222+
showColors(false), needProvenanceRangeToCharBlockMappings(false) {}
223223

224224
/// Show the -help text.
225225
unsigned showHelp : 1;
@@ -230,6 +230,9 @@ struct FrontendOptions {
230230
/// Instrument the parse to get a more verbose log
231231
unsigned instrumentedParse : 1;
232232

233+
/// Enable color diagnostics.
234+
unsigned showColors : 1;
235+
233236
/// Enable Provenance to character-stream mapping. Allows e.g. IDEs to find
234237
/// symbols based on source-code location. This is not needed in regular
235238
/// compilation.

flang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@ CompilerInvocationBase::~CompilerInvocationBase() = default;
5353
//===----------------------------------------------------------------------===//
5454
// Deserialization (from args)
5555
//===----------------------------------------------------------------------===//
56-
static bool parseShowColorsArgs(
57-
const llvm::opt::ArgList &args, bool defaultColor) {
58-
// Color diagnostics default to auto ("on" if terminal supports) in the driver
59-
// but default to off in cc1, needing an explicit OPT_fdiagnostics_color.
56+
static bool parseShowColorsArgs(const llvm::opt::ArgList &args,
57+
bool defaultColor = true) {
58+
// Color diagnostics default to auto ("on" if terminal supports) in the
59+
// compiler driver `flang-new` but default to off in the frontend driver
60+
// `flang-new -fc1`, needing an explicit OPT_fdiagnostics_color.
6061
// Support both clang's -f[no-]color-diagnostics and gcc's
6162
// -f[no-]diagnostics-colors[=never|always|auto].
6263
enum {
@@ -88,9 +89,8 @@ static bool parseShowColorsArgs(
8889
}
8990

9091
bool Fortran::frontend::parseDiagnosticArgs(clang::DiagnosticOptions &opts,
91-
llvm::opt::ArgList &args,
92-
bool defaultDiagColor) {
93-
opts.ShowColors = parseShowColorsArgs(args, defaultDiagColor);
92+
llvm::opt::ArgList &args) {
93+
opts.ShowColors = parseShowColorsArgs(args);
9494

9595
return true;
9696
}
@@ -502,6 +502,10 @@ static bool parseDiagArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
502502
}
503503
}
504504

505+
// Default to off for `flang-new -fc1`.
506+
res.getFrontendOpts().showColors =
507+
parseShowColorsArgs(args, /*defaultDiagColor=*/false);
508+
505509
return diags.getNumErrors() == numErrorsBefore;
506510
}
507511

flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,9 @@ bool executeCompilerInvocation(CompilerInstance *flang) {
158158
return false;
159159
}
160160

161+
// Honor color diagnostics.
162+
flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
163+
161164
// Create and execute the frontend action.
162165
std::unique_ptr<FrontendAction> act(createFrontendAction(*flang));
163166
if (!act)
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
! Test that flang-new forwards -f{no-}color-diagnostics options to
2+
! flang-new -fc1 as expected.
3+
4+
! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 -fcolor-diagnostics \
5+
! RUN: | FileCheck %s --check-prefix=CHECK-CD
6+
! CHECK-CD: "-fc1"{{.*}} "-fcolor-diagnostics"
7+
8+
! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 -fno-color-diagnostics \
9+
! RUN: | FileCheck %s --check-prefix=CHECK-NCD
10+
! CHECK-NCD-NOT: "-fc1"{{.*}} "-fcolor-diagnostics"
11+
12+
! Check that the last flag wins.
13+
! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 \
14+
! RUN: -fno-color-diagnostics -fcolor-diagnostics \
15+
! RUN: | FileCheck %s --check-prefix=CHECK-NCD_CD_S
16+
! CHECK-NCD_CD_S: "-fc1"{{.*}} "-fcolor-diagnostics"
17+
18+
! RUN: %flang -fsyntax-only -### %s -o %t 2>&1 \
19+
! RUN: -fcolor-diagnostics -fno-color-diagnostics \
20+
! RUN: | FileCheck %s --check-prefix=CHECK-CD_NCD_S
21+
! CHECK-CD_NCD_S-NOT: "-fc1"{{.*}} "-fcolor-diagnostics"
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
! Test the behaviors of -f{no-}color-diagnostics.
2+
! Windows command prompt doesn't support ANSI escape sequences.
3+
! REQUIRES: shell
4+
5+
! RUN: not %flang %s -fcolor-diagnostics 2>&1 \
6+
! RUN: | FileCheck %s --check-prefix=CHECK_CD
7+
! RUN: not %flang %s -fno-color-diagnostics 2>&1 \
8+
! RUN: | FileCheck %s --check-prefix=CHECK_NCD
9+
! RUN: not %flang_fc1 %s -fcolor-diagnostics 2>&1 \
10+
! RUN: | FileCheck %s --check-prefix=CHECK_CD
11+
! RUN: not %flang_fc1 %s -fno-color-diagnostics 2>&1 \
12+
! RUN: | FileCheck %s --check-prefix=UNSUPPORTED_OPTION
13+
! RUN: not %flang_fc1 %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD
14+
15+
! CHECK_CD: {{.*}}[0;1;31merror: {{.*}}[0m{{.*}}[1mSemantic errors in {{.*}}color-diagnostics.f90{{.*}}[0m
16+
17+
! CHECK_NCD: Semantic errors in {{.*}}color-diagnostics.f90
18+
19+
! UNSUPPORTED_OPTION: error: unknown argument: '-fno-color-diagnostics'
20+
21+
program m
22+
integer :: i = k
23+
end

flang/test/Driver/driver-help.f90

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
! HELP-FC1-NEXT: -falternative-parameter-statement
8484
! HELP-FC1-NEXT: Enable the old style PARAMETER statement
8585
! HELP-FC1-NEXT: -fbackslash Specify that backslash in string introduces an escape character
86+
! HELP-FC1-NEXT: -fcolor-diagnostics Enable colors in diagnostics
8687
! HELP-FC1-NEXT: -fdebug-dump-all Dump symbols and the parse tree after the semantic checks
8788
! HELP-FC1-NEXT: -fdebug-dump-parse-tree-no-sema
8889
! HELP-FC1-NEXT: Dump the parse tree (skips the semantic checks)

0 commit comments

Comments
 (0)