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.

Sign up for GitHub

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

Opaque Mode #1079

Merged
merged 7 commits into from
Aug 2, 2018
Merged

Opaque Mode #1079

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8a03e70
Hard code opaque mode
rgrinberg Jul 27, 2018
b394896
Make opaque information available to compilation context
rgrinberg Jul 31, 2018
e09bbfc
Fix opaque calculation for includes
rgrinberg Aug 2, 2018
9f43a88
Remove opaque from Lib.t
rgrinberg Aug 2, 2018
bde4943
Make file_deps and file_deps_with_exts share code
rgrinberg Aug 2, 2018
1597b61
Move 4.03 minium version check for opaque to top level
rgrinberg Aug 2, 2018
4cbd698
Update CHANGELOG for -opaque by in dev
rgrinberg Aug 2, 2018
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
32 changes: 23 additions & 9 deletions src/compilation_context.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module SC = Super_context
module Includes = struct
type t = string list Arg_spec.t Cm_kind.Dict.t

let make sctx ~requires : _ Cm_kind.Dict.t =
let make sctx ~opaque ~requires : _ Cm_kind.Dict.t =
match requires with
| Error exn -> Cm_kind.Dict.make_all (Arg_spec.Dyn (fun _ -> raise exn))
| Ok libs ->
Expand All @@ -18,15 +18,25 @@ module Includes = struct
(SC.Libs.file_deps sctx libs ~ext:".cmi")
]
in
let cmi_and_cmx_includes =
Arg_spec.S [ iflags
; Hidden_deps
(SC.Libs.file_deps sctx libs ~ext:".cmi-and-.cmx")
]
let cmx_includes =
Arg_spec.S
[ iflags
; Hidden_deps
( if opaque then
List.map libs ~f:(fun lib ->
(lib, if Lib.is_local lib then
".cmi"
else
".cmi-and-.cmx"))
|> SC.Libs.file_deps_with_exts sctx
else
SC.Libs.file_deps sctx libs ~ext:".cmi-and-.cmx"
)
]
in
{ cmi = cmi_includes
; cmo = cmi_includes
; cmx = cmi_and_cmx_includes
; cmx = cmx_includes
}

let empty =
Expand All @@ -47,6 +57,7 @@ type t =
; includes : Includes.t
; preprocessing : Preprocessing.t
; no_keep_locs : bool
; opaque : bool
}

let super_context t = t.super_context
Expand All @@ -62,12 +73,14 @@ let requires t = t.requires
let includes t = t.includes
let preprocessing t = t.preprocessing
let no_keep_locs t = t.no_keep_locs
let opaque t = t.opaque

let context t = Super_context.context t.super_context

let create ~super_context ~scope ~dir ?(dir_kind=File_tree.Dune_file.Kind.Dune)
?(obj_dir=dir) ~modules ?alias_module ?lib_interface_module ~flags
~requires ?(preprocessing=Preprocessing.dummy) ?(no_keep_locs=false) () =
~requires ?(preprocessing=Preprocessing.dummy) ?(no_keep_locs=false)
~opaque () =
{ super_context
; scope
; dir
Expand All @@ -78,9 +91,10 @@ let create ~super_context ~scope ~dir ?(dir_kind=File_tree.Dune_file.Kind.Dune)
; lib_interface_module
; flags
; requires
; includes = Includes.make super_context ~requires
; includes = Includes.make super_context ~opaque ~requires
; preprocessing
; no_keep_locs
; opaque
}

let for_alias_module t =
Expand Down
2 changes: 2 additions & 0 deletions src/compilation_context.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ val create
-> requires : Lib.t list Or_exn.t
-> ?preprocessing : Preprocessing.t
-> ?no_keep_locs : bool
-> opaque : bool
-> unit
-> t

Expand All @@ -45,3 +46,4 @@ val requires : t -> Lib.t list Or_exn.t
val includes : t -> string list Arg_spec.t Cm_kind.Dict.t
val preprocessing : t -> Preprocessing.t
val no_keep_locs : t -> bool
val opaque : t -> bool
4 changes: 4 additions & 0 deletions src/gen_rules.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ module Gen(P : Install_rules.Params) = struct
let sctx = P.sctx
let ctx = SC.context sctx

let opaque = ctx.profile = "dev" && ctx.version >= (4, 03, 0)

(* +-----------------------------------------------------------------+
| Library stuff |
+-----------------------------------------------------------------+ *)
Expand Down Expand Up @@ -199,6 +201,7 @@ module Gen(P : Install_rules.Params) = struct
~requires
~preprocessing:pp
~no_keep_locs:lib.no_keep_locs
~opaque
in

let dep_graphs = Ocamldep.rules cctx in
Expand Down Expand Up @@ -520,6 +523,7 @@ module Gen(P : Install_rules.Params) = struct
~flags
~requires
~preprocessing:pp
~opaque
in

