Skip to content

Commit ec64177

Browse files
committed
Enable layout histories (#1823)
* enable display_histories * accept test changes and mark them as unreviewed * improve CR messages for unreviewed tests --------- Co-authored-by: Alan Chang <[email protected]> Remove duplicated creation_reasons from layout history error messages (#1825) * stop storing duplicates in creation_reasons * accept test changes * reduce to only displaying one creation reason * accept test changes * prefer other reasons over Concrete_creation * accept test changes * stop building history tree if flatten is enabled * remove CR version Co-authored-by: Richard Eisenberg <[email protected]> * Add CR for assumption in combine_histories Co-authored-by: Richard Eisenberg <[email protected]> --------- Co-authored-by: Alan Chang <[email protected]> Co-authored-by: Richard Eisenberg <[email protected]> Improve layout history error message formatting and wording (#1832) * remove extra whitespace * fix Unequal_var_layouts error fmt * improve message wording * fix spacing * accept test changes * grammar and phrasing * test changes --------- Co-authored-by: Alan Chang <[email protected]> Improve layout history error message: add path field to Type_argument value_creation_reason (#1834) * add Path.t to Type_argument * test changes * add Imported_type_argument * Imported_type_argument for imported type params * fix float64 error format * test changes * add more tests * update layout reason not just in failure case * tests with more than one type param * combine Imported_type_argument with Type_argument * add option_argument_layout to predef * track position and arity for Type_argument * test changes * more tests around typa argument arity * have Imported_type_argument as a creation_reason * test changes * add note about performance benchmark * extract position format logic into function * add comments about 1-indexed * change equals to is * test changes * define list_argument_layout * format comments --------- Co-authored-by: Alan Chang <[email protected]> Improve layout history error message: additional info on value layout defaulting (#1855) * add message when defaulting to layout value * test changes * relax to all const defaulting * test changes * improve wording * test changes --------- Co-authored-by: alanechang <[email protected]> Improve layout history error message: change message wording and add more tests (#1887) * change imported reason wording * test changes * lowercase * test changes * add more tests * change wording for match * update Immediate_polymorphic_variant message * test changes Improve layout history error message: hide layout history reason of various generalized vars (#1889) * add generalized creation reason * use change layout reason for value decl * change layout reason for decl type params * test changes * additional tests * add "the" * test changes * improve generalize on type decl * test changes * update err msg test case * add print_loc_in_lowercase * test changes * update_decls_layout_reason code cleanup Improve layout history error message: better test coverage (#1924) * Annotation * concrete layout * value layout * immediate layout * float64 layout * the rest * fix probe test * fix tests * more changes * more value test * add note * remove unused With_constraint constructor * fix format * remove unused void_creation_reason * remove unused Value_kind * add Missing_cmi test * consistent names for Type_variable&Univar * add param to Unannotated_type_parameter * add CRs * fix wording * add warning to all reasons we can't produce * more CRs * fix formatting * add new tests * enhance creation reason when missing cmi * fix Unannotated_type_parameter * test changes * code formatting (only) * Record missing cmi in the layout history * Add a CR * add broken test * fix is_missing_cmi check --------- Co-authored-by: Richard Eisenberg <[email protected]> Support for [@error_message] attribute on layout annotations (#1943) support for error_message attr on layout annots add error_message to the builtin_attrs table change error_message_attr to return first refactor - layout_of_annotation_or_attr to layout_of_annotation - Err_msg_attr to With_error_message fix tests improve formatting Support for [@error_message] attribute on Pexp_constraint (#1954) * implement error_message on Pexp_constraint * add tests * force message to be on new line * fix typo Improve layout history error message: omit history for signature mismatch (#2089) strip away layout history for signature mismatch Layout history: mark `typing-immediate/immediate.ml` as reviewed (#2144) mark test file as reviewed Improve layout history error message: review new messages (#2091) * remove layouts v2.9 CRs * "must be representable" instead of var name * catch missing history generalization * add history to Bad_univar_layout * remove crs * Toplevel_nonvalue refer to types * change Non_value_in_sig to refer to types * fix typo of Polymorphic * type argument to a class constructor * change message of Not_a_value * remove stale test file * refer to types in all messages * update unannotated universal variable message * split into Class_type_argument&Class_term_argument * add comment on update_generalized_ty_layout_reason * undo change to Not_a_value * update Not_a_value message fix merge conflicts
1 parent 50c77b2 commit ec64177

File tree

72 files changed

+4033
-260
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

72 files changed

+4033
-260
lines changed

chamelon/compat.jst.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ open Typedtree
22
open Types
33
open Mode
44

5-
let dummy_jkind = Jkind.value ~why:Type_argument
5+
let dummy_jkind = Jkind.value ~why:(Unknown "dummy_layout")
66
let dummy_value_mode = Value.legacy
77
let mkTvar name = Tvar { name; jkind = dummy_jkind }
88

ocaml/lambda/matching.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4101,7 +4101,7 @@ let report_error ppf = function
41014101
fprintf ppf
41024102
"Non-value detected in translation:@ Please report this error to \
41034103
the Jane Street compilers team.@ %a"
4104-
(Jkind.Violation.report_with_name ~name:"This expression") err
4104+
(Jkind.Violation.report_with_name ~name:"this expression") err
41054105
| Illegal_record_field c ->
41064106
fprintf ppf
41074107
"Sort %s detected where value was expected in a record field:@ Please \

ocaml/parsing/builtin_attributes.ml

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,15 @@ let builtin_attrs =
102102
; "loop"; "ocaml.loop"
103103
; "tail_mod_cons"; "ocaml.tail_mod_cons"
104104
; "unaliasable"; "ocaml.unaliasable"
105+
<<<<<<< HEAD
105106
; "builtin"; "ocaml.builtin"
106107
; "no_effects"; "ocaml.no_effects"
107108
; "no_coeffects"; "ocaml.no_coeffects"
108109
; "only_generative_effects"; "ocaml.only_generative_effects";
110+
||||||| parent of 114ab8b0 (Enable layout histories (#1823))
111+
=======
112+
; "error_message"; "ocaml.error_message"
113+
>>>>>>> 114ab8b0 (Enable layout histories (#1823))
109114
]
110115

111116
(* nroberts: When we upstream the builtin-attribute whitelisting, we shouldn't
@@ -696,3 +701,18 @@ let tailcall attr =
696701
(Warnings.Attribute_payload
697702
(t.attr_name.txt, "Only 'hint' is supported"));
698703
Ok (Some `Tail_if_possible)
704+
705+
let error_message_attr l =
706+
let inner x =
707+
match x.attr_name.txt with
708+
| "ocaml.error_message"|"error_message" ->
709+
begin match string_of_payload x.attr_payload with
710+
| Some _ as r ->
711+
mark_used x.attr_name;
712+
r
713+
| None -> warn_payload x.attr_loc x.attr_name.txt
714+
"error_message attribute expects a string argument";
715+
None
716+
end
717+
| _ -> None in
718+
List.find_map inner l

ocaml/parsing/builtin_attributes.mli

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ val has_once : Parsetree.attributes -> (bool, unit) result
192192
[unique_], and [once_] mode annotation attributes from let-bindings to both
193193
the let-bound expression and its pattern.
194194
*)
195+
<<<<<<< HEAD
195196
val mode_annotation_attributes_filter : Attributes_filter.t
196197

197198
(* CR layouts v1.5: Remove everything except for [Immediate64] and [Immediate]
@@ -208,3 +209,23 @@ val jkind_attribute_of_string : string -> jkind_attribute option
208209
as the attribute mechanism predates layouts.
209210
*)
210211
val jkind : Parsetree.attributes -> jkind_attribute Location.loc option
212+
||||||| parent of 114ab8b0 (Enable layout histories (#1823))
213+
(* CR layouts: we should eventually be able to delete ~legacy_immediate (after we
214+
turn on layouts by default). *)
215+
val jkind : legacy_immediate:bool -> Parsetree.attributes ->
216+
(Jane_asttypes.jkind_annotation option,
217+
Jane_asttypes.jkind_annotation) result
218+
=======
219+
(* CR layouts: we should eventually be able to delete ~legacy_immediate (after we
220+
turn on layouts by default). *)
221+
val jkind : legacy_immediate:bool -> Parsetree.attributes ->
222+
(Jane_asttypes.jkind_annotation option,
223+
Jane_asttypes.jkind_annotation) result
224+
225+
(** Finds the first "error_message" attribute, marks it as used, and returns its
226+
string payload. Returns [None] if no such attribute is present.
227+
228+
There should be at most one "error_message" attribute, additional ones are sliently
229+
ignored. **)
230+
val error_message_attr : Parsetree.attributes -> string option
231+
>>>>>>> 114ab8b0 (Enable layout histories (#1823))

ocaml/parsing/location.ml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ let print_filename ppf file =
261261
Some of the information (filename, line number or characters numbers) in the
262262
location might be invalid; in which case we do not print it.
263263
*)
264-
let print_loc ppf loc =
264+
let print_loc ~capitalize_first ppf loc =
265265
setup_colors ();
266266
let file_valid = function
267267
| "_none_" ->
@@ -287,7 +287,8 @@ let print_loc ppf loc =
287287

288288
let first = ref true in
289289
let capitalize s =
290-
if !first then (first := false; String.capitalize_ascii s)
290+
if !first then (first := false;
291+
if capitalize_first then String.capitalize_ascii s else s)
291292
else s in
292293
let comma () =
293294
if !first then () else Format.fprintf ppf ", " in
@@ -316,6 +317,9 @@ let print_loc ppf loc =
316317

317318
Format.fprintf ppf "@}"
318319

320+
let print_loc_in_lowercase = print_loc ~capitalize_first:false
321+
let print_loc = print_loc ~capitalize_first:true
322+
319323
(* Print a comma-separated list of locations *)
320324
let print_locs ppf locs =
321325
Format.pp_print_list ~pp_sep:(fun ppf () -> Format.fprintf ppf ",@ ")

ocaml/parsing/location.mli

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ val show_filename: string -> string
202202
val print_filename: formatter -> string -> unit
203203

204204
val print_loc: formatter -> t -> unit
205+
val print_loc_in_lowercase: formatter -> t -> unit
205206
val print_locs: formatter -> t list -> unit
206207

207208

ocaml/testsuite/tests/typing-immediate/immediate.ml

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,14 @@ end;;
143143
Line 2, characters 2-31:
144144
2 | type t = string [@@immediate]
145145
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
146-
Error: Type string has layout value, which is not a sublayout of immediate.
146+
Error: The layout of type string is value, because
147+
it is the primitive value type string.
148+
But the layout of type string must be a sublayout of immediate, because
149+
of the definition of t at line 2, characters 2-31.
147150
|}];;
151+
(* CR layouts v2.9: The "of the definition of t ..." part is not great and it
152+
should only refer to definitions that type check. Fixing it will involve
153+
building a second [final_env] in [transl_type_decl] which is costly. *)
148154

149155
(* Cannot directly declare a non-immediate type as immediate (variant) *)
150156
module B = struct
@@ -154,7 +160,10 @@ end;;
154160
Line 2, characters 2-41:
155161
2 | type t = Foo of int | Bar [@@immediate]
156162
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
157-
Error: Type t has layout value, which is not a sublayout of immediate.
163+
Error: The layout of type t is value, because
164+
it's a boxed variant type.
165+
But the layout of type t must be a sublayout of immediate, because
166+
of the annotation on the declaration of the type t.
158167
|}];;
159168

160169
(* Cannot directly declare a non-immediate type as immediate (record) *)
@@ -165,8 +174,13 @@ end;;
165174
Line 2, characters 2-38:
166175
2 | type t = { foo : int } [@@immediate]
167176
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
168-
Error: Type t has layout value, which is not a sublayout of immediate.
177+
Error: The layout of type t is value, because
178+
it's a boxed record type.
179+
But the layout of type t must be a sublayout of immediate, because
180+
of the annotation on the declaration of the type t/2.
169181
|}];;
182+
(* CR layouts v2.9: Investigate why the "/2" is here and check if it's only limited
183+
to expect tests. *)
170184

171185
(* Not guaranteed that t is immediate, so this is an invalid declaration *)
172186
module C = struct
@@ -177,7 +191,10 @@ end;;
177191
Line 3, characters 2-26:
178192
3 | type s = t [@@immediate]
179193
^^^^^^^^^^^^^^^^^^^^^^^^
180-
Error: Type t has layout value, which is not a sublayout of immediate.
194+
Error: The layout of type t is value, because
195+
of the definition of t at line 2, characters 2-8.
196+
But the layout of type t must be a sublayout of immediate, because
197+
of the definition of s at line 3, characters 2-26.
181198
|}];;
182199

183200
(* Can't ascribe to an immediate type signature with a non-immediate type *)
@@ -198,12 +215,14 @@ Error: Signature mismatch:
198215
type t = string
199216
is not included in
200217
type t : immediate
201-
the first has layout value, which is not a sublayout of immediate.
218+
The layout of the first is value, because
219+
it is the primitive value type string.
220+
But the layout of the first must be a sublayout of immediate, because
221+
of the definition of t at line 1, characters 15-35.
202222
|}];;
203223

204224
(* Same as above but with explicit signature *)
205225
module M_invalid : S = struct type t = string end;;
206-
module FM_invalid = F (struct type t = string end);;
207226
[%%expect{|
208227
Line 1, characters 23-49:
209228
1 | module M_invalid : S = struct type t = string end;;
@@ -214,7 +233,27 @@ Error: Signature mismatch:
214233
type t = string
215234
is not included in
216235
type t : immediate
217-
the first has layout value, which is not a sublayout of immediate.
236+
The layout of the first is value, because
237+
it is the primitive value type string.
238+
But the layout of the first must be a sublayout of immediate, because
239+
of the definition of t at line 1, characters 20-40.
240+
|}];;
241+
242+
module FM_invalid = F (struct type t = string end);;
243+
[%%expect{|
244+
Line 1, characters 20-50:
245+
1 | module FM_invalid = F (struct type t = string end);;
246+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
247+
Error: Modules do not match: sig type t = string end is not included in
248+
S
249+
Type declarations do not match:
250+
type t = string
251+
is not included in
252+
type t : immediate
253+
The layout of the first is value, because
254+
it is the primitive value type string.
255+
But the layout of the first must be a sublayout of immediate, because
256+
of the definition of t at line 1, characters 20-40.
218257
|}];;
219258

220259
(* Can't use a non-immediate type even if mutually recursive *)
@@ -226,7 +265,10 @@ end;;
226265
Line 2, characters 2-26:
227266
2 | type t = s [@@immediate]
228267
^^^^^^^^^^^^^^^^^^^^^^^^
229-
Error: Type s has layout value, which is not a sublayout of immediate.
268+
Error: The layout of type s is value, because
269+
it is the primitive value type string.
270+
But the layout of type s must be a sublayout of immediate, because
271+
of the definition of t at line 2, characters 2-26.
230272
|}];;
231273

232274

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
type 'a t = 'a
2+
type ('a ,'b) t2 = 'a
3+
type ('a ,'b,'c,'d,'e) t5 = 'a
4+
let f x = x + 1
5+
let f2 x = x
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
(* TEST
2+
flags = "-extension layouts_alpha"
3+
* expect
4+
*)
5+
6+
(*********************)
7+
(* Annotation errors *)
8+
9+
type ('a : value) value = unit
10+
11+
(* Type_declaration *)
12+
type t_void : void
13+
and t = t_void value
14+
15+
[%%expect{|
16+
type 'a value = unit
17+
Line 5, characters 8-14:
18+
5 | and t = t_void value
19+
^^^^^^
20+
Error: This type t_void = ('a : void) should be an instance of type
21+
('b : value)
22+
The layout of t_void is void, because
23+
of the annotation on the declaration of the type t_void.
24+
But the layout of t_void must overlap with value, because
25+
of the definition of value at line 1, characters 0-30.
26+
|}]
27+
28+
(* Type_parameter *)
29+
type ('a : void) t = 'a value
30+
31+
[%%expect{|
32+
Line 1, characters 21-23:
33+
1 | type ('a : void) t = 'a value
34+
^^
35+
Error: This type ('a : value) should be an instance of type ('a0 : void)
36+
The layout of 'a is void, because
37+
of the annotation on 'a in the declaration of the type t.
38+
But the layout of 'a must overlap with value, because
39+
of the definition of value at line 1, characters 0-30.
40+
|}]
41+
42+
(* Newtype_declaration *)
43+
let f (type a : void) (x: a value) = x
44+
45+
[%%expect{|
46+
Line 1, characters 26-27:
47+
1 | let f (type a : void) (x: a value) = x
48+
^
49+
Error: This type a should be an instance of type ('a : value)
50+
The layout of a is void, because
51+
of the annotation on the abstract type declaration for a.
52+
But the layout of a must be a sublayout of value, because
53+
of the definition of value at line 1, characters 0-30.
54+
|}]
55+
56+
(* Constructor_type_parameter *)
57+
type _ g = A : ('a: void) . 'a value -> unit g
58+
59+
[%%expect{|
60+
Line 1, characters 28-30:
61+
1 | type _ g = A : ('a: void) . 'a value -> unit g
62+
^^
63+
Error: This type ('a : void) should be an instance of type ('b : value)
64+
The layout of 'a is void, because
65+
of the annotation on a in the declaration of constructor A.
66+
But the layout of 'a must overlap with value, because
67+
of the definition of value at line 1, characters 0-30.
68+
|}]
69+
70+
(* Univar *)
71+
let f : ('a : void). 'a -> 'a value = assert false
72+
73+
[%%expect{|
74+
Line 1, characters 27-29:
75+
1 | let f : ('a : void). 'a -> 'a value = assert false
76+
^^
77+
Error: This type ('a : void) should be an instance of type ('b : value)
78+
The layout of 'a is void, because
79+
of the annotation on the universal variable 'a.
80+
But the layout of 'a must overlap with value, because
81+
of the definition of value at line 1, characters 0-30.
82+
|}]
83+
84+
(* Type_variable *)
85+
type t = 'a -> int as ('b : void)
86+
87+
[%%expect{|
88+
Line 1, characters 9-33:
89+
1 | type t = 'a -> int as ('b : void)
90+
^^^^^^^^^^^^^^^^^^^^^^^^
91+
Error: This alias is bound to type 'a -> int
92+
but is used as an instance of type ('b : void)
93+
The layout of 'a -> int is value, because
94+
it's a function type.
95+
But the layout of 'a -> int must be a sublayout of void, because
96+
of the annotation on the type variable 'b.
97+
|}]
98+
99+
(* Type_wildcard *)
100+
type t = 'a -> int as (_ : void)
101+
102+
[%%expect{|
103+
Line 1, characters 27-31:
104+
1 | type t = 'a -> int as (_ : void)
105+
^^^^
106+
Error: Bad layout annotation:
107+
The layout of 'a -> int is value, because
108+
it's a function type.
109+
But the layout of 'a -> int must be a sublayout of void, because
110+
of the annotation on the wildcard _ at line 1, characters 27-31.
111+
|}]
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
(* TEST
2+
flags = "-extension layouts_alpha"
3+
* expect
4+
*)
5+
6+
(*********************)
7+
(* Any layout errors *)
8+
9+
(* Missing_cmi *)
10+
(* See [any_missing_cmi.ml] *)
11+
12+
(* Wildcard *)
13+
(* Unable to produce this error *)
14+
15+
(* Unification_var *)
16+
(* Unable to produce this error *)
17+
18+
(* Initial_typedecl_env *)
19+
(* Unable to produce this error *)
20+
21+
(* Dummy_layout *)
22+
(* Unable to produce this error *)
23+
24+
(* Type_expression_call *)
25+
(* Unable to produce this error *)

0 commit comments

Comments
 (0)