compiler error message can improve: when forgetting = after proc signature

[kobi2187]

You know, for beginners.

example code:

proc getData[A, B](table: TableRef[A, B]|Table[A, B]): seq[(A, B)] =
    for k, v in table.pairs:
        result.add((k, v))

without = I get (in vscode):

implementation of 'htmlentitydefs.getData(table: TableRef[getData.A, getData.B] or Table[getData.A, getData.B])[declared in htmlentitydefs.nim(268, 6)]' expected
undeclared identifier: 'k'
undeclared identifier: 'v'
invalid indentation
wrong number of variables
undeclared identifier: 'table'
undeclared identifier: 'result'
type mismatch: got <>

but expected one of:
proc add[T](x: var seq[T]; y: openArray[T])
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add(result: var string; x: int64)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add(x: var string; y: string)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add(x: var string; y: cstring)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add(result: var string; x: float)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add[T](x: var seq[T]; y: T)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add(x: var string; y: char)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add[A, B](t: var OrderedTable[A, B]; key: A; val: B)
for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add[A, B](t: TableRef[A, B]; key: A; val: B)
first type mismatch at position: 3
required type: B

[kobi2187]

Btw, nim is so cool! really enjoying it, and the power to extend types transparently.

[nc-x]

The error message is pretty clear IMO.

implementation of 'htmlentitydefs.getData(table: TableRef[getData.A, getData.B] or Table[getData.A, getData.B])[declared in htmlentitydefs.nim(268, 6)]' expected

This means that what you have written is a forward declaration and does not contain its implementation. Which means that you did this intentionally and are going to write the implementation later in this file. Or it could mean that you missed the =.

invalid indentation

This means that the indentation is wrong, because you miss the = no new scope is created, so you cannot indent.

undeclared identifier: ...

This means that because you never implemented the proc, those identifiers are never declared.

wrong number of variables

pairs takes one argument but the argument table here is non-existent.

type mismatch: got <>

(IDK what is result doing in global scope and why it is immutable but) this means that you passed wrong arguments to the add procedure. Which is also correct, since k,v are non-existant.

---

So I don't see what should/could be done here if people don't bother to learn the language.

[timotheecour]

its a common error, and its easy to improve err msg generated by compiler here. We can just show just 1 relevant error (eg:

Missing `=` between proc signature and body

) instead of cascade of errors as shown above.

[nc-x]

@timotheecour
But then (unlike the current error message), that error message is wrong if you intended it to be a forward declaration and accidentally indented the next line.

The current error message is 100% correct for all cases.

Also the node in the Ast for the above case stores just the proc as a forward decl, and the next line is another node. There is no easy way AFAIK to produce 100% correct messages for all cases if your suggestion is followed.

Adding extra complexity to the code for fixing something which is already correct is a wasted effort IMO.

So this should be a WontFix. But then thats my opinion.

[timotheecour]

I added low priority label.

here's some more info that shd be mentioned.

I'm using latest devel 0229dfd

I have no idea what vscode is doing in terms of post processing err msgs from nim (nor what version was @kobi2187 using) but I'm getting a different error msg:

• with nim c main.nim is actually different

/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: invalid indentation
      for k, v in table.pairs:

(ignore the full path; what matters is fact error is Error: invalid indentation)

• with both nim check and --errormax:10 I get:

/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: invalid indentation
      for k, v in table.pairs:
      ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: invalid indentation
      for k, v in table.pairs:
      ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 17) Error: undeclared identifier: 'table'
      for k, v in table.pairs:
                  ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: wrong number of variables
      for k, v in table.pairs:
      ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) template/generic instantiation from here
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(27, 9) Error: undeclared identifier: 'result'
          result.add((k, v))
          ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) template/generic instantiation from here
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(27, 21) Error: undeclared identifier: 'k'
          result.add((k, v))
                      ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) template/generic instantiation from here
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(27, 24) Error: undeclared identifier: 'v'
          result.add((k, v))
                         ^
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) template/generic instantiation from here
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(27, 15) Error: type mismatch: got <>
but expected one of:
proc add[T](x: var seq[T]; y: openArray[T])
  for a 'var' type a variable needs to be passed, but 'result' is immutable
proc add(result: var string; x: int64)
...
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(25, 6) Error: implementation of 't0096.getData(table: TableRef[getData.A, getData.B] or Table[getData.A, getData.B])[declared in /Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(25, 6)]' expected
  proc getData[A, B](table: TableRef[A, B]|Table[A, B]): seq[(A, B)]

[nc-x]

 /Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: invalid indentation
      for k, v in table.pairs:

(ignore the full path; what matters is fact error is Error: invalid indentation)

The compiler by default stops at the first error message. The first error encountered is of course the invalid indentation. Because the previous line (the proc forward declaration) is absolutely correct and has no errors whatsoever.

---

(And your second output is the same as in the OP)

[timotheecour]

(And your second output is the same as in the OP)

no, the one in OP shows invalid indentation (which is the relevant msg) somewhere in the middle; so that's a vscode plugin bug IMO (in how they present nim errors), not a nim bug.

I still think we can improve a bit by simply saying in this case (for 1st msg):

/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: invalid indentation
      for k, v in table.pairs:

changed to:

/Users/timothee/git_clone/nim/timn/tests/nim/all/t0096.nim(26, 5) Error: invalid indentation (or missing `=` if following a declaration))
      for k, v in table.pairs:

that's a 1 line patch in compiler, and is a small but user friendly improvement.

(a more refined patch would autodetect whether we're following a declaration, but may be un-necessary)

[EDIT] a separate issue is that sometimes errors produced via nim check (or errormax) are direct cause of previous already shown errors instead of showing "un-correlated" ones

[nc-x]

(or missing `=` if following a declaration)

The part if following a declaration would be false most of the time. There are many places where you can get the indentation wrong. And for those cases, the error message would consist of half useless stuff, and then you will have someone (might be me ;)) open up an issue on github saying bad error message when indentation wrong.

So IMO this is not an improvement.

But lets wait for what @Araq has to say.

[kobi2187]

Hello again,
I think error messages in general can be improved a lot, and can be systematic.
starting from an empty file, that produces a warning
for syntax errors stop at first problem,
for ambiguous intentions like in the above example, maybe show both intentions:

1. expected = sign after proc signature on line #...
2. forward declaration on line #... needs implementation but none was found
   which is currently my preferred suggestion for handling this.

I think, for a newcomer, it's important to have clear messages of what to change, and how --from the language perspective, less important is the why (internals) from the compiler's perspective.

Most languages that I know don't need the "=" sign after the signature, so this is bound to be a common mistake for newcomers.

PS: for inspiration and examples, the best error reporting I saw was in the cobra language. it was always clear what to do, and sometimes even explained how to fix the code. from that perspective it was a joy to work with. it sounds like a small thing, but actually prevents a lot of head scratching, and doesn't interrupt your train of thought.

[nc-x]

Well I am not going to argue anymore. If someone wants to implement this, go ahead.

But I still stand by my points, which I am rewriting here for those who don't want to read through all the above posts.

1. These error messages are already pretty clear and concise. Yeah, in general, many error messages can and should be improved but this is not one of them.
2. Newcomers might have a difficulty because... well because they are newcomers. But then they are supposed to learn the syntax and semantics of the language, not guess them. The basic concepts that are required to comprehend the error messages in this issue can pretty easily be understood if someone has taken time to go through the manual and the tutorials.
3. "Showing both intentions..." would make for the worst error messages ever. Either someone should take the effort that the error message is correct for each different scenario, or let it be. Showing a message like "this is a blah blah error, it could be due to one of the following reasons - you missed a blah, you accidentally added a blah, or you probably meant a blah" would be an instant turnoff.
4. The benefits that you guys are asking for, does not justify the complexity to implement the "smart" error message system so that it gives more context-aware error messages because the error message is already correct

---

Anyways I won't be discussing this topic anymore.

[alehander92]

@nc-x syntax errors are always a bit harder to explain: it is obvious to you as an experienced nim guy what might be the error, but this is because you learned to deal with it, not because it's a good error message.

Actually this kind of error message makes the user guess the error instead of saying exactly what might be the possible reasons. Showing possible reasons is not going to turn off people: it's helpful, and help is the only reason error messages exist at all instead of just returning 1.

The change might be technically hard, if this is so, I guess it's ok to not implement it, but it doesn't mean the error message is perfect.

[Araq]

Well nim check can produce fewer follow error messages, so I'm leaving this open until it produces better results in some way.

[Araq]

One year passed, nothing changed.

[timotheecour]

special case of the more general #10229