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

fix #16150 improve type mismatch errors #16152

Merged
merged 3 commits into from
Dec 9, 2020
Merged

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Nov 26, 2020

fix #16150

after PR:

Error: type mismatch: got 'seq[string]' for 'p("hello")' but expected 'O = object'

with --declaredlocs --listfullpaths:off

t11377.nim(14, 8) Error: type mismatch:
 got 'seq[string]' for 'p("hello")' [sequence]
 but expected 'O = object' [object declared in t11377.nim(6, 5)]

use -d:nimLegacyTypeMismatch for previous behavior.

CI failure unrelated: #16169

compiler/semfields.nim Outdated Show resolved Hide resolved
@Clyybber
Copy link
Contributor

I think this is too verbose, consider if/block/case expressions. We should fix the column number instead. (They don't start at 0, try echo undefined)

@Araq Araq merged commit f344a70 into nim-lang:devel Dec 9, 2020
@timotheecour timotheecour deleted the pr_fix_16150 branch December 10, 2020 01:55
@timotheecour
Copy link
Member Author

@Clyybber

I think this is too verbose, consider if/block/case expressions.

behavior as implemented in this PR is consistent with sigmatch errors which also show the expression on error, after template expansion, and regardless of how verbose that can become:

when true:
  proc fn(a: int) = discard
  proc bar(a: int): auto = 1.0
  var z = 2
  template foo(): untyped = bar(1+z)
  # fn(foo()) # will show an error that contains: but expression 'bar(1 + z)' is of type: float64
  var a: int = foo() # will now show an error containing:  got 'float64' for 'bar(1 + z)' but expected 'int'
  • future work can add an option to instead show the expression as it appears in source code (before template expansion), and apply this to all error messages (including type mismatch and sigmatch errors, in particular the rendering of condition in doAssert failure messages) instead of showing the expression after template expansion.

This is not trivial to do though (related: issues like #7039 and #11789 for const folding or this: #14863 (comment)

Simply remembering both variants can create a compiler that uses an expontential amount of memory. (Nest these calls to get 2x2x2x2x... memory consumption.) There should only be a single AST that works with all the constructs. My "semtityped" AST idea.

There are ways to fix it though.

We should fix the column number instead. (They don't start at 0, try echo undefined)

  • That should also be fixed, it's an orthogonal issue

mildred pushed a commit to mildred/Nim that referenced this pull request Jan 11, 2021
* fix nim-lang#16150 improve type mismatch errors

* allow -d:nimLegacyTypeMismatch

* address comment
ardek66 pushed a commit to ardek66/Nim that referenced this pull request Mar 26, 2021
* fix nim-lang#16150 improve type mismatch errors

* allow -d:nimLegacyTypeMismatch

* address comment
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.

Can we improve error message for wrong proc result type?
4 participants