munificent / craftinginterpreters

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

Locals may have errors in its calculations #1166

Open defpis opened 1 week ago

defpis commented 1 week ago

In the chapter "Resolving and Binding", I found that a function will create two scopes: one for the arguments, and another for the function body.

private void resolveFunction(Stmt.Function function) {
  beginScope(); // Create new scope.
  for (Token param : function.params) {
    declare(param);
    define(param);
  }
  resolve(function.body); // Call visitBlockStmt create new nest scope.
  endScope();
}

// resolve(function.body);
@Override
public Void visitBlockStmt(Stmt.Block stmt) {
  beginScope();
  resolve(stmt.statements);
  endScope();
  return null;
}

... -> scope{ function.params } -> scope{ function.body } -> ...

However, in the interpreter code, a function call will only create one environment.

@Override
public Object call(Interpreter interpreter, List<Object> arguments) {
  Environment environment = new Environment(closure); // Create new environment.

  for (int i = 0; i < declaration.params.size(); i++) {
    environment.define(declaration.params.get(i).lexeme,
        arguments.get(i));
  }

  interpreter.executeBlock(declaration.body, environment); // Just use current environment.
  return null;
}

So, when we calculate the distance of variable bindings in the enclosure and then try to get the value from the environment, it may cause an error.

A simple solution is to iterate over the function body directly in the resolver:

private void resolveFunction(Stmt.Function function) {
  beginScope(); // Create new scope.
  for (Token param : function.params) {
    declare(param);
    define(param);
  }
  // resolve(function.body); // Call visitBlockStmt create new nest scope.
  for (Stmt stmt : function.body.statements) {
    resolve(stmt);
  }
  endScope();
}
defpis commented 1 week ago

I am not sure whether it is this mistake or not. If it is another problem, I hope you can help to point it out, thank you very much.