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 Ctype.moregeneral's handling of row_name #886

Merged
merged 2 commits into from Feb 15, 2017

Conversation

Projects
None yet
4 participants
@lpw25
Contributor

lpw25 commented Nov 3, 2016

The following code:

type t = [ `Foo of float ]

module M : sig
  val baz : t ref
end = struct
  let baz = ref (`Foo 1)
end

currently produces the following error:

Error: Signature mismatch:
       Modules do not match:
         sig val baz : t ref end
       is not included in
         sig val baz : t ref end
       Values do not match:
         val baz : t ref
       is not included in
         val baz : t ref

which incorrectly gives the inner type of baz as t ref.

This is due to moregeneral linking the row variable with a type which is not t (it is actually the empty variant type) but which is still associated with the name t.

With this PR the error is instead given as:

Error: Signature mismatch:
       Modules do not match:
         sig val baz : [ `Foo of int ] ref end
       is not included in
         sig val baz : t ref end
       Values do not match:
         val baz : [ `Foo of int ] ref
       is not included in
         val baz : t ref

I've made this PR against 4.04 since it is a very small fix and should be easy for @garrigue to review before the release. But as it only affects the printing of types it can be safely left for 4.05 if that is preferred.

@damiendoligez

This comment has been minimized.

Show comment
Hide comment
@damiendoligez

damiendoligez Nov 3, 2016

Member

Time to the release is now counted in hours, so I'd rather leave this for 4.05 (or 4.04.1 if there is one).

Member

damiendoligez commented Nov 3, 2016

Time to the release is now counted in hours, so I'd rather leave this for 4.05 (or 4.04.1 if there is one).

@damiendoligez damiendoligez added this to the 4.05-or-later milestone Nov 3, 2016

@mshinwell

This comment has been minimized.

Show comment
Hide comment
@mshinwell

mshinwell Dec 9, 2016

Contributor

@garrigue Can you please review this?

Contributor

mshinwell commented Dec 9, 2016

@garrigue Can you please review this?

@mshinwell mshinwell requested a review from garrigue Dec 9, 2016

@garrigue

This comment has been minimized.

Show comment
Hide comment
@garrigue

garrigue Dec 10, 2016

Contributor

The fix is correct. But now we must merge this into trunk?

Contributor

garrigue commented Dec 10, 2016

The fix is correct. But now we must merge this into trunk?

@lpw25 lpw25 changed the base branch from 4.04 to trunk Dec 12, 2016

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Dec 12, 2016

Contributor

It's now based on trunk.

Contributor

lpw25 commented Dec 12, 2016

It's now based on trunk.

@lpw25 lpw25 merged commit fa3e806 into ocaml:trunk Feb 15, 2017

2 checks passed

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

camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017

Merge pull request #886 from lpw25/fix-moregeneral-variants
Fix Ctype.moregeneral's handling of row_name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment