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

information in the typing error message could be made more precise #7316

Open
vicuna opened this issue Aug 4, 2016 · 8 comments
Open

information in the typing error message could be made more precise #7316

vicuna opened this issue Aug 4, 2016 · 8 comments

Comments

@vicuna
Copy link

vicuna commented Aug 4, 2016

Original bug ID: 7316
Reporter: kosik
Status: acknowledged (set by @damiendoligez on 2016-11-08T10:46:34Z)
Resolution: open
Priority: low
Severity: feature
Version: 4.03.0
Target version: 4.05.0 +dev/beta1/beta2/beta3/rc1
Category: typing
Monitored by: @gasche

Bug description

After typing:

  module M1 =
  struct
    type t1 =
      | V0
      | V1
      | V2
      | V3
      | V4
      | V5
      | V6
      | V7
      | V8
      | V9
  end
 
  module M2 =
  struct
    type t2 = M1.t1 =
      | V0
      | V1
      | V2
      | V3
      | V4
      | V5
      | V6
      | V7777
      | V8
      | V9
  end

Ocaml reports that:

  This variant or record definition does not match that of type M1.t1
  Fields number 8 have different names, V7 and V7777.

I think there are multiple ways how this kind of error message can be made more useful:

(1)

It can be simplified:

If there is a problem with a variant, just report:

    This variant definition does not match that of type M1.t1
    Fields number 8 have different names, V7 and V7777.

(i.e. omit the "or record" part if, in a given situation, the problem is not related to records).

(2)

The location information can be more precise:

  Instead of marking the whole variant definition,
  the compiler could mark the just the offending (i.e. V7777) part.

(3)

Grammar.

  Replace "Fields number" with "Field number" since the compiler reports only one field number.

Steps to reproduce

echo 'module M1 =
struct
  type t1 =
    | V0
    | V1
    | V2
    | V3
    | V4
    | V5
    | V6
    | V7
    | V8
    | V9
end

module M2 =
struct
  type t2 = M1.t1 =
    | V0
    | V1
    | V2
    | V3
    | V4
    | V5
    | V6
    | V7777
    | V8
    | V9
end;;' | ocaml
@vicuna
Copy link
Author

vicuna commented Aug 4, 2016

Comment author: kosik

Error messages for record can be, I think, improved in a similar way.
Currently, when I type:

  module M1 =
  struct
    type t = {f0:unit;
              f1:unit;
              f2:unit;
              f3:unit;
              f4:unit;
              f5:unit;
              f6:unit;
              f7:unit;
              f8:unit;
              f9:unit}
  end
 
  module M2 =
  struct
    type t = M1.t = {f0:unit;
                     f1:unit;
                     f2:unit;
                     f3:unit;
                     f4:unit;
                     f5:unit;
                     f6:unit;
                     f7777:unit;
                     f8:unit;
                     f9:unit}
  end

Ocaml compiler states that:

  Error: This variant or record definition does not match that of type M1.t
         Fields number 8 have different names, f7 and f7777.

Like before, there are multiple ways how this error message can be improved:

(1)

It can be simplified:

In this case, there is a problem with records (not with variants),
so I think the "This variant or" part of the error message can be left out.

(2)

The location information can be more precise:

Instead of marking the whole record definition,
the compiler could mark the just the offending (i.e. f7777) record field.

(3)

Grammar:

Replace "Fields number" with "Field number" since the compiler reports only one field number.

@vicuna
Copy link
Author

vicuna commented Nov 8, 2016

Comment author: @damiendoligez

"Fields" is in the plural because there are two fields involved: one in M1.t and the other in M2.t. You can read the sentence as "the fields numbered 8 are different".

@vicuna vicuna added the typing label Mar 14, 2019
@vicuna vicuna added this to the 4.05.0 milestone Mar 14, 2019
@github-actions
Copy link

github-actions bot commented May 9, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 9, 2020
@Drup
Copy link
Contributor

Drup commented May 9, 2020

@Octachron Well, I feel like that's also a good target for some diffing :D

@gasche
Copy link
Member

gasche commented May 9, 2020

@Drup could you submit a testsuite PR that contains the current behavior, in a known-misbehavior category or with otherwise a clear indication that this is something we want to improve?

@github-actions github-actions bot removed the Stale label Jun 10, 2020
@XVilka
Copy link
Contributor

XVilka commented Sep 18, 2021

There is a label "error-messages", I suggest to assign it to this issue, it will finding all these error message improvements easier:
error-messages

@github-actions
Copy link

github-actions bot commented Oct 5, 2022

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Oct 5, 2022
@Octachron
Copy link
Member

The current error message only reports

Error: This variant or record definition does not match that of type M1.t
       Fields have different names, f7 and f7777.

I agree that the variant or record definition exposes an implementation detail of the typechecker and should be narrowed to the right kind in the error message.

The location could be narrowed too, but that might require more work.

@github-actions github-actions bot removed the Stale label Oct 7, 2022
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

6 participants