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

Fix copy scope bugs #9385

Merged
merged 3 commits into from
Jun 25, 2020
Merged

Fix copy scope bugs #9385

merged 3 commits into from
Jun 25, 2020

Conversation

lpw25
Copy link
Contributor

@lpw25 lpw25 commented Mar 21, 2020

There were two bugs in the handling of copy_scopes in Subst which lead to #9384 and the bugs from the comments on #9384.

One bug is the classic with_foo currying bug. The other uses separate copy_states for the parameters and body of a type constructor -- losing sharing between the two.

stedolan added a commit to janestreet/ocaml that referenced this pull request Apr 1, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
@lpw25
Copy link
Contributor Author

lpw25 commented May 6, 2020

@trefis I think you know this code, care to review it?

hhugo pushed a commit to janestreet/ocaml that referenced this pull request May 25, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
hhugo pushed a commit to janestreet/ocaml that referenced this pull request May 29, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
hhugo pushed a commit to janestreet/ocaml that referenced this pull request Jun 2, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
hhugo pushed a commit to janestreet/ocaml that referenced this pull request Jun 3, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
Copy link
Contributor

@trefis trefis left a comment

Choose a reason for hiding this comment

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

Good catch, thanks!

Sorry for the long delay before reviewing.

@hhugo
Copy link
Contributor

hhugo commented Jun 5, 2020

@lpw25 this just need a Change entry and fixes to comply with check-typo.
Will this be cherry picked in 4.11 ?

hhugo pushed a commit to janestreet/ocaml that referenced this pull request Jun 5, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
@Octachron
Copy link
Member

This seems like a small bug fix in the alpha period. It should be fine to cherry-pick to 4.11 . Except if @trefis or @lpw25 have any objections.

hhugo pushed a commit to janestreet/ocaml that referenced this pull request Jun 5, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
@lpw25
Copy link
Contributor Author

lpw25 commented Jun 5, 2020

Rebased, fixed check-typo issue, Changes entry added. This should indeed go in 4.11.

hhugo pushed a commit to janestreet/ocaml that referenced this pull request Jun 10, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
@lpw25
Copy link
Contributor Author

lpw25 commented Jun 14, 2020

Tests were failing after the rebase, I've updated them now.

hhugo pushed a commit to janestreet/ocaml that referenced this pull request Jun 24, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
@hhugo
Copy link
Contributor

hhugo commented Jun 24, 2020

Is this ready to be merged ?

@lpw25 lpw25 merged commit 1f9be49 into ocaml:trunk Jun 25, 2020
lpw25 added a commit that referenced this pull request Jun 25, 2020
Fix copy scope bugs

(cherry picked from commit 1f9be49)
@lpw25
Copy link
Contributor Author

lpw25 commented Jun 25, 2020

Cherry picked to 4.11 as a091c89

mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 16, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 20, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 20, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 21, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 21, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 30, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Jul 30, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 3, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 4, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 5, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 7, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 10, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 10, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 17, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 18, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 19, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 20, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Aug 28, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Sep 2, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Sep 2, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Sep 2, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
mshinwell pushed a commit to mshinwell/ocaml that referenced this pull request Sep 7, 2020
Fix incorrect copy_scopes in Subst

(cherry picked from commit 288bb81)
(cherry picked from commit 2e08e99)
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