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

Record "weak dependencies" with -trans-mod #39

Closed
wants to merge 2 commits into from

Conversation

lpw25
Copy link
Contributor

@lpw25 lpw25 commented Apr 17, 2014

Currently, -trans-mod simply ignores dependencies on unused module
aliases. This commit changes this behaviour so that unused module aliases are
included in the list of imported interfaces but without a CRC.

This change means that the imported interfaces list (as displayed by
ocamlobjinfo) is now an accurate representation of what was used during the
compilation of a module. Whilst the contents of unused module aliases is not
used, their existence is still required in order for the module to compile
successfully.

Previously, a simple file foo.ml containing an unused module alias to Bar
would have the following output from ocamlobjinfo for foo.cmo:

File foo.cmo
Unit name: Foo
Interfaces imported:
        4ad29aa1be509426919169d97aad0a82        Pervasives
        df1763e3e7e64b9b8ebea6f93b0a95b3        Foo
Uses unsafe features: no
Force link: no

with this patch it instead has output:

File foo.cmo
Unit name: Foo
Interfaces imported:
        --------------------------------        Bar
        6cda9672639792333f53de8e8ff9e71d        Pervasives
        df1763e3e7e64b9b8ebea6f93b0a95b3        Foo
Uses unsafe features: no
Force link: no

This is useful because now we can see that if bar.cmi is deleted then this
module will no longer compile successfully.

This commit achieves this change by making the digests optional in every list
of imported interfaces and implementations.

In the case of implementations this replaces the previous use of the
cmx_not_found_crc dummy value for implementations where a .cmx was not
available.

ocamlobjinfo and read_cmt print imports with an empty digest as a line of
dashes. All other uses of the imported interface lists ignore imports with an
empty digest, leaving their behaviour unchanged.

This commit also adds a (new) description of -trans-mod to the man pages.

Currently, -trans-mod simply ignores dependencies on unused module
aliases. This commit changes this behaviour so that unused module aliases are
included in the list of imported interfaces but without a CRC.

This change means that the imported interfaces list (as displayed by
`ocamlobjinfo`) is now an accurate representation of what was used during the
compilation of a module. Whilst the contents of unused module aliases is not
used, their existance is still required in order for the module to compile
successfully.

Previously, a simple file `foo.ml` containing an unused module alias to `Bar`
would have the following output from `ocamlobjinfo` for `foo.cmo`:

    File foo.cmo
    Unit name: Foo
    Interfaces imported:
            4ad29aa1be509426919169d97aad0a82        Pervasives
            df1763e3e7e64b9b8ebea6f93b0a95b3        Foo
    Uses unsafe features: no
    Force link: no

with this patch it instead has output:

    File foo.cmo
    Unit name: Foo
    Interfaces imported:
            --------------------------------        Bar
            6cda9672639792333f53de8e8ff9e71d        Pervasives
            df1763e3e7e64b9b8ebea6f93b0a95b3        Foo
    Uses unsafe features: no
    Force link: no

This is useful because now we can see that if `bar.cmi` is deleted then this
module will no longer compile successfully.

This commit acheives this change by making the digests optional in every list
of imported interfaces and implementations.

In the case of implementations this replaces the previous use of the
`cmx_not_found_crc` dummy value for implementations where a `.cmx` was not
available.

`ocamlobjinfo` and `read_cmt` print imports with an empty digest as a line of
dashes. All other uses of the imported interface lists ignore imports with an
empty digest, leaving their behaviour unchanged.

This commit also adds a (new) description of -trans-mod to the man pages.
@lpw25
Copy link
Contributor Author

lpw25 commented Apr 17, 2014

Note that I did not bump the magic numbers as I'm still not clear what the policy is on those.

then raise(Error(Unavailable_unit name))
| Loaded -> ()
match crco with
Some crc when old_crc <> crc ->
Copy link
Member

Choose a reason for hiding this comment

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

minor nitpick, but I'd find the code more readable with

