Fix reproducibility for -no-alias-deps#9991
Conversation
gasche
left a comment
There was a problem hiding this comment.
Which "strings" are we talking about here? (I think you mean the strings in the type-checker that are part of the compilation-unit metadata; and in particular not string constants in the module itself, right?)
Why is there no new testsuite example that checks something on strings somehow?
bbd3956 to
783150a
Compare
gasche
left a comment
There was a problem hiding this comment.
I still don't really understand the explanation for the PR, but I think that the implementation makes sense.
|
The "strings" are the names of persistent modules within identifiers. If the cmi file is present then the string in the identifier in the environment comes from the read of the file system. If the cmi file is not present then the string in the identifier comes from the source code at each point where the file is referenced, and is shared with the other occurrences of that string in the cmt file. |
783150a to
d607057
Compare
gasche
left a comment
There was a problem hiding this comment.
The PR now looks good to me. (But please consider addressing @Octachron question(s) before merging.)
d607057 to
0166a70
Compare
|
Rebased, added a comment and added a Changes entry. |
|
This fix seems specialized enough to be backported to 4.12. Any objections? |
|
None. |
Fix reproducibility for `-no-alias-deps` (cherry picked from commit d0ddf25)
|
Cherry-picked to 4.12 as 1d2363f . |
Fix reproducibility for `-no-alias-deps` (cherry picked from commit d0ddf25)
|
For records: an unforeseen consequence of this PR is that more cmi files compiled with and without external f: M.x -> M.xwith -no-alias-deps in bytecode mode and without it in native mode. |
|
We just came across this problem when building Sundials/ML with OCaml 4.12.0 as @Octachron predicted! The lack of |
#9345 makes environment summaries reproducible by only adding
Env_persistentin cases where the entry materially changes the environment. This ensures that cmt files do not depend on which unrelated cmi files are on the filesystem.However, such unrelated cmis still affect sharing of strings in initial environment itself. Normally this sharing is not observable because if you try to look up a module for which there is no cmi then you will error. However, with
-no-alias-depswe allow look ups of module aliases to succeed even if there is no cmi, which reveals the difference in sharing and leads to reproducibility issues.This PR changes the behaviour in
-no-alias-depsmode to only add persistent modules to the initial environment if doing so would materially change the environment.It also modifies the reproducibility test in the test suite to trigger this case.