From 030d3f08e8bd0c0a1196b8769dd4606118b58d17 Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Thu, 19 Sep 2019 15:47:41 +0200 Subject: [PATCH 1/6] Pass preprocessor_deps to Preprocessing.make Signed-off-by: Jules Aguillon --- src/dune/cinaps.ml | 3 +-- src/dune/exe_rules.ml | 6 ++---- src/dune/lib_rules.ml | 4 +--- src/dune/preprocessing.ml | 3 ++- src/dune/preprocessing.mli | 2 +- 5 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/dune/cinaps.ml b/src/dune/cinaps.ml index 2c56acb5dda..ab350fb2b0d 100644 --- a/src/dune/cinaps.ml +++ b/src/dune/cinaps.ml @@ -79,8 +79,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) + ~preprocessor_deps:t.preprocessor_deps ~lib_name:None ~scope in let modules = diff --git a/src/dune/exe_rules.ml b/src/dune/exe_rules.ml index be67c5405b4..fd4d11dbbf2 100644 --- a/src/dune/exe_rules.ml +++ b/src/dune/exe_rules.ml @@ -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 = diff --git a/src/dune/lib_rules.ml b/src/dune/lib_rules.ml index 825f731f24e..860fd6d2eb3 100644 --- a/src/dune/lib_rules.ml +++ b/src/dune/lib_rules.ml @@ -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 diff --git a/src/dune/preprocessing.ml b/src/dune/preprocessing.ml index c5ddb6ea2e3..90d9f270408 100644 --- a/src/dune/preprocessing.ml +++ b/src/dune/preprocessing.ml @@ -569,7 +569,8 @@ let dummy = Per_module.for_all (fun m ~lint:_ -> m) let make sctx ~dir ~expander ~dep_kind ~lint ~preprocess ~preprocessor_deps ~lib_name ~scope = let preprocessor_deps = - Build.memoize "preprocessor deps" preprocessor_deps + SC.Deps.interpret sctx preprocessor_deps ~expander + |> Build.memoize "preprocessor deps" in let lint_module = Staged.unstage diff --git a/src/dune/preprocessing.mli b/src/dune/preprocessing.mli index 845093cc530..4400718d94e 100644 --- a/src/dune/preprocessing.mli +++ b/src/dune/preprocessing.mli @@ -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:Dune_file.Dep_conf.t list -> lib_name:Lib_name.Local.t option -> scope:Scope.t -> t From 577eb0bb77a163072c996118dabe91bd49e845e4 Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Thu, 19 Sep 2019 16:42:59 +0200 Subject: [PATCH 2/6] Add unused-preprocessor-deps test Signed-off-by: Jules Aguillon --- test/blackbox-tests/dune.inc | 10 ++++++++++ .../test-cases/unused-preprocessor-deps/dune | 4 ++++ .../test-cases/unused-preprocessor-deps/dune-project | 1 + .../test-cases/unused-preprocessor-deps/e.ml | 0 .../test-cases/unused-preprocessor-deps/run.t | 4 ++++ 5 files changed, 19 insertions(+) create mode 100644 test/blackbox-tests/test-cases/unused-preprocessor-deps/dune create mode 100644 test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project create mode 100644 test/blackbox-tests/test-cases/unused-preprocessor-deps/e.ml create mode 100644 test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index 66d964460c6..4767f9b9412 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -1590,6 +1590,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)) @@ -1985,6 +1993,7 @@ (alias trace-file) (alias transitive-deps-mode) (alias unreadable-src) + (alias unused-preprocessor-deps) (alias upgrader) (alias use-meta) (alias variables-for-artifacts) @@ -2179,6 +2188,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) diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune new file mode 100644 index 00000000000..e391c0e2f0e --- /dev/null +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune @@ -0,0 +1,4 @@ +(executable + (name e) + ;(preprocess (pps ppx_deriving.std)) + (preprocessor_deps does-not-exist.txt)) diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project new file mode 100644 index 00000000000..929c696e561 --- /dev/null +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project @@ -0,0 +1 @@ +(lang dune 2.0) diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/e.ml b/test/blackbox-tests/test-cases/unused-preprocessor-deps/e.ml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t new file mode 100644 index 00000000000..5433c3542f3 --- /dev/null +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t @@ -0,0 +1,4 @@ +`preprocessor_deps` is provided without `preprocess` and is ignored. +Should warn. + + $ dune build From 06c6553fdf385d8c441b46a1fe9a2bb3704472c7 Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Thu, 19 Sep 2019 18:14:28 +0200 Subject: [PATCH 3/6] Trigger a warning when preprocessor_deps would be ignored Signed-off-by: Jules Aguillon --- src/dune/cinaps.ml | 10 ++++--- src/dune/dune_file.ml | 4 +-- src/dune/dune_file.mli | 2 +- src/dune/preprocessing.ml | 26 ++++++++++++++----- src/dune/preprocessing.mli | 2 +- .../test-cases/unused-preprocessor-deps/run.t | 5 ++++ 6 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/dune/cinaps.ml b/src/dune/cinaps.ml index ab350fb2b0d..1a8a1df5c7e 100644 --- a/src/dune/cinaps.ml +++ b/src/dune/cinaps.ml @@ -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 } @@ -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) @@ -79,8 +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:t.preprocessor_deps - ~lib_name:None ~scope + ~preprocessor_deps:t.preprocessor_deps ~lib_name:None ~scope in let modules = Modules.exe_unwrapped modules diff --git a/src/dune/dune_file.ml b/src/dune/dune_file.ml index e790c4c8d1c..425d26bc20e 100644 --- a/src/dune/dune_file.ml +++ b/src/dune/dune_file.ml @@ -562,7 +562,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 @@ -579,7 +579,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) diff --git a/src/dune/dune_file.mli b/src/dune/dune_file.mli index 5449ed7f3a5..af18129c8e9 100644 --- a/src/dune/dune_file.mli +++ b/src/dune/dune_file.mli @@ -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 diff --git a/src/dune/preprocessing.ml b/src/dune/preprocessing.ml index 90d9f270408..740b3226a56 100644 --- a/src/dune/preprocessing.ml +++ b/src/dune/preprocessing.ml @@ -566,21 +566,35 @@ 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 = - SC.Deps.interpret sctx preprocessor_deps ~expander - |> Build.memoize "preprocessor deps" + let loc, deps = preprocessor_deps in + if (not (is_preprocessing preprocess)) && not (List.is_empty deps) then + User_warning.emit ~loc + [ 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 diff --git a/src/dune/preprocessing.mli b/src/dune/preprocessing.mli index 4400718d94e..3aee81bb281 100644 --- a/src/dune/preprocessing.mli +++ b/src/dune/preprocessing.mli @@ -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:Dune_file.Dep_conf.t list + -> preprocessor_deps:Loc.t * Dune_file.Dep_conf.t list -> lib_name:Lib_name.Local.t option -> scope:Scope.t -> t diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t index 5433c3542f3..d5f3ae609f6 100644 --- a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t @@ -2,3 +2,8 @@ Should warn. $ dune build + File "dune", line 4, characters 1-39: + 4 | (preprocessor_deps does-not-exist.txt)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Warning: This preprocessor_deps field will be ignored because no preprocessor + is configured. From baa7aa6d9aa14fdfc6f18bb992cee09725176d3c Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Thu, 19 Sep 2019 18:19:25 +0200 Subject: [PATCH 4/6] Add a test with future_syntax Signed-off-by: Jules Aguillon --- .../test-cases/unused-preprocessor-deps/dune | 9 ++++++++- .../test-cases/unused-preprocessor-deps/dune-project | 1 + .../test-cases/unused-preprocessor-deps/e.ml | 0 .../test-cases/unused-preprocessor-deps/run.t | 10 ++++++++-- 4 files changed, 17 insertions(+), 3 deletions(-) delete mode 100644 test/blackbox-tests/test-cases/unused-preprocessor-deps/e.ml diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune index e391c0e2f0e..9a5f8e74b38 100644 --- a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune @@ -1,4 +1,11 @@ (executable - (name e) + (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)) diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project index 929c696e561..d38488fe5eb 100644 --- a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project @@ -1 +1,2 @@ (lang dune 2.0) +(allow_approximate_merlin) diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/e.ml b/test/blackbox-tests/test-cases/unused-preprocessor-deps/e.ml deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t index d5f3ae609f6..bc014dbad27 100644 --- a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t @@ -1,9 +1,15 @@ `preprocessor_deps` is provided without `preprocess` and is ignored. Should warn. + $ touch a.ml b.ml $ dune build - File "dune", line 4, characters 1-39: - 4 | (preprocessor_deps does-not-exist.txt)) + 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. From 1595ddf1c97a3ba51e7bde3886c131569a7cb7e0 Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Fri, 20 Sep 2019 11:14:48 +0200 Subject: [PATCH 5/6] Make an error with dune_lang >= 2.0 Signed-off-by: Jules Aguillon --- src/dune/preprocessing.ml | 7 ++++--- .../test-cases/unused-preprocessor-deps/dune-project | 2 -- .../test-cases/unused-preprocessor-deps/run.t | 12 ++++++++++++ 3 files changed, 16 insertions(+), 5 deletions(-) delete mode 100644 test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project diff --git a/src/dune/preprocessing.ml b/src/dune/preprocessing.ml index 740b3226a56..d9addc8fa15 100644 --- a/src/dune/preprocessing.ml +++ b/src/dune/preprocessing.ml @@ -581,12 +581,13 @@ let make sctx ~dir ~expander ~dep_kind ~lint ~preprocess ~preprocessor_deps in let preprocessor_deps = let loc, deps = preprocessor_deps in - if (not (is_preprocessing preprocess)) && not (List.is_empty deps) then - User_warning.emit ~loc + ( 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 = diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project b/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project deleted file mode 100644 index d38488fe5eb..00000000000 --- a/test/blackbox-tests/test-cases/unused-preprocessor-deps/dune-project +++ /dev/null @@ -1,2 +0,0 @@ -(lang dune 2.0) -(allow_approximate_merlin) diff --git a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t index bc014dbad27..9272b0feee9 100644 --- a/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t +++ b/test/blackbox-tests/test-cases/unused-preprocessor-deps/run.t @@ -2,6 +2,9 @@ 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)) @@ -13,3 +16,12 @@ Should warn. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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] From 9dcb20087b708ac2bb6c888357002ac8f692519a Mon Sep 17 00:00:00 2001 From: Jules Aguillon Date: Wed, 25 Sep 2019 11:40:17 +0200 Subject: [PATCH 6/6] Unecessary parens Signed-off-by: Jules Aguillon --- src/dune/preprocessing.ml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dune/preprocessing.ml b/src/dune/preprocessing.ml index d9addc8fa15..ea5d9fff1b3 100644 --- a/src/dune/preprocessing.ml +++ b/src/dune/preprocessing.ml @@ -581,7 +581,7 @@ let make sctx ~dir ~expander ~dep_kind ~lint ~preprocess ~preprocessor_deps in let preprocessor_deps = let loc, deps = preprocessor_deps in - ( if (not (is_preprocessing preprocess)) && not (List.is_empty deps) then + ( 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