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

Env.lookup_module: don't allow creating loops #8810

Merged
merged 4 commits into from Jul 18, 2019

Conversation

@trefis
Copy link
Contributor

commented Jul 17, 2019

$ cat bar.ml
module Foo = Bar

$ ~/.opam/4.07.1/bin/ocamlc -c -no-alias-deps -w -49 ./bar.ml
File "./bar.ml", line 1, characters 13-16:
Error: Unbound module Bar

$ ~/.opam/4.08.0/bin/ocamlc -c -no-alias-deps -w -49 ./bar.ml

$ cat baz.ml
module type X = module type of Bar [@remove_aliases]

$ ~/.opam/4.08.0/bin/ocamlc -c -no-alias-deps -w -49 ./baz.ml
Fatal error: exception Stack overflow
Raised by primitive operation at file "typing/ident.ml" (inlined), line 102, characters 20-40
Called from file "typing/ident.ml", line 200, characters 14-46
Called from file "typing/env.ml" (inlined), line 328, characters 26-52
Called from file "typing/env.ml", line 2146, characters 6-208
Called from file "typing/env.ml", line 2221, characters 12-76
Called from file "typing/mtype.ml", line 79, characters 8-64
Called from file "typing/mtype.ml", line 39, characters 19-55
Called from file "typing/env.ml", line 1753, characters 8-65
Called from file "typing/env.ml" (inlined), line 2169, characters 27-52
Called from file "typing/mtype.ml", line 444, characters 19-43
Called from file "typing/mtype.ml", line 470, characters 12-48
Called from file "typing/mtype.ml", line 442, characters 34-67
Called from file "typing/mtype.ml", line 449, characters 10-54
Called from file "typing/mtype.ml", line 470, characters 12-48
Called from file "typing/mtype.ml", line 442, characters 34-67
Called from file "typing/mtype.ml", line 449, characters 10-54
Called from file "typing/mtype.ml", line 470, characters 12-48
...

After this PR, bar.ml is rejected.

@garrigue

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2019

Interesting. Could you add a test case?

trefis added 2 commits Jul 17, 2019
@trefis trefis force-pushed the trefis:lookup-loop branch from ad792c4 to 311efd5 Jul 18, 2019
@trefis

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Done.

@trefis

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Btw, I believe we want to backport this on 4.09.
And apparently there is a 4.08.1+rc1, so we might want to backport on 4.08. (Though I'm surprised we're doing a 4.08.1 release).

@avsm

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

The -pack bug in 4.08.0 is causing build failures in packages like nocrypto, hence the motivation for a 4.08.1 release to fix that.

Copy link
Contributor

left a comment

Fine for merging.

@trefis

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

Thanks @garrigue !

I'll move the changelog entry to the 4.09 section, merge, and cherry-pick on the 4.09 branch.
(I don't think it's worth it to cherry-pick to 4.08, especially given that 4.08.1 has already been tagged)

@trefis trefis merged commit ddb4cd9 into ocaml:trunk Jul 18, 2019
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@trefis trefis deleted the trefis:lookup-loop branch Jul 29, 2019
trefis added a commit that referenced this pull request Aug 28, 2019
@trefis

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

I had apparently forgotten to cherry-pick to 4.09.
This is now done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.