Skip to content

Commit

Permalink
Add static name resolution
Browse files Browse the repository at this point in the history
    This implements a part of the name resolution document: https://github.com/bazelbuild/proposals/blob/master/docs/2018-06-18-name-resolution.md
    and it is enabled with the flag --incompatible_static_name_resolution

    There are two visible changes:

    1. Local variables can be used before their definition point.
    2. Local variables will shadow global variables, even if they are not initialiazed yet.

    bazelbuild/bazel#5637

    RELNOTES: None.
    PiperOrigin-RevId: 210562752
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 52de5d8 commit 17c681d
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1074,38 +1074,16 @@ public Environment setupOverride(String varname, Object value) {
}

/**
* Returns the value of a variable defined in Local scope. Do not search in any parent scope. This
* function should be used once the AST has been analysed and we know which variables are local.
* Returns the value of a variable defined in the current lexical frame. Do not search in any
* parent scope. This function should be used once the AST has been analysed and we know which
* variables are local.
*/
public Object localLookup(String varname) {
return lexicalFrame.get(varname);
}

/**
* Returns the value of a variable defined in the Module scope (e.g. global variables,
* functions).
*/
public Object moduleLookup(String varname) {
return globalFrame.get(varname);
}

/** Returns the value of a variable defined in the Universe scope (builtins). */
public Object universeLookup(String varname) {
// TODO(laurentlb): We should distinguish between Module and Universe. Values in Module can
// shadow those in Universe.
return globalFrame.get(varname);
}

/** Returns the value of a variable defined with setupDynamic. */
public Object dynamicLookup(String varname) {
return dynamicFrame.get(varname);
}

/**
* Returns the value from the environment whose name is "varname" if it exists, otherwise null.
*
* <p>TODO(laurentlb): Remove this method. Callers should know where the value is defined and use
* the corresponding method (e.g. localLookup or moduleLookup).
*/
public Object lookup(String varname) {
// Lexical frame takes precedence, then globals, then dynamics.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,39 +80,12 @@ void setScope(ValidationEnvironment.Scope scope) {

@Override
Object doEval(Environment env) throws EvalException {
Object result;
if (scope == null) {
// Legacy behavior, in case the AST was not analyzed.
result = env.lookup(name);
if (result == null) {
throw createInvalidIdentifierException(env.getVariableNames());
}
return result;
Object value =
scope == ValidationEnvironment.Scope.Local ? env.localLookup(name) : env.lookup(name);
if (value == null) {
throw createInvalidIdentifierException(env.getVariableNames());
}

switch (scope) {
case Local:
result = env.localLookup(name);
break;
case Module:
result = env.moduleLookup(name);
break;
case Universe:
result = env.universeLookup(name);
break;
default:
throw new IllegalStateException(scope.toString());
}

if (result == null) {
// Since Scope was set, we know that the variable is defined in the scope.
// However, the assignment was not yet executed.
throw new EvalException(
getLocation(),
scope.getQualifier() + " variable '" + name + "' is referenced before assignment.");
}

return result;
return value;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public Object call(Object[] arguments, FuncallExpression ast, Environment env)

ImmutableList<String> names = signature.getSignature().getNames();
LexicalFrame lexicalFrame = LexicalFrame.create(env.mutability(), /*numArgs=*/ names.size());
try (SilentCloseable c =
Profiler.instance().profile(ProfilerTask.STARLARK_USER_FN, getName())) {
try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.SKYLARK_USER_FN, getName())) {
env.enterScope(this, lexicalFrame, ast, definitionGlobals);

// Registering the functions's arguments as variables in the local Environment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,11 @@ public final class ValidationEnvironment extends SyntaxTreeVisitor {

enum Scope {
/** Symbols defined inside a function or a comprehension. */
Local("local"),
Local,
/** Symbols defined at a module top-level, e.g. functions, loaded symbols. */
Module("global"),
Module,
/** Predefined symbols (builtins) */
Universe("builtin");

private final String qualifier;

private Scope(String qualifier) {
this.qualifier = qualifier;
}

public String getQualifier() {
return qualifier;
}
Universe,
}

private static class Block {
Expand Down Expand Up @@ -182,10 +172,7 @@ public void visit(Identifier node) {
if (b == null) {
throw new ValidationException(node.createInvalidIdentifierException(getAllSymbols()));
}
if (semantics.incompatibleStaticNameResolution()) {
// The scoping information is reliable only with the new behavior.
node.setScope(b.scope);
}
node.setScope(b.scope);
}

private void validateLValue(Location loc, Expression expr) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,9 @@ public void testVariableIsReferencedBeforeAssignment() throws Exception {
} catch (EvalExceptionWithStackTrace e) {
assertThat(e)
.hasMessageThat()
.contains("Variable 'global_var' is referenced before assignment.");
.contains(
"Variable 'global_var' is referenced before assignment. "
+ "The variable is defined in the global scope.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ public void testLocalVariableDefinedBelow() throws Exception {
public void testShadowisNotInitialized() throws Exception {
new SkylarkTest("--incompatible_static_name_resolution=true")
.testIfErrorContains(
/* error message */ "local variable 'gl' is referenced before assignment",
/* error message */ "name 'gl' is not defined",
"gl = 5",
"def foo():", // returns the value before the first even number
" if False: gl = 2",
Expand Down

0 comments on commit 17c681d

Please sign in to comment.