Skip to content

Better type error reporting#684

Merged
marcoeilers merged 6 commits into
masterfrom
meilers_type_errors
May 4, 2023
Merged

Better type error reporting#684
marcoeilers merged 6 commits into
masterfrom
meilers_type_errors

Conversation

@marcoeilers

@marcoeilers marcoeilers commented May 3, 2023

Copy link
Copy Markdown
Contributor

Addressing issue #642 (at least in most cases).

For the example in #642, Viper now reports

Silicon found 2 errors in 1.16s:
  [0] Type error in the expression at test.vpr@6.10. Expected Int but got Foo. (test.vpr@6.10--6.29)
  [1] Type error in the expression at test.vpr@6.33. Expected Int but got Foo. (test.vpr@6.33--6.52)

Additionally, in some cases, Viper used to mention internal type variables in error messages, e.g. Expected type Ref, but found #R#1451 at the expression at test.vpr@15.8 (test.vpr@15.8--15.11). This PR fixes that to be Expected type Ref, but found Foo[Bool, Perm] at the expression at test.vpr@17.6 (test.vpr@17.6--17.17) instead.

@marcoeilers marcoeilers requested a review from fpoli May 3, 2023 20:49
Comment thread src/main/scala/viper/silver/parser/Resolver.scala Outdated
Comment thread src/main/scala/viper/silver/parser/ParseAst.scala

@fpoli fpoli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Someone should probably run an auto formatter on these files at some point; spaces around symbols are not used uniformly.

@fpoli

fpoli commented May 4, 2023

Copy link
Copy Markdown
Member

For the example in #642, Viper now reports

Silicon found 2 errors in 1.16s:
  [0] Type error in the expression at test.vpr@6.10. Expected Int but got Foo. (test.vpr@6.10--6.29)
  [1] Type error in the expression at test.vpr@6.33. Expected Int but got Foo. (test.vpr@6.33--6.52)

That means that the position of the errors is now

Position: assert new_foo(new_foo(1)) == new_foo(new_foo(1))
                 ^^^^^^^^^^^^^^^^^^^    ^^^^^^^^^^^^^^^^^^^

I find it unusual and perhaps a bit confusing that the errors are precise about the types but not about the positions. The actual expressions that have type Foo but should be Int are

Position: assert new_foo(new_foo(1)) == new_foo(new_foo(1))
                         ^^^^^^^^^^             ^^^^^^^^^^

With the imprecise error positions, I would prefer an error message saying "Type error in a subexpression of ..." to remember that I should check all the arguments myself.

@marcoeilers

Copy link
Copy Markdown
Contributor Author

For the example in #642, Viper now reports

Silicon found 2 errors in 1.16s:
  [0] Type error in the expression at test.vpr@6.10. Expected Int but got Foo. (test.vpr@6.10--6.29)
  [1] Type error in the expression at test.vpr@6.33. Expected Int but got Foo. (test.vpr@6.33--6.52)

That means that the position of the errors is now

Position: assert new_foo(new_foo(1)) == new_foo(new_foo(1))
                 ^^^^^^^^^^^^^^^^^^^    ^^^^^^^^^^^^^^^^^^^

I find it unusual and perhaps a bit confusing that the errors are precise about the types but not about the positions. The actual expressions that have type Foo but should be Int are

Position: assert new_foo(new_foo(1)) == new_foo(new_foo(1))
                         ^^^^^^^^^^             ^^^^^^^^^^

With the imprecise error positions, I would prefer an error message saying "Type error in a subexpression of ..." to remember that I should check all the arguments myself.

Positions are also fixed now. New output:

Silicon found 2 errors in 1.18s:
  [0] Type error in the expression at test.vpr@6.18. Expected type Int but found Foo. (test.vpr@6.18--6.28)
  [1] Type error in the expression at test.vpr@6.41. Expected type Int but found Foo. (test.vpr@6.41--6.51)

@marcoeilers marcoeilers merged commit 5bdd26c into master May 4, 2023
@marcoeilers marcoeilers deleted the meilers_type_errors branch May 4, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants