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
Improve error reporting for ill-typed applicative functor type #1491
Changes from all commits
e172c39
c5722f1
a9aef56
2c67a57
f03aaad
cf2dd9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
(* TEST | ||
* expect | ||
*) | ||
|
||
type t = Set.Make(String).t | ||
[%%expect{| | ||
type t = Set.Make(String).t | ||
|} ] | ||
|
||
|
||
(* Check the error messages of an ill-typed applicatived functor type. *) | ||
module M = struct type t let equal = (=) end | ||
[%%expect{| | ||
module M : sig type t val equal : 'a -> 'a -> bool end | ||
|} ] | ||
|
||
type t = Set.Make(M).t | ||
[%%expect{| | ||
Line _, characters 9-22: | ||
type t = Set.Make(M).t | ||
^^^^^^^^^^^^^ | ||
Error: The type of M does not match Set.Make's parameter | ||
Modules do not match: | ||
sig type t = M.t val equal : 'a -> 'a -> bool end | ||
is not included in | ||
Set.OrderedType | ||
The value `compare' is required but not provided | ||
File "set.mli", line 52, characters 4-31: Expected declaration | ||
|} ] | ||
|
||
|
||
(* We would report the wrong error here if we didn't strengthen the | ||
type of the argument (type t wouldn't match). *) | ||
module F(X : sig type t = M.t val equal : unit end) | ||
= struct type t end | ||
[%%expect{| | ||
module F : | ||
functor (X : sig type t = M.t val equal : unit end) -> sig type t end | ||
|} ] | ||
|
||
type t = F(M).t | ||
[%%expect{| | ||
Line _, characters 9-15: | ||
type t = F(M).t | ||
^^^^^^ | ||
Error: The type of M does not match F's parameter | ||
Modules do not match: | ||
sig type t = M.t val equal : 'a -> 'a -> bool end | ||
is not included in | ||
sig type t = M.t val equal : unit end | ||
Values do not match: | ||
val equal : 'a -> 'a -> bool | ||
is not included in | ||
val equal : unit | ||
|} ] | ||
|
||
|
||
(* MPR#7611 *) | ||
module Generative() = struct type t end | ||
[%%expect{| | ||
module Generative : functor () -> sig type t end | ||
|}] | ||
|
||
type t = Generative(M).t | ||
[%%expect{| | ||
Line _, characters 9-24: | ||
type t = Generative(M).t | ||
^^^^^^^^^^^^^^^ | ||
Error: The functor Generative is generative, it cannot be applied in type | ||
expressions | ||
|}] | ||
|
||
|
||
|
||
module F(X : sig module type S module F : S end) = struct | ||
type t = X.F(Parsing).t | ||
end | ||
[%%expect{| | ||
Line _, characters 11-25: | ||
type t = X.F(Parsing).t | ||
^^^^^^^^^^^^^^ | ||
Error: The module X.F is abstract, it cannot be applied | ||
|}] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
aliases.ml | ||
applicative_functor_type.ml | ||
firstclass.ml | ||
generative.ml | ||
pr5911.ml | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -64,10 +64,15 @@ type error = | |
| Unbound_class of Longident.t | ||
| Unbound_modtype of Longident.t | ||
| Unbound_cltype of Longident.t | ||
| Ill_typed_functor_application of Longident.t | ||
| Ill_typed_functor_application | ||
of Longident.t * Longident.t * Includemod.error list option | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a small question here: is there a meaningful difference between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think neither is possible, so well, it's hard to say what counts as different. |
||
| Illegal_reference_to_recursive_module | ||
| Access_functor_as_structure of Longident.t | ||
| Apply_structure_as_functor of Longident.t | ||
| Wrong_use_of_module of Longident.t * [ `Structure_used_as_functor | ||
| `Abstract_used_as_functor | ||
| `Functor_used_as_structure | ||
| `Abstract_used_as_structure | ||
| `Generative_used_as_applicative | ||
] | ||
| Cannot_scrape_alias of Longident.t * Path.t | ||
| Opened_object of Path.t option | ||
| Not_an_object of type_expr | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That shouldn't be here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why not. It's about the same as fixing a typo in a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @sliquister .
I had already noticed the issue here but it's apparently not worth anyone's time to create a separate PR. So fixing it here sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I though it was a change that was included by mistake. Not sure why it's included in a commit that has literally nothing to do with it, but fair enough. :)