Exe.build_and_link_many cctx
Expand Down
1 change: 1 addition & 0 deletions src/inline_tests.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ include Sub_system.Register_end_point(
~scope
~dir:inline_test_dir
~modules
~opaque:false
~requires:runner_libs
~flags:(Ocaml_flags.of_list ["-w"; "-24"]);
in
Expand Down
9 changes: 5 additions & 4 deletions src/module_compilation.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ let build_cm cctx ?sandbox ?(dynlink=true) ~dep_graphs ~cm_kind (m : Module.t) =
| Cmi | Cmo -> other_targets
in
let dep_graph = Ml_kind.Dict.get dep_graphs ml_kind in
let opaque = CC.opaque cctx in
let other_cm_files =
Build.dyn_paths
(Ocamldep.Dep_graph.deps_of dep_graph m >>^ fun deps ->
List.concat_map deps
~f:(fun m ->
let deps = [Module.cm_file_unsafe m ~obj_dir Cmi] in
if Module.has_impl m && cm_kind = Cmx then
if Module.has_impl m && cm_kind = Cmx && not opaque then
Module.cm_file_unsafe m ~obj_dir Cmx :: deps
else
deps))
Expand All @@ -86,8 +87,8 @@ let build_cm cctx ?sandbox ?(dynlink=true) ~dep_graphs ~cm_kind (m : Module.t) =
let in_dir = Target.file dir target in
SC.add_rule sctx (Build.symlink ~src:in_obj_dir ~dst:in_dir))
end;
let opaque =
if cm_kind = Cmi && not (Module.has_impl m) && ctx.version >= (4, 03, 0) then
let opaque_arg =
if opaque && cm_kind = Cmi then
Arg_spec.A "-opaque"
else
As []
Expand Down Expand Up @@ -122,7 +123,7 @@ let build_cm cctx ?sandbox ?(dynlink=true) ~dep_graphs ~cm_kind (m : Module.t) =
; Cm_kind.Dict.get (CC.includes cctx) cm_kind
; As extra_args
; if dynlink || cm_kind <> Cmx then As [] else A "-nodynlink"
; A "-no-alias-deps"; opaque
; A "-no-alias-deps"; opaque_arg
; (match CC.alias_module cctx with
| None -> S []
| Some (m : Module.t) ->
Expand Down
19 changes: 12 additions & 7 deletions src/super_context.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,14 +706,19 @@ module Libs = struct
(lib_files_alias ~dir ~name:(Library.best_name lib) ~ext))
|> Path.Set.of_list)

let file_deps_of_lib t (lib : Lib.t) ~ext =
if Lib.is_local lib then
Alias.stamp_file
(lib_files_alias ~dir:(Lib.src_dir lib) ~name:(Lib.name lib) ~ext)
else
Build_system.stamp_file_for_files_of t.build_system
~dir:(Lib.obj_dir lib) ~ext

let file_deps_with_exts t lib_exts =
List.rev_map lib_exts ~f:(fun (lib, ext) -> file_deps_of_lib t lib ~ext)

let file_deps t libs ~ext =
List.rev_map libs ~f:(fun (lib : Lib.t) ->
if Lib.is_local lib then
Alias.stamp_file
(lib_files_alias ~dir:(Lib.src_dir lib) ~name:(Lib.name lib) ~ext)
else
Build_system.stamp_file_for_files_of t.build_system
~dir:(Lib.obj_dir lib) ~ext)
List.rev_map libs ~f:(file_deps_of_lib t ~ext)
end

module Deps = struct
Expand Down
2 changes: 2 additions & 0 deletions src/super_context.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ module Libs : sig
all the files with extension [ext] of libraries [libs]. *)
val file_deps : t -> Lib.L.t -> ext:string -> Path.t list

val file_deps_with_exts : t -> (Lib.t * string) list -> Path.t list
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A suggestion for a better name/implementation would be appreciated. I could also implement file_deps on top of this function, but don't see the point of making the code slower for no reason.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is not too much of a problem, but I agree that duplicating is not ideal.

Fortunately, you can extract the body of List.map into private function val file_deps_for_lib : Lib.t -> ext:string -> Build_system.t -> Path.t. That way, the two functions file_deps & file_deps_with_exts become one-liners.


(** Setup the alias that depends on all files with a given extension
for a library *)
val setup_file_deps_alias
Expand Down
1 change: 1 addition & 0 deletions src/utop.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ let setup sctx ~dir ~(libs : Library.t list) ~scope =
~scope
~dir:utop_exe_dir
~modules
~opaque:false
~requires
~flags:(Ocaml_flags.append_common
(Ocaml_flags.default ~profile:(Super_context.profile sctx))
Expand Down
6 changes: 3 additions & 3 deletions test/blackbox-tests/test-cases/intf-only/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ Successes:
ocamlc .foo.objs/foo__Intf.{cmi,cmti}
ocamlc .foo.objs/foo.{cmi,cmo,cmt}
ocamlc test/.bar.objs/bar.{cmi,cmo,cmt}
ocamlc test/bar.cma
ocamlopt .foo.objs/foo.{cmx,o}
ocamlopt test/.bar.objs/bar.{cmx,o}
ocamlopt test/bar.{a,cmxa}
ocamlopt test/bar.cmxs
ocamlc foo.cma
ocamlopt .foo.objs/foo.{cmx,o}
ocamlopt foo.{a,cmxa}
ocamlopt foo.cmxs
ocamlc foo.cma
ocamlc test/bar.cma

Errors:

Expand Down
6 changes: 3 additions & 3 deletions test/blackbox-tests/test-cases/menhir/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
ocamldep src/.test.eobjs/test_menhir1.mli.d
ocamlc src/.test.eobjs/test_menhir1.{cmi,cmti}
ocamlc src/.test.eobjs/lexer1.{cmi,cmo,cmt}
ocamlopt src/.test.eobjs/lexer1.{cmx,o}
ocamlc src/.test.eobjs/test_base.{cmi,cmti}
ocamlc src/.test.eobjs/lexer2.{cmi,cmo,cmt}
ocamlc src/.test.eobjs/test.{cmi,cmo,cmt}
ocamlopt src/.test.eobjs/lexer2.{cmx,o}
ocamlopt src/.test.eobjs/test_menhir1.{cmx,o}
ocamlopt src/.test.eobjs/lexer1.{cmx,o}
ocamlopt src/.test.eobjs/test_base.{cmx,o}
ocamlopt src/.test.eobjs/lexer2.{cmx,o}
ocamlc src/.test.eobjs/test.{cmi,cmo,cmt}
ocamlopt src/.test.eobjs/test.{cmx,o}
ocamlopt src/test.exe
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/package-dep/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
ocamldep .foo.objs/foo.ml.d
ocamlc .foo.objs/foo.{cmi,cmo,cmt}
ocamlc .bar.objs/bar.{cmi,cmo,cmt}
ocamlc bar.cma
ocamlopt .foo.objs/foo.{cmx,o}
ocamlopt .bar.objs/bar.{cmx,o}
ocamlopt bar.{a,cmxa}
ocamlopt bar.cmxs
ocamlc bar.cma
ocamlopt .foo.objs/foo.{cmx,o}
ocamlopt foo.{a,cmxa}
ocamlopt foo.cmxs
ocamlc foo.cma
Expand Down
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/reporting-of-cycles/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ the second run of dune.

$ dune build @package-cycle
Dependency cycle between the following files:
_build/.aliases/default/.a-files-00000000000000000000000000000000
--> _build/.aliases/default/.b-files-00000000000000000000000000000000
_build/.aliases/default/.b-files-00000000000000000000000000000000
--> _build/.aliases/default/.a-files-00000000000000000000000000000000
--> _build/.aliases/default/.b-files-00000000000000000000000000000000
[1]

$ dune build @simple-repro-case
Expand Down
12 changes: 6 additions & 6 deletions test/blackbox-tests/test-cases/scope-bug/run.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@
ocamldep blib/sub/.sub.objs/sub.ml.d
ocamlc blib/sub/.sub.objs/sub.{cmi,cmo,cmt}
ocamlc blib/.blib.objs/blib.{cmi,cmo,cmt}
ocamlc blib/blib.cma
ocamlc alib/.alib.objs/alib__.{cmi,cmo,cmt}
ocamlopt alib/.alib.objs/alib__.{cmx,o}
ocamlopt blib/sub/.sub.objs/sub.{cmx,o}
ocamlopt blib/.blib.objs/blib.{cmx,o}
ocamlopt blib/blib.{a,cmxa}
ocamlopt blib/blib.cmxs
ocamlc alib/.alib.objs/alib__.{cmi,cmo,cmt}
ocamlopt alib/.alib.objs/alib__.{cmx,o}
ocamlopt blib/sub/.sub.objs/sub.{cmx,o}
ocamlopt blib/sub/sub.{a,cmxa}
ocamlopt blib/sub/sub.cmxs
ocamlc blib/sub/sub.cma
ocamlc blib/blib.cma
ocamlc alib/.alib.objs/alib.{cmi,cmo,cmt}
ocamlopt alib/.alib.objs/alib.{cmx,o}
ocamlc alib/.alib.objs/alib__Main.{cmi,cmo,cmt}
ocamlopt alib/.alib.objs/alib__Main.{cmx,o}
ocamlopt alib/alib.{a,cmxa}
ocamlopt alib/alib.cmxs
ocamlopt blib/sub/sub.{a,cmxa}
ocamlopt blib/sub/sub.cmxs
ocamlc alib/alib.cma