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

More robust handling of type variables in mcomp #1517

Merged
merged 3 commits into from Dec 7, 2017

Conversation

Projects
None yet
2 participants
@trefis
Copy link
Contributor

trefis commented Dec 6, 2017

When comparing two types, Ctype.mcomp asserts that if neither of them is a variable, then they can't both be expanded to a variable.
The first commit adds an example of a code to the testsuite showing that this assertion doesn't hold.

The second commit removes the assertion and handles variables in the exact same way as before expansion of the types.
MPR#6158 and the related commit seem to confirm that this is the correct fix.

The handling of variables pre-expansion is redundant, but I left it in place to serve as a short-circuit since expansion might be costly.
I however doubt that it will be triggered often in practice, as "external" calls to mcomp are always preceeded by calls to reify.

@garrigue
Copy link
Contributor

garrigue left a comment

Well spotted.
The fix is trivial, so please go forward and merge it.

trefis added some commits Dec 6, 2017

testsuite: add a test trigger an assert false in Ctype.mcomp.
Fatal error: exception File "typing/ctype.ml", line 2028, characters 30-36: Assertion failed'

@trefis trefis force-pushed the trefis:mcomp-vars branch from e7d32e7 to 0f341e8 Dec 7, 2017

@trefis trefis merged commit 148e6a2 into ocaml:trunk Dec 7, 2017

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
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.