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

Generalized open: fix another expansiveness bug #2167

Merged
merged 3 commits into from Nov 28, 2018

Conversation

Projects
None yet
4 participants
@yallop
Copy link
Member

commented Nov 27, 2018

[Related: #1506, #2147, #2166].

Now that open accepts an arbitrary module expression, the structure item open M should only be considered nonexpansive if M is nonexpansive.

Before this PR:

# let f = let module M = struct open struct let r = ref [] end let s = r end in M.s;;
val f : 'a list ref = {contents = []}
# f := [3];;
- : unit = ()
# List.hd f.contents ^ "two";; 
Segmentation fault

After this PR:

# let f = let module M = struct open struct let r = ref [] end let s = r end in M.s;;
val f : '_weak1 list ref = {contents = []}
@trefis

trefis approved these changes Nov 27, 2018

@trefis

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

Thanks for catching this!

@alainfrisch

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

check-typo failure:

./typing/typecore.ml:1880.39: [white-at-eol] whitespace at end of line

@yallop yallop force-pushed the yallop:nonexpansive-open branch from acfb507 to 50a32cd Nov 27, 2018

@yallop

This comment has been minimized.

Copy link
Member Author

commented Nov 27, 2018

@alainfrisch

check-typo failure:

Thanks! Fixed.

@objmagic

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

@yallop Thanks for catching this bug. I totally missed it...

@trefis trefis merged commit d8c5227 into ocaml:trunk Nov 28, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yallop yallop deleted the yallop:nonexpansive-open branch Nov 28, 2018

gretay-js added a commit to gretay-js/ocaml that referenced this pull request Dec 3, 2018

Generalized open: fix another expansiveness bug (ocaml#2167)
* open struct: add a failing test for expansiveness checking

* bug fix: 'open M' should only be non-expansive if 'M' is.

* open struct expansiveness fix: Changes entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.