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

Warn about unused preprocessor_deps #2660

Merged
merged 7 commits into from
Sep 25, 2019
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
11 changes: 6 additions & 5 deletions src/dune/cinaps.ml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type t =
; files : Predicate_lang.t
; libraries : Lib_dep.t list
; preprocess : Dune_file.Preprocess_map.t
; preprocessor_deps : Dune_file.Dep_conf.t list
; preprocessor_deps : Loc.t * Dune_file.Dep_conf.t list
; flags : Ocaml_flags.Spec.t
}

Expand All @@ -26,7 +26,10 @@ let decode =
field "preprocess" Dune_file.Preprocess_map.decode
~default:Dune_file.Preprocess_map.default
and+ preprocessor_deps =
field "preprocessor_deps" (repeat Dune_file.Dep_conf.decode) ~default:[]
located
(field "preprocessor_deps"
(repeat Dune_file.Dep_conf.decode)
~default:[])
and+ libraries =
field "libraries"
(Dune_file.Lib_deps.decode ~allow_re_export:false)
Expand Down Expand Up @@ -79,9 +82,7 @@ let gen_rules sctx t ~dir ~scope =
let preprocess =
Preprocessing.make sctx ~dir ~expander ~dep_kind:Required
~lint:Dune_file.Preprocess_map.no_preprocessing ~preprocess:t.preprocess
~preprocessor_deps:
(Super_context.Deps.interpret sctx ~expander t.preprocessor_deps)
~lib_name:None ~scope
~preprocessor_deps:t.preprocessor_deps ~lib_name:None ~scope
in
let modules =
Modules.exe_unwrapped modules
Expand Down
4 changes: 2 additions & 2 deletions src/dune/dune_file.ml
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ module Buildable = struct
; c_names : Ordered_set_lang.t option
; cxx_names : Ordered_set_lang.t option
; preprocess : Preprocess_map.t
; preprocessor_deps : Dep_conf.t list
; preprocessor_deps : Loc.t * Dep_conf.t list
; lint : Preprocess_map.t
; flags : Ocaml_flags.Spec.t
; js_of_ocaml : Js_of_ocaml.t
Expand All @@ -554,7 +554,7 @@ module Buildable = struct
and+ preprocess =
field "preprocess" Preprocess_map.decode ~default:Preprocess_map.default
and+ preprocessor_deps =
field "preprocessor_deps" (repeat Dep_conf.decode) ~default:[]
located (field "preprocessor_deps" (repeat Dep_conf.decode) ~default:[])
and+ lint = field "lint" Lint.decode ~default:Lint.default
and+ c_flags = Dune_env.Stanza.c_flags ~since:since_c
and+ c_names = field_o "c_names" (check_c Ordered_set_lang.decode)
Expand Down
2 changes: 1 addition & 1 deletion src/dune/dune_file.mli
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ module Buildable : sig
; c_names : Ordered_set_lang.t option
; cxx_names : Ordered_set_lang.t option
; preprocess : Preprocess_map.t
; preprocessor_deps : Dep_conf.t list
; preprocessor_deps : Loc.t * Dep_conf.t list
; lint : Lint.t
; flags : Ocaml_flags.Spec.t
; js_of_ocaml : Js_of_ocaml.t
Expand Down
6 changes: 2 additions & 4 deletions src/dune/exe_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ let executables_rules ~sctx ~dir ~expander ~dir_contents ~scope ~compile_info
let modules =
Dir_contents.modules_of_executables dir_contents ~first_exe ~obj_dir
in
let preprocessor_deps =
SC.Deps.interpret sctx exes.buildable.preprocessor_deps ~expander
in
let pp =
Preprocessing.make sctx ~dir ~dep_kind:Required ~scope ~expander
~preprocess:exes.buildable.preprocess ~preprocessor_deps
~preprocess:exes.buildable.preprocess
~preprocessor_deps:exes.buildable.preprocessor_deps
~lint:exes.buildable.lint ~lib_name:None
in
let modules =
Expand Down
4 changes: 1 addition & 3 deletions src/dune/lib_rules.ml
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,7 @@ let cctx (lib : Library.t) ~sctx ~source_modules ~dir ~expander ~scope
let pp =
Preprocessing.make sctx ~dir ~dep_kind ~scope
~preprocess:lib.buildable.preprocess ~expander
~preprocessor_deps:
(Super_context.Deps.interpret sctx ~expander
lib.buildable.preprocessor_deps)
~preprocessor_deps:lib.buildable.preprocessor_deps
~lint:lib.buildable.lint
~lib_name:(Some (snd lib.name))
in
Expand Down
26 changes: 21 additions & 5 deletions src/dune/preprocessing.ml
Original file line number Diff line number Diff line change
Expand Up @@ -566,20 +566,36 @@ type t = (Module.t -> lint:bool -> Module.t) Per_module.t