if `crco <> None && crco <> Some old_crc then
  raise ...;
match state with ...

@gasche
Copy link
Member

gasche commented Apr 18, 2014

The magic numbers need to change as soon as the format of compiled files changes (would a .cm*-consumer written against the previous interface work unchanged when passed files produced by the new implementation?). If I understand correctly, the serialized values have Some foo in stead of foo, so the magic numbers need to change.

I find the idea of making the dependency on the interface in the .cmi very good, but I'm not sure I understand why implementations are impacted as well. I understood .cmx files to be about cross-module inlining or optimizations, does -trans-mod actually change that in any way? I would have guessed that code inlined from a module-aliased dependency still need to recompiled if the dependency's implementation changes (while -trans-mod is about interface changes). Or is it that you thought the "optional CRC" could bring better structure to the .cmx-handling as well? What is the overlap with the -nocmx PR?

@lpw25
Copy link
Contributor Author

lpw25 commented Apr 18, 2014

The magic numbers need to change as soon as the format of compiled files changes (would a .cm*-consumer written against the previous interface work unchanged when passed files produced by the new implementation?)

I guess the thing I don't quite understand is that whenever the stuff in types.ml changes the .cmi files will have a different format, but that doesn't seem to result in a bump to the magic number. Anyway, I'll add a commit to bump the affected magic numbers.

Or is it that you thought the "optional CRC" could bring better structure to the .cmx-handling as well? What is the overlap with the -nocmx PR?

The change to the implementation imports lists is indeed unrelated to -trans-mod, but it does seem to be a better representation than what is used currently.

I considered making that a separate patch, but since the code changes are almost identical to those required for the interfaces lists I thought it would be easier to review as a single patch. Both changes also depend on the same change to Consistbl.extract.

A None value in the implementations list represents an implementation for which a .cmx file was not found. IIUC the -nocmx PR still creates a .cmx so I don't think the two are related.

@lpw25
Copy link
Contributor Author

lpw25 commented Apr 20, 2014

I've attached a commit that bumps the magic numbers for cmi, cmo, cma, cmx, cmxa, cmxs and cmt. I'm not certain the change to the cmxs magic number is needed. The dynunit type has changed, but it appears that this is stored as a symbol (caml_plugin_header) rather than saved directly into the file.

@lpw25
Copy link
Contributor Author

lpw25 commented Apr 23, 2014

Are any more changes required before this can be merged? I would like to see it in 4.02 as it is part of our solution to the "pack" problem (along with module aliases and the -prefix argument which Jeremie will hopefully add soon).

@dbuenzli
Copy link
Contributor

@lpw25 is there any documentation about all that ? It may influence the design of a library I'll work on in a few weeks. If not, could you maybe make a blog post on what all this will mean for the future.

@lpw25
Copy link
Contributor Author

lpw25 commented Apr 23, 2014

@dbuenzli I was planning on doing a blog post once all the pieces were on trunk, so that I could show simple examples that would compile. Ideally that would include support from ocamlbuild, but I think that is too difficult to do before 4.02, so I'll probably do it once this patch and -prefix are merged.

@dbuenzli
Copy link
Contributor

Excellent ! I'm especially interested in the "damned this will link in everything and produce huge executables so I have to pollute the toplevel module name space" problem.

@mshinwell
Copy link
Contributor

We've been testing this at Jane Street, and shown that it (plus the prefix stuff) does solve the executable size problem. I'm going to commit this to trunk shortly.

@mshinwell
Copy link
Contributor

Committed to trunk (rev 14719).
Leo, please close this.

@lpw25 lpw25 closed this May 1, 2014
yallop added a commit to yallop/ocaml that referenced this pull request Aug 18, 2015
Change the syntax of implicit parameters
Gbury pushed a commit to Gbury/ocaml that referenced this pull request Oct 29, 2019
lthls added a commit to lthls/ocaml that referenced this pull request Dec 19, 2019
anmolsahoo25 pushed a commit to anmolsahoo25/ocaml that referenced this pull request Aug 25, 2020
poechsel pushed a commit to poechsel/ocaml that referenced this pull request Jul 2, 2021
Fix error "integer literal exceeds the range of representable integers
of type nativeint" in the code that runs only on 64-bit targets, but
need to compile on 32-bit target.
lpw25 pushed a commit to lpw25/ocaml that referenced this pull request Nov 4, 2021
lpw25 pushed a commit to lpw25/ocaml that referenced this pull request Nov 12, 2021
stedolan added a commit to stedolan/ocaml that referenced this pull request May 24, 2022
173842c Merge flambda-backend changes
ed7eba2 Remove leading space from LINE. (ocaml-flambda/flambda-backend#484)
bd61170 Bump magic numbers (ocaml#5)
c50c47d Add CI builds with local allocations enabled
1412792 Move local allocations support behind '-extension local'
6d8e42a Better tail call behaviour in caml_applyN
c7dac3d Typemod: toplevel bindings escape even if no variables are bound
82d6c3e Several fixes for partial application and currying
d05c70c Pprintast support for new local syntax
e0e62fc Typecheck x |> f y as (f y x), not ((f y) x)
d7e34ce Remove autogeneration of @ocaml.curry
b9a0593 Port ocaml-flambda/flambda-backend#493
0a872d9 Code review fixes from ocaml-flambda/flambda-backend#491
6c168bb Remove local allocation counting
3c6e7f0 Code review fixes from ocaml-flambda/flambda-backend#478
bb97207 Rename Lambda.apply_position
a7cb650 Quieten Makefile when runtime dep files are not present
c656dc9 Merge flambda-backend changes
11b5424 Avoid printing double spaces in function argument lists
7751faa Restore locations to Typedtree.{pat,let}_bound_idents_full
e450b6c add build_ocaml_compiler.sexp
0403bb3 Revert PR 9895 to continue installing VERSION
b3447db Ensure new local attributes are namespaced properly
7f213fc Allow empty functions again
8f22ad8 Bugfix: ensure local domain state is initialised
80f54dd Bugfix for Selectgen with regions
e8133a1 Fix external-external signature inclusion
9840051 Bootstrap
d879f23 Merge remote-tracking branch 'jane/local-reviewed' into local-merge
94454f5 Use Local_store for the local allocations ref
54a164c Create fewer regions, according to typechecking (ocaml#59)
1c2479b Merge flambda-backend changes
ce34678 Fix printing of modes in return types
91f2281 Hook mode variable solving into Btype.snapshot/backtrack
54e4b09 Move Alloc_mode and Value_mode to Btype
ff4611e Merge flambda-backend changes
ce62e45 Ensure allocations are initialised, even dead ones
6b6ec5a Fix the alloc.ml test on 32-bit builds
81e9879 Merge flambda-backend changes
40a7f89 Update repo URL for ocaml-jst, and rename script.
0454ee7 Add some new locally-allocating primitives (ocaml#57)
8acdda1 Reset the local stack pointer in exception handlers (ocaml#56)
8dafa98 Improve typing for (||) and (&&) (ocaml#55)
8c64754 Fix make_check_all_arches (ocaml#54)
b50cd45 Allow arguments to primitives to be local even in tail position (ocaml#53)
cad125d Fix modes from or-patterns (ocaml#50)
4efdb72 Fix tailcalls tests with inlining (ocaml#52)
4a795cb Flambda support (ocaml#49)
74722cb Add [@ocaml.principal] and [@ocaml.noprincipal] attributes, and use in oo.mli
6d7d3b8 Ensure that functions are evaluated after their arguments (flambda-backend ocaml#353)
89bda6b Keep Sys.opaque_identity in Cmm and Mach (port upstream PR 9412)
a39126a Fix tailcalls within regions (ocaml#48)
4ac4cfd Fix stdlib manpages build
3a95f5e Merge flambda-backend changes
efe80c9 Add jane/pull-flambda-patches script
fca94c4 Register allocations for Omitted parameter closures (ocaml#47)
103b139 Remove various FIXMEs (ocaml#46)
62ba2c1 Bootstrap
a0062ad Allow local allocations for various primitives (ocaml#43)
7a2165e Allow primitives to be poly-moded (ocaml#43)
2af3f55 Fix a flaky test by refactoring TypePairs (ocaml#10638)
58dd807 Bootstrap
ee3be10 Fix modes in build_apply for partial applications
fe73656 Tweak for evaluation order of labelled partial applications (ocaml#10653)
0527570 Fix caml_modify on local allocations (ocaml#40)
e657e99 Relax modes for `as` patterns (ocaml#42)
f815bf2 Add special mode handling for tuples in matches and let bindings (ocaml#38)
39f1211 Only take the upper bounds of modes associated with allocations (ocaml#37)
aec6fde Interpret arrow types in "local positions" differently
c4f3319 Bootstrap
ff6fdad Add some missing regions
40d586d Bootstrap
66d8110 Switch to a system with 3 modes for values
f2c5a85 Bugfix for Comballoc with local allocations. (ocaml#41)
83bcd09 Fix bug with root scanning during compaction (ocaml#39)
1b5ec83 Track modes in Lambda.lfunction and onwards (ocaml#33)
f1e2e97 Port ocaml#10728
56703cd Port ocaml#10081
eb66785 Support local allocations in i386 and fix amd64 bug (ocaml#31)
c936b19 Disallow local recursive non-functions (ocaml#30)
c7a193a GC support for local allocations (ocaml#29)
8dd7270 Nonlocal fields (ocaml#28)
e19a2f0 Bootstrap
694b9ac Add syntax to the parser for local allocations (ocaml#26)
f183008 Lower initial stack size
918226f Allow local closure allocations (ocaml#27)
2552e7d Introduce mode variables (ocaml#25)
bc41c99 Minor fixes for local allocations (ocaml#24)
a2a4e60 Runtime and compiler support for more local allocations (ocaml#23)
d030554 Typechecking for local allocations (ocaml#21)
9ee2332 Bugfix missing from ocaml#20
02c4cef Retain block-structured local regions until Mach.
86dbe1c amd64: Move stack realloc calls out-of-line
324d218 More typing modes and locking of environments
a4080b8 Initial version of local allocation (unsafe)

git-subtree-dir: ocaml
git-subtree-split: 173842c
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
2118848 Ensure that (if c then E) counts E as tail position for typing (ocaml#42)
a937de9 Remove mode variables in ocamlc -i (ocaml#41)
865d039 Improve typing of local unboxed values (ocaml#26)
97be59d Backport PR 10090 (ocaml#39)
7613440 Allow use of `local_` to adjust regions for loops. (ocaml#35)

git-subtree-dir: ocaml
git-subtree-split: 2118848
sadiqj pushed a commit to sadiqj/ocaml that referenced this pull request Feb 21, 2023
* added new lambda construct Lmutvar (ocaml#9954)

* added new construct Lmutlet and removed Variable attribute

* refactorized Llet and Lmutlet printing

* Simplified some computations for mutable variable cases

* Fixed an overlooked transformation to Lmutvar and added cautionnary comment

* Fixups for backported PR (mostly in translcomprehension)

Co-authored-by: Keryan Didier <keryan.didier@ocamlpro.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants