File, line number and 'error' repeated in compiler error message.

[nevdelap]

These are the two cases I can remember seeing.

class Test

	def main is shared
		print (1 +
			  2)	# test.cobra(5): error: test.cobra(5,1): error: Cannot mix tabs and s...
		a = 1 +		# test.cobra(6): error: test.cobra(6,10): error: Expecting an expression.

Tested against 2402.

[hopscc]

Occurs in all errors generated from lexing/tokenizer.

Tokenizer errors put source line and info in message.
Parser uses these messages to generate a ParserException.
Default conversion to console string prepends file, line and type info again.

patchfile has codefix, new test and rel note addendum

[Chuck]

• The test case does appear to be useful because it does not fail before applying the rest of the patch.

• I couldn't find anywhere that extra location information was being included just for tokenizer errors. Neither the tokenizer itself nor the TokenizerError class do this. Parser._recordError does this. Sure enough, you can get double location info with a parser error (that is not a tokenizer error) whether the patch is applied or not:

  class X
  	def main
  		x = _ y
  # 106-parser-error.cobra(4): error: 106-parser-error.cobra(4,7): error: Unexpected line continuation character.
  

[Chuck]

In the above comment, "Parser._recordError" should be "Parser._makeError"

[Chuck]

I have the fix.

[Chuck]

Fixed in changeset:2407

The fix was to simply remove any error message enhancement in Parser._makeError. We also now have a strong test for the compiler error message format in order to avoid regressions.

[nevdelap]

Hi, I tested this, and where it was giving...

test.cobra(6): error: test.cobra(6,10): error:

it's now giving...

test.cobra(6): error:

rather than...

test.cobra(6,10): error:

I thought the column number where the error is would be a useful bit of information to retain in the error message.

[hopscc]

Yeah the column number where the error was detected is definitely necessary - needed that a couple of times.

Sorry chuck My bad
I generated code that threw all the errors via .throwError from CobraTokenizer and all of them gave the double position info.
Tracked that to passing through Parser._makeError as well as Node.consoleString so corrected that for messages coming from the Tokenizer only - didnt think about anything from the Parser also passing
through the same codepath.
Generated a different clean test for the test suite after checking only for the error message with single position info, which passed
- never tried it on a clean tree - double bad.

Thats what happens when you try and fit a quick bugfix into a spare half hour or so and it doesnt...

Reopened for column number fix.
override consoleString in Parser.cobra (?)

[hopscc]

patch to reassert colno info on errors, mod error line format test to accomodate
No new tests(:-).

[Chuck]

Cobra doesn't generally include column number information because it does not currently correct it for TABs for editors.

[hopscc]

a) Thats only true since your fix removing the error message enhancement in _makeError
(re: including col number info).

b) Neither has any other stand alone compiler (i.e not integrated with an editor) since compilation was invented. (re: correcting for TABs).

[Chuck]

Regarding (a) that _makeError method is not invoked when errors come from outside the parser such as during .bindInh, .bindInt and .bindImp. I think it would be weird if parser errors had (incorrect) column info and other errors had none...

[hopscc]

Well thats been the case from whatever initial release of cobra displayed error info
(lines and cols) up till now and it hasnt seemed to be weird enough to cause an issue
and
Perhaps a better answer might be to provide col info for errors coming from outside the parser rather than suppressing ones from inside the parser (and yes I know theres issues with doing that as well).

How can it be an improvement to provide less info about the point of an error when the info is available?

Also Its only incorrect column info if you are saying that the position is the "editor display column number" as opposed to the line column number or text line offset or <whatever descriptions compilers usually use for column in a line an error occurred at>.

[Chuck]

Patch applied in changeset:2425