munificent / craftinginterpreters

Repository for the book "Crafting Interpreters"
http://www.craftinginterpreters.com/
Other
8.79k stars 1.03k forks source link

[jlox] Lox.runtimeError has a different format from Lox.error #933

Closed quipa closed 3 years ago

quipa commented 3 years ago

Lox.runtimeError places the message before the [line n], while the Scanner and Parser place it afterwards.

> 1 @ 2           // Scanner error
[line 1] Error: Unexpected character.
> 1 + * 2         // Parser error
[line 1] Error at '*': Expect expression.
> 1 + "hello"     // Interpreter error
Operands must be two numbers or two strings.
[line 1]

Could be replaced by following code to keep errors consistent.

static void runtimeError(RuntimeError error) {
  System.err.println(
    "[line " + error.token.line + "] " + error.getMessage());
  hadRuntimeError = true;
}

This would give the following error messages:

> 1 @ 2           // Scanner error
[line 1] Error: Unexpected character.
> 1 + * 2         // Parser error
[line 1] Error at '*': Expect expression.
> 1 + "hello"    // Interpreter error
[line 1] Operands must be two numbers or two strings.

Also couldn't it be renamed static void error(RuntimeError error) and use report like for the Scanner and Parser?

  static void error(int line, String message) {
    report(line, "", message);
    hadError = true;
  }

  static void error(Token token, String message) {
    if (token.type == TokenType.EOF) {
      report(token.line, " at end", message);
    } else {
      report(token.line, " at '" + token.lexeme + "'", message);
    }
    hadError = true;
  }

  static void error(RuntimeError error) {
    report(error.token.line, "", error.getMessage());
    hadRuntimeError = true;
  }

  private static void report(int line, String where, String message) {
    System.err.println(
      "[line " + line + "] Error" + where + ": " + message);
  }

This would be even more consistent (notice Error: Operands ...):

> 1 @ 2
[line 1] Error: Unexpected character.
> 1 + * 2
[line 1] Error at '*': Expect expression.
> 1 + "hello"
[line 1] Error: Operands must be two numbers or two strings.
> 

Even nicer would be to have an equivalent ScannerError and ParserError to organise things (probably overkill though).

munificent commented 3 years ago

This is deliberate. Eventually, you'll get to a point in the book where runtime errors print stack traces, and at that point, it makes sense for the error message to come before the line information. :)