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

Record tails in error messages are printed wrong #938

Closed
vkleen opened this issue Nov 23, 2022 · 5 comments
Closed

Record tails in error messages are printed wrong #938

vkleen opened this issue Nov 23, 2022 · 5 comments

Comments

@vkleen
Copy link
Contributor

vkleen commented Nov 23, 2022

Record tails in error messages are printed without a preceding ;. E.g.

nickel> let fail : forall a. {; a} -> Bool = fun r => r
error: incompatible types
  ┌─ repl-input-0:1:47
  │
1 │ let fail : forall a. {; a} -> Bool = fun r => r
  │                                               ^ this expression
  │
  = The type of the expression was expected to be `Bool`
  = The type of the expression was inferred to be `{a}`
  = These types are not compatible

The {a} should really be {; a} in keeping with the surface syntax.

@yannham
Copy link
Member

yannham commented Nov 24, 2022

I guess the pretty printer is at fault, here

@vkleen
Copy link
Contributor Author

vkleen commented Nov 24, 2022

It seems the pretty printer does the right thing:

❯ cargo run -- pprint-ast <<<'let f: forall a. {; a} = {} in f'
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `target/debug/nickel pprint-ast`
let f : forall a. { ; a}  = {  } in f%

It's probably the more ancient printer for error reporting?

@vkleen
Copy link
Contributor Author

vkleen commented Nov 24, 2022

The problem seems to be that the Display implementation for Types doesn't use the pretty printer. Instead it's an ad-hoc implementation that's probably missing an edge case somewhere. I think we should replace that Display implementation with a call to the pretty printer similar to the one for RichTerm.

@yannham
Copy link
Member

yannham commented Nov 24, 2022

Ah, indeed, it should use the pretty printer. Note that we have to make sure the output is strictly the same, because the error reporting code for contracts relies on knowing the exact number of characters (at least, for basic type constructors) and spacing to underline the right thing. Or maybe we can do things differently, to ensure that we underline the right thing in a more resilient way.

@yannham
Copy link
Member

yannham commented Jun 15, 2023

This is fixed on master, probably because of #1262.

@yannham yannham closed this as completed Jun 15, 2023
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

3 participants