Skip to content

Commit acdf563

Browse files
cknittclaude
andauthored
Show signature mismatches as the actual and expected declaration, each under its location (#9140)
* Show signature mismatches as the actual and expected declaration, each under its location Why: A signature mismatch printed every enclosing module in full, then the two declarations, then their locations as "file:line:" lines with a label on the next line that read like headings with nothing under them, and the reasons without a break. Which text was where had to be matched up by hand, and declarations that read the same looked like a declaration not including itself. The type checker's errors separate their parts with blank lines and indent the content under a lead-in. Includemod.report_error now leaves out a module or module type comparison when an item inside it follows, so that only the items that differ are printed, each after a line naming where it is: "In module P:", "In module type P.T:", "In the result of functor P:", "In the parameter X of functor P:", or "At position ..." otherwise, once for items at the same position. Missing items are listed in the order of the expected signature, and in a functor's parameter, where inclusion is reversed, the implementation's side is still the actual declaration. Diagnostic modified (type declaration in an interface): Example: // Foo.res type t = {a: int, b: int} // Foo.resi type t = {x: int, y: int} Before (diagnostic): The implementation Foo.res does not match the interface foo.cmi: Type declarations do not match: type t = {a: int, b: int} is not included in type t = {x: int, y: int} Foo.resi:1:1-25: Expected declaration Foo.res:1:1-25: Actual declaration Fields number 1 have different names, a and x. After (diagnostic): The implementation Foo.res does not match the interface foo.cmi: Actual declaration (Foo.res:1:1-25): type t = {a: int, b: int} Expected declaration (Foo.resi:1:1-25): type t = {x: int, y: int} Fields number 1 have different names, a and x. Diagnostic modified (value in a nested module): Example: // Foo.res module Inner = { let value = "not an int" } // Foo.resi module Inner: { let value: int } Before (diagnostic): In module Inner: Modules do not match: { let value: string } is not included in { let value: int } In module Inner: Values do not match: let value: string is not included in let value: int Foo.resi:2:3-16: Expected declaration Foo.res:2:7-11: Actual declaration After (diagnostic): In module Inner: Actual declaration (Foo.res:2:7-11): let value: string Expected declaration (Foo.resi:2:3-16): let value: int Diagnostic modified (functor result): Example: module type S = { type t } module M: { module F: (X: S) => S with type t = X.t } = { module F = (X: S) => { type t = int } } Before (diagnostic): Signature mismatch: Modules do not match: … In module F: Modules do not match: … At position module F: (X) => <here> Modules do not match: … At position module F: (X) => <here> Type declarations do not match: type t = int is not included in type t = X.t functor_signature_mismatch.res:6:36: Expected declaration functor_signature_mismatch.res:9:5-16: Actual declaration After (diagnostic): Signature mismatch: In the result of functor F: Actual declaration (functor_signature_mismatch.res:9:5-16): type t = int Expected declaration (functor_signature_mismatch.res:6:36): type t = X.t Diagnostic modified (functor parameter): Example: module M: { module F: (X: {let a: int}) => {} } = { module F = (X: {let a: string}) => {} } Before (diagnostic): Signature mismatch: Modules do not match: … the module, then the functor, each pair printed in full At position module F: (X: <here>) => ... Values do not match: let a: int is not included in let a: string inclusion_layout_functor_parameter.res:12:7-19: Expected declaration inclusion_layout_functor_parameter.res:6:7-16: Actual declaration After (diagnostic): In the parameter X of functor F: Actual declaration (inclusion_layout_functor_parameter.res:12:7-19): let a: string Expected declaration (inclusion_layout_functor_parameter.res:6:7-16): let a: int Diagnostic modified (missing values): Example: module M: { module N: { let a: int let b: string } } = { module N = {} } Before (diagnostic): Signature mismatch: Modules do not match: … the module, then module N, each pair printed in full In module N: The value `b` is required but not provided inclusion_layout_missing_values.res:5:5-17: Expected declaration In module N: The value `a` is required but not provided inclusion_layout_missing_values.res:4:5-14: Expected declaration After (diagnostic): In module N: The value `a` is required but not provided. Expected declaration (inclusion_layout_missing_values.res:4:5-14): let a: int The value `b` is required but not provided. Expected declaration (inclusion_layout_missing_values.res:5:5-17): let b: string Diagnostic modified (runtime representation reason): Before (diagnostic): Their runtime representations differ: the first declaration states values whose typeof is "object", the second declaration states values whose typeof is "string". After (diagnostic): Their runtime representations differ: The actual declaration states values whose typeof is "object". The expected declaration states values whose typeof is "string". Subst.extension_constructor dropped the location of an extension constructor when writing a .cmi, unlike every other declaration. It keeps it now, so the expected side of an extension mismatch has a position, and reanalyze also records a reference to an exception declared in an interface. Diagnostic modified (extension constructor declared in an interface): Example: // Foo.res type t += Boom(string) // Foo.resi type t += Boom(int) Before (diagnostic): Foo.res: Expected declaration After (diagnostic): Expected declaration (Foo.resi:4:3-13): type t += Boom(int) Review: Is anything dropped that was useful? The enclosing modules printed in full repeated the items below them; the path line names where the items are. A big part was printed as "..." for the same reason, so the size check is gone. Review: Why "actual" and "expected" and not "implementation" and "interface"? The same errors are reported for module type constraints and functor arguments within one file, where there is no interface. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLpf7RN1MiRcubPgRn Signed-off-by: Christoph Knittel <ck@cca.io> * Label the two sides when runtime representations differ Why: "The actual declaration states values whose typeof is "object"" reads clumsily: a declaration doesn't state values. The two lines now use the labels of the declaration blocks above them, followed by what each side states. Diagnostic modified (runtime representation reason): Before (diagnostic): Their runtime representations differ: The actual declaration states values whose typeof is "object". The expected declaration states values whose typeof is "string". After (diagnostic): Their runtime representations differ: Actual declaration: typeof "object" Expected declaration: typeof "string" Diagnostic modified (@notundefined reason): Before (diagnostic): Their runtime representations differ: The expected declaration is @notundefined, but the actual declaration can hold undefined. After (diagnostic): Their runtime representations differ: Actual declaration: can hold undefined Expected declaration: @notundefined Mapping modified (what a declaration states, Ast_untagged_variants.describe_declared_values): Before (representation): @instanceof("Dog") ↦ instances of Dog @instanceof("Dog") of a subclass ↦ instances of Dog, a subclass of Animal arrays ↦ arrays @typeof("string") ↦ values whose typeof is "string" nothing ↦ no runtime class or typeof After (representation): @instanceof("Dog") ↦ instances of Dog @instanceof("Dog") of a subclass ↦ instances of Dog, a subclass of Animal arrays ↦ arrays @typeof("string") ↦ typeof "string" nothing ↦ no stated class or typeof Review: Where do the labels come from? From the side names the caller passes, without their article: "the actual" and "declaration" make "Actual declaration"; the type definition errors of Typedecl get "Original definition" and "This definition". Review: Why no period? The lines are labels with a value, like the declaration labels, not sentences. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLpf7RN1MiRcubPgRn Signed-off-by: Christoph Knittel <ck@cca.io> * Keep the module type comparison around a permutation of its fields Why: Codex review of #9140: when two module types have the same fields in another order, the innermost symptom is the permutation, which shows no declarations, and leaving out the module type comparison around it left only "Illegal permutation of structure fields". A comparison is now left out only when a later symptom shows the declarations that differ. The parser gave a module type declaration in a signature no location, unlike one in a structure, so the expected side printed without a position. It now spans from the name to the end, as in a structure. Diagnostic modified (fields of a module type in another order): Example: module M: { module type T = { let a: int let b: int } } = { module type T = { let b: int let a: int } } Before (diagnostic): Signature mismatch: In module type T: Illegal permutation of structure fields After (diagnostic): Signature mismatch: Actual declaration (include_modtype_permutation.res:7:15-10:3): module type T = { let b: int let a: int } Expected declaration (include_modtype_permutation.res:2:15-5:3): module type T = { let a: int let b: int } In module type T: Illegal permutation of structure fields Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLpf7RN1MiRcubPgRn Signed-off-by: Christoph Knittel <ck@cca.io> * Report the actual side of a module type found by comparing the other way Why: Codex review of #9140: two module types must be equal, and check_modtype_equiv also includes the expected one in the actual one. An item only the actual module type has is found by that reverse check, but the error called it required and its declaration the expected one. This was mislabelled before too, and the comparison of the two module types printed above it was the only hint, which the new layout leaves out. The reverse check now adds a Reverse position to the path. Like a functor parameter it swaps the sides, and it is not printed. Diagnostic modified (an item only the actual module type has): Example: module M: { module type T = { let a: int } } = { module type T = { let a: int let b: int } } Before (diagnostic): In module type T: The value `b` is required but not provided. Expected declaration (inclusion_layout_module_type_extra.res:10:5-14): let b: int After (diagnostic): In module type T: The value `b` is only present in the actual declaration. Actual declaration (inclusion_layout_module_type_extra.res:10:5-14): let b: int Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLpf7RN1MiRcubPgRn Signed-off-by: Christoph Knittel <ck@cca.io> --------- Signed-off-by: Christoph Knittel <ck@cca.io> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 25e3dfb commit acdf563

81 files changed

Lines changed: 741 additions & 801 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- Show a signature mismatch as the actual and the expected declaration, each under its location, with only the items that differ and the module path to them.
2+
- Keep the location of an extension constructor or exception declared in an interface, so that errors and reanalyze point to it.

‎compiler/ml/ast_untagged_variants.ml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,16 +322,16 @@ let runtime_annotation_to_string = function
322322
| Type_of block_type ->
323323
Printf.sprintf "@%s(\"%s\")" type_of (typeof_name block_type)
324324

325-
(* What a declaration's annotations state, for error messages *)
326-
let describe_declared_block_type = function
325+
(* What a declaration's annotations state about its values, for error
326+
messages *)
327+
let describe_declared_values = function
327328
| Some (InstanceType (Class {name; ancestors = []})) -> "instances of " ^ name
328329
| Some (InstanceType (Class {name; ancestors})) ->
329330
"instances of " ^ name ^ ", a subclass of "
330331
^ String.concat " and " ancestors
331332
| Some (InstanceType Array) -> "arrays"
332-
| Some block_type ->
333-
"values whose typeof is \"" ^ typeof_name block_type ^ "\""
334-
| None -> "no runtime class or typeof"
333+
| Some block_type -> "typeof \"" ^ typeof_name block_type ^ "\""
334+
| None -> "no stated class or typeof"
335335

336336
(* The [@notUndefined], [@instanceOf] or [@typeOf] annotation in [attrs],
337337
validated. Only [Typedecl] passes [~loc]; later readers see attributes it

‎compiler/ml/includecore.ml‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,16 @@ type type_mismatch =
191191
| Runtime_annotation of string * string
192192
| Not_undefined
193193

194+
(* A side of a comparison as a label: "the actual" and "declaration" make
195+
"Actual declaration" *)
196+
let label side decl =
197+
let side =
198+
match String.split_on_char ' ' side with
199+
| "the" :: rest -> String.concat " " rest
200+
| _ -> side
201+
in
202+
String.capitalize_ascii (side ^ " " ^ decl)
203+
194204
let report_type_mismatch0 first second decl ppf err =
195205
let pr fmt = Format.fprintf ppf fmt in
196206
match err with
@@ -225,27 +235,31 @@ let report_type_mismatch0 first second decl ppf err =
225235
| Record_representation (_rep1, _rep2) ->
226236
pr "Their internal representations differ"
227237
| Unboxed_representation b ->
228-
pr "Their internal representations differ:@ %s %s %s"
229-
(if b then second else first)
238+
pr "@[<v 2>Their internal representations differ:@,%s %s %s@]"
239+
(String.capitalize_ascii (if b then second else first))
230240
decl "uses unboxed representation"
231-
| Immediate -> pr "%s is not an immediate type" first
241+
| Immediate -> pr "%s %s is not an immediate type" first decl
232242
| Tag_name -> pr "Their @tag annotations differ"
233243
| Variant_representation s ->
234244
pr "The internal representations for case %s are not equal" (Ident.name s)
235245
| Not_undefined ->
236246
pr
237-
"Their runtime representations differ:@ %s %s is @notUndefined,@ but %s \
238-
%s can hold undefined"
239-
second decl first decl
247+
"@[<v 2>Their runtime representations differ:@,\
248+
%s: can hold undefined@,\
249+
%s: @notUndefined@]"
250+
(label first decl) (label second decl)
240251
| Runtime_annotation (declared1, declared2) ->
241-
pr
242-
"Their runtime representations differ:@ %s %s states %s,@ %s %s states %s"
243-
first decl declared1 second decl declared2
252+
pr "@[<v 2>Their runtime representations differ:@,%s: %s@,%s: %s@]"
253+
(label first decl) declared1 (label second decl) declared2
244254

245255
let report_type_mismatch first second decl ppf =
246256
List.iter (fun err ->
247-
if err = Manifest then ()
248-
else
257+
match err with
258+
| Manifest -> ()
259+
(* Its lines are labels, not sentences *)
260+
| Runtime_annotation _ | Not_undefined ->
261+
Format.fprintf ppf "@ %a" (report_type_mismatch0 first second decl) err
262+
| _ ->
249263
Format.fprintf ppf "@ %a." (report_type_mismatch0 first second decl) err)
250264

251265
let rec compare_constructor_arguments env cstr params1 params2 arg1 arg2 =
@@ -390,8 +404,8 @@ let compare_runtime_annotations env id decl1 decl2 =
390404
else
391405
[
392406
Runtime_annotation
393-
( Ast_untagged_variants.describe_declared_block_type declared1,
394-
Ast_untagged_variants.describe_declared_block_type declared2 );
407+
( Ast_untagged_variants.describe_declared_values declared1,
408+
Ast_untagged_variants.describe_declared_values declared2 );
395409
]
396410

397411
let type_declarations ?(equality = false) ~loc env name decl1 id decl2 =

0 commit comments

Comments
 (0)