let dummy = Per_module.for_all (fun m ~lint:_ -> m)

let is_preprocessing t =
Per_module.fold t ~init:true ~f:(fun p acc ->
match p with
| Preprocess.Without_future_syntax.No_preprocessing -> false
| _ -> acc)

let make sctx ~dir ~expander ~dep_kind ~lint ~preprocess ~preprocessor_deps
~lib_name ~scope =
let preprocess =
Per_module.map preprocess ~f:(fun pp ->
Dune_file.Preprocess.remove_future_syntax ~for_:Compiler pp
(Super_context.context sctx).version)
in
let preprocessor_deps =
Build.memoize "preprocessor deps" preprocessor_deps
let loc, deps = preprocessor_deps in
( if not (is_preprocessing preprocess) && not (List.is_empty deps) then
let is_error = Dune_project.dune_version (Scope.project scope) >= (2, 0) in
User_warning.emit ~loc ~is_error
[ Pp.text
"This preprocessor_deps field will be ignored because no \
preprocessor is configured."
] );
SC.Deps.interpret sctx deps ~expander |> Build.memoize "preprocessor deps"
in
let lint_module =
Staged.unstage
(lint_module sctx ~dir ~expander ~dep_kind ~lint ~lib_name ~scope)
in
Per_module.map preprocess ~f:(fun pp ->
match
Dune_file.Preprocess.remove_future_syntax ~for_:Compiler pp
(Super_context.context sctx).version
with
match pp with
| No_preprocessing ->
fun m ~lint ->
let ast = setup_dialect_rules sctx ~dir ~dep_kind ~expander m in
Expand Down
2 changes: 1 addition & 1 deletion src/dune/preprocessing.mli
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ val make :
-> dep_kind:Lib_deps_info.Kind.t
-> lint:Dune_file.Preprocess_map.t
-> preprocess:Dune_file.Preprocess_map.t
-> preprocessor_deps:unit Build.t
-> preprocessor_deps:Loc.t * Dune_file.Dep_conf.t list
-> lib_name:Lib_name.Local.t option
-> scope:Scope.t
-> t
Expand Down
10 changes: 10 additions & 0 deletions test/blackbox-tests/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,14 @@
test-cases/unreadable-src
(progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))

(alias
(name unused-preprocessor-deps)
(deps (package dune) (source_tree test-cases/unused-preprocessor-deps))
(action
(chdir
test-cases/unused-preprocessor-deps
(progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))

(alias
(name upgrader)
(deps (package dune) (source_tree test-cases/upgrader))
Expand Down Expand Up @@ -2003,6 +2011,7 @@
(alias trace-file)
(alias transitive-deps-mode)
(alias unreadable-src)
(alias unused-preprocessor-deps)
(alias upgrader)
(alias use-meta)
(alias variables-for-artifacts)
Expand Down Expand Up @@ -2199,6 +2208,7 @@
(alias trace-file)
(alias transitive-deps-mode)
(alias unreadable-src)
(alias unused-preprocessor-deps)
(alias upgrader)
(alias variables-for-artifacts)
(alias variant-dep-stack)
Expand Down
11 changes: 11 additions & 0 deletions test/blackbox-tests/test-cases/unused-preprocessor-deps/dune
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
(executable
(name a)
(modules a)
;(preprocess (pps ppx_deriving.std))
(preprocessor_deps does-not-exist.txt))

(library
(name b)
(modules b)
(preprocess future_syntax)
(preprocessor_deps does-not-exist.txt))
27 changes: 27 additions & 0 deletions test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
`preprocessor_deps` is provided without `preprocess` and is ignored.
Should warn.

$ touch a.ml b.ml

$ echo "(lang dune 1.11)" > dune-project
$ echo "(allow_approximate_merlin)" >> dune-project
$ dune build
File "dune", line 5, characters 1-39:
5 | (preprocessor_deps does-not-exist.txt))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning: This preprocessor_deps field will be ignored because no preprocessor
is configured.
File "dune", line 11, characters 1-39:
11 | (preprocessor_deps does-not-exist.txt))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning: This preprocessor_deps field will be ignored because no preprocessor
is configured.

$ echo "(lang dune 2.0)" > dune-project
$ dune build
File "dune", line 5, characters 1-39:
5 | (preprocessor_deps does-not-exist.txt))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: This preprocessor_deps field will be ignored because no preprocessor
is configured.
[1]