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

Change error message for non-value class lets #1953

Merged
merged 1 commit into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Change error message for non-value class lets
The old way of producing this error required of_sort_for_error,
which causes trouble in the modal kinds world. This kills off
that terrible function.
  • Loading branch information
goldfirere committed Oct 19, 2023
commit 07ae983f91421297fcb7af67d0c7882aeadb5ed3
4 changes: 2 additions & 2 deletions ocaml/testsuite/tests/typing-layouts/basics_alpha.ml
Original file line number Diff line number Diff line change
Expand Up @@ -788,8 +788,8 @@ end
Line 3, characters 11-12:
3 | let VV v = v in
^
Error: Variables bound in a class must have layout value.
v has layout void, which is not a sublayout of value.
Error: The types of variables bound by a 'let' in a class function
must have layout value. Instead, v's type has layout void.
|}];;

(* Hits the Cfk_concrete case of Pcf_val *)
Expand Down
4 changes: 2 additions & 2 deletions ocaml/testsuite/tests/typing-layouts/basics_beta.ml
Original file line number Diff line number Diff line change
Expand Up @@ -702,8 +702,8 @@ end
Line 4, characters 8-9:
4 | let d = f u in
^
Error: Variables bound in a class must have layout value.
d has layout float64, which is not a sublayout of value.
Error: The types of variables bound by a 'let' in a class function
must have layout value. Instead, d's type has layout float64.
|}];;

(* Hits the Cfk_concrete case of Pcf_val *)
Expand Down
2 changes: 0 additions & 2 deletions ocaml/typing/jkind.ml
Original file line number Diff line number Diff line change
Expand Up @@ -490,8 +490,6 @@ let of_new_sort_var ~why =

let of_new_sort ~why = fst (of_new_sort_var ~why)

let of_sort_for_error ~why s = fresh_jkind (Sort s) ~why:(Concrete_creation why)

let of_const ~why : const -> t = function
| Any -> fresh_jkind Any ~why
| Immediate -> fresh_jkind Immediate ~why
Expand Down
5 changes: 0 additions & 5 deletions ocaml/typing/jkind.mli
Original file line number Diff line number Diff line change
Expand Up @@ -362,11 +362,6 @@ val of_new_sort_var : why:concrete_jkind_reason -> t * sort
(** Create a fresh sort variable, packed into a jkind. *)
val of_new_sort : why:concrete_jkind_reason -> t

(** There should not be a need to convert a sort to a jkind, but this is
occasionally useful for formatting error messages. Do not use in actual
type-checking. *)
val of_sort_for_error : why:concrete_jkind_reason -> sort -> t

val of_const : why:creation_reason -> const -> t

(* CR layouts v1.5: remove legacy_immediate when the old attributes mechanism
Expand Down
13 changes: 8 additions & 5 deletions ocaml/typing/typeclass.ml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ type error =
| Closing_self_type of class_signature
| Polymorphic_class_parameter
| Non_value_binding of string * Jkind.Violation.t
| Non_value_let_binding of string * Jkind.sort

exception Error of Location.t * Env.t * error
exception Error_forward of Location.error
Expand Down Expand Up @@ -1397,12 +1398,9 @@ and class_expr_aux cl_num val_env met_env virt self_scope scl =
(fun (loc, mode, sort) ->
Typecore.escape ~loc ~env:val_env ~reason:Other mode;
if not (Jkind.Sort.(equate sort value))
then let viol = Jkind.Violation.of_ (Not_a_subjkind(
Jkind.of_sort_for_error ~why:Let_binding sort,
Jkind.value ~why:Class_let_binding))
in
then
raise (Error(loc, met_env,
Non_value_binding (Ident.name id, viol)))
Non_value_let_binding (Ident.name id, sort)))
)
modes_and_sorts;
let path = Pident id in
Expand Down Expand Up @@ -2248,6 +2246,11 @@ let report_error env ppf = function
fprintf ppf
"@[Variables bound in a class must have layout value.@ %a@]"
(Jkind.Violation.report_with_name ~name:nm) err
| Non_value_let_binding (nm, sort) ->
fprintf ppf
"@[The types of variables bound by a 'let' in a class function@ \
must have layout value. Instead, %s's type has layout %a.@]"
nm Jkind.Sort.format sort

let report_error env ppf err =
Printtyp.wrap_printing_env ~error:true
Expand Down
1 change: 1 addition & 0 deletions ocaml/typing/typeclass.mli
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ type error =
| Closing_self_type of class_signature
| Polymorphic_class_parameter
| Non_value_binding of string * Jkind.Violation.t
| Non_value_let_binding of string * Jkind.sort

exception Error of Location.t * Env.t * error
exception Error_forward of Location.error
Expand Down
Loading