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

Destructive substitution regression #7905

Closed
vicuna opened this issue Jan 29, 2019 · 6 comments
Closed

Destructive substitution regression #7905

vicuna opened this issue Jan 29, 2019 · 6 comments
Assignees

Comments

@vicuna
Copy link

vicuna commented Jan 29, 2019

Original bug ID: 7905
Reporter: @nojb
Assigned to: @Octachron
Status: resolved (set by @Octachron on 2019-01-31T20:17:53Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.08.0+dev/beta1/beta2
Category: typing
Monitored by: @yallop

Bug description

The following is accepted in 4.07, but rejected in 4.08.

module M : sig
  type t = A of t
end
type t := A.t = A of t

File "foo.mli", line 5, characters 34-35:
5 |              type t := M.t = A of t
                                      ^
Error: Unbound type constructor t
@vicuna
Copy link
Author

vicuna commented Jan 29, 2019

Comment author: @Octachron

As far as I can see, this is a local substitution, a new feature of 4.08? Your code example results in a syntax error with 4.07. Nevertheless, the error message might fall in an unfortunate local minimum: rewriting the local substitution to

type t := A.t = A of A.t

yields:

5 | type t := A.t = A of A.t
^^^^^^^^^^^^^^^^^^^^^^^^
Error: Only type synonyms are allowed on the right of :=

@vicuna
Copy link
Author

vicuna commented Jan 29, 2019

Comment author: @nojb

Indeed, you are right, I got confused -- sorry for the noise!

@vicuna
Copy link
Author

vicuna commented Jan 29, 2019

Comment author: @gasche

octachron, is the behavior you spotted a bug? The message you have (for the intuitively "correct" definition) looks like a parsing-error message delayed to type-checking time (I'm fine with this, but we may move it back to the parser one day if it has proper error messages), but the error of nojebar suggests that type-checking happens before the construction is rejected. Does it mean that the not-allowed rejection happens too late in the pipeline?

@vicuna
Copy link
Author

vicuna commented Jan 29, 2019

Comment author: @Octachron

It does look like the rejection happens too late.

@vicuna
Copy link
Author

vicuna commented Jan 29, 2019

Comment author: @trefis

https://github.com/ocaml/ocaml/blob/trunk/typing/typemod.ml#L1171

Feel free to move the code checking the shape of the declarations before the actual type-checking part.
But I agree that the error should come from the parser, when we finally get errors from there.

@vicuna
Copy link
Author

vicuna commented Jan 31, 2019

Comment author: @Octachron

The error message should be improved with #2230 . So thanks for the report nojebar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants