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 MPR#7711 (constraint hides object fields and causes assertion) #1581

Merged
merged 4 commits into from Jan 30, 2018

Conversation

Projects
None yet
2 participants
@garrigue
Copy link
Contributor

garrigue commented Jan 24, 2018

Fix MPR#7711 by checking whether we are updating the self type in Ctype.unify3.

garrigue added some commits Jan 24, 2018

@garrigue garrigue requested a review from Octachron Jan 24, 2018

@Octachron
Copy link
Contributor

Octachron left a comment

This PR fixes the crash that I observed and I believe it is correct. (At most, I may have seen a very small slowdown on pathological class with ludicrous number of methods.)

@@ -2437,7 +2443,9 @@ and unify3 env t1 t1' t2 t2' =
begin match !umode with
| Expression ->
occur !env t1' t2';
link_type t1' t2
if is_self_type d1
then link_type t1' t2' (* PR#7711 *)

This comment has been minimized.

@Octachron

Octachron Jan 28, 2018

Contributor

Would it make sense to add a one-line resume of the issue, something like:
(* PR#7711: class-related functions assume that a class signature is always an object type *)

@@ -311,6 +316,7 @@ let concrete_object ty =
| Tvar _ -> false
| _ -> true


This comment has been minimized.

@Octachron

Octachron Jan 28, 2018

Contributor

There is an extraneous blank line here.

garrigue added some commits Jan 30, 2018

@garrigue garrigue merged commit 54225b5 into ocaml:trunk Jan 30, 2018

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.