Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Colored messages from C/C++ compiler in C stubs #4083

Merged
merged 2 commits into from
Dec 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
Unreleased
----------

- Use colored output with GCC and Clang when compiling C stubs. The
flag `-fdiagnostics-color=always` is added to the `:standard` set of
flags. (#4083, @MisterDA)

- Fix the parsing of decimal and hexadecimal escape literals in `dune`,
`dune-package`, and other dune s-expression based files (#6710, @shym)

Expand Down
12 changes: 11 additions & 1 deletion src/dune_rules/cxx_flags.ml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ let base_cxx_flags ~for_ cc =
| Msvc, Link -> []
| Other _, (Link | Compile) -> []

let fdiagnostics_color = function
| (Gcc | Clang) when Lazy.force Ansi_color.stderr_supports_color ->
[ "-fdiagnostics-color=always" ]
| _ -> []

let preprocessed_filename = "ccomp"

let ccomp_type build_dir =
Expand All @@ -46,8 +51,13 @@ let check_warn = function
]
| _ -> ()

let get_flags ~for_ ctx =
let ccomp_type ctx =
let open Action_builder.O in
let+ ccomp_type = ccomp_type ctx.Context.build_dir in
check_warn ccomp_type;
ccomp_type

let get_flags ~for_ ctx =
let open Action_builder.O in
let+ ccomp_type = ccomp_type ctx in
base_cxx_flags ~for_ ccomp_type
16 changes: 15 additions & 1 deletion src/dune_rules/cxx_flags.mli
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,24 @@ type phase =
| Compile
| Link

(** The detected compiler *)
type ccomp_type =
| Gcc
| Msvc
| Clang
| Other of string

(** The name of the file created in the .dune folder after calling the C
preprocessor *)
val preprocessed_filename : string

(** [get_flags c_compiler] returns the necessary flags to turn this compiler
(** [ccomp_type ctx] returns the C/C++ compiler type. *)
val ccomp_type : Context.t -> ccomp_type Action_builder.t

(** [get_flags for_:phase ctx] returns the necessary flags to turn this compiler
into a c++ compiler for some of the most common compilers *)
val get_flags : for_:phase -> Context.t -> string list Action_builder.t

(** [fdiagnostics_color cc] returns the flags activating color diagnostics for
the C/C++ compiler, if supported. *)
val fdiagnostics_color : ccomp_type -> string list
21 changes: 14 additions & 7 deletions src/dune_rules/super_context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,23 @@ let default_context_flags (ctx : Context.t) ~project =
in
let c, cxx =
match Dune_project.use_standard_c_and_cxx_flags project with
| None | Some false ->
(Action_builder.return cflags, Action_builder.return cxxflags)
| None | Some false -> Action_builder.(return cflags, return cxxflags)
| Some true ->
let c = cflags @ Ocaml_config.ocamlc_cppflags ctx.ocaml_config in
let open Action_builder.O in
let c =
let+ cc = Cxx_flags.ccomp_type ctx in
let fdiagnostics_color = Cxx_flags.fdiagnostics_color cc in
cflags
@ Ocaml_config.ocamlc_cppflags ctx.ocaml_config
@ fdiagnostics_color
in
let cxx =
let open Action_builder.O in
let+ db_flags = Cxx_flags.get_flags ~for_:Compile ctx in
db_flags @ cxxflags
let+ cc = Cxx_flags.ccomp_type ctx
and+ db_flags = Cxx_flags.get_flags ~for_:Compile ctx in
let fdiagnostics_color = Cxx_flags.fdiagnostics_color cc in
db_flags @ cxxflags @ fdiagnostics_color
in
(Action_builder.return c, cxx)
(c, cxx)
in
Foreign_language.Dict.make ~c ~cxx

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(lang dune 3.5)
53 changes: 53 additions & 0 deletions test/blackbox-tests/test-cases/c-flags-diagnostics-color.t/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
This test won't work with MSVC as it's using GCC and clang CLI
parameters.

We have to force Dune outputting colors, for that we use
`CLICOLOR_FORCE=1`.

The flag should be present in the command, as we want it in the
:standard set.

We don't actually test that the compiler outputs colors, just that
Dune correctly passes the flag when required, and doesn't when it's
not.

test, color enabled, color flag default (enabled)
=================================================

$ cat >dune <<EOF
> (library
> (name test)
> (foreign_stubs (language c) (names stub)))
> EOF

$ CLICOLOR_FORCE=1 dune rules -m stub.o | grep -ce "-fdiagnostics-color=always"
1

test, color enabled, color flag disabled
========================================

$ cat >dune <<EOF
> (library
> (name test)
> (foreign_stubs
> (flags :standard \ -fdiagnostics-color=always)
> (language c) (names stub)))
> EOF

$ CLICOLOR_FORCE=1 dune rules -m stub.o | grep -ce "-fdiagnostics-color=always"
0
[1]

test, color disabled, color flag default (enabled)
==================================================

$ cat >dune <<EOF
> (library
> (name test)
> (foreign_stubs
> (language c) (names stub)))
> EOF

$ CLICOLOR=0 dune rules -m stub.o | grep -ce "-fdiagnostics-color=always"
0
[1]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#error "error message"