Handle evaluation of statements, not just expressions.
Also handle statements in conditional breakpoints.
This is more consistent with other common debuggers (e.g. java, python).
Calls Parser#parseStatement with local parsing level, so some statement types aren't handled (e.g. load statements), which is broadly consistent with other debuggers.
Assignment, augmented assignment, and return statements return a non-None value,
and simple expression statements still return the result of evaluating the expression.
TAG_CHANGE_OK=This proto has never yet been used
TYPE_CHANGE_OK=This proto has never yet been used
PiperOrigin-RevId: 202135678
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto b/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto
index a1066c3..cc0de17 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/proto/skylark_debugging.proto
@@ -66,14 +66,14 @@
Stepping stepping = 2;
}
-// A request to evaluate a Skylark expression in a thread's current environment.
+// A request to evaluate a Skylark statement in a thread's current environment.
message EvaluateRequest {
// The identifier of the thread in whose execution context the expression
// should be evaluated.
int64 thread_id = 1;
- // The Skylark expression to evaluate.
- string expression = 2;
+ // The Skylark statement to evaluate.
+ string statement = 2;
}
// A request to list the stack frames of a thread.
@@ -141,7 +141,7 @@
// The response to an EvaluateRequest.
message EvaluateResponse {
- // The result of evaluating an expression.
+ // The result of evaluating a statement.
Value result = 1;
}
@@ -185,7 +185,8 @@
}
// An optional condition for the breakpoint. When present, the breakpoint will
// be triggered iff both the primary condition holds and this expression
- // evaluates to True.
+ // evaluates to True. It is unspecified how many times this expression will be
+ // evaluated, so it should be free of side-effects.
string expression = 2;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
index 93e833d..433f4ef 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/SkylarkDebugServer.java
@@ -201,7 +201,7 @@
long sequenceNumber, SkylarkDebuggingProtos.EvaluateRequest request)
throws DebugRequestException {
return DebugEventHelper.evaluateResponse(
- sequenceNumber, threadHandler.evaluate(request.getThreadId(), request.getExpression()));
+ sequenceNumber, threadHandler.evaluate(request.getThreadId(), request.getStatement()));
}
/** Handles a {@code ContinueExecutionRequest} and returns its response. */
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java
index 585d581..6f92949 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java
@@ -235,7 +235,7 @@
}
}
- SkylarkDebuggingProtos.Value evaluate(long threadId, String expression)
+ SkylarkDebuggingProtos.Value evaluate(long threadId, String statement)
throws DebugRequestException {
Debuggable debuggable;
synchronized (this) {
@@ -250,7 +250,7 @@
// accessed in response to a client request, and requests are handled serially
// TODO(bazel-team): support asynchronous replies, and use finer-grained locks
try {
- Object result = doEvaluate(debuggable, expression);
+ Object result = doEvaluate(debuggable, statement);
return DebuggerSerialization.getValueProto("Evaluation result", result);
} catch (EvalException | InterruptedException e) {
throw new DebugRequestException(e.getMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Debuggable.java b/src/main/java/com/google/devtools/build/lib/syntax/Debuggable.java
index 4540f55..ee871e6 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Debuggable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Debuggable.java
@@ -22,8 +22,8 @@
/** A context in which debugging can occur. Implemented by Skylark environments. */
public interface Debuggable {
- /** Evaluates a Skylark expression in the adapter's environment. */
- Object evaluate(String expression) throws EvalException, InterruptedException;
+ /** Evaluates a Skylark statement in the adapter's environment. */
+ Object evaluate(String statement) throws EvalException, InterruptedException;
/**
* Returns the stack frames corresponding of the context's current (paused) state.
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 5b22906..d5b0d6c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -29,6 +29,7 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.syntax.Mutability.Freezable;
import com.google.devtools.build.lib.syntax.Mutability.MutabilityException;
+import com.google.devtools.build.lib.syntax.Parser.ParsingLevel;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.SpellChecker;
@@ -1169,15 +1170,31 @@
}
@Override
- public Object evaluate(String expression) throws EvalException, InterruptedException {
- ParserInputSource inputSource =
- ParserInputSource.create(expression, PathFragment.create("<debug eval>"));
+ public Object evaluate(String contents) throws EvalException, InterruptedException {
+ ParserInputSource input =
+ ParserInputSource.create(contents, PathFragment.create("<debug eval>"));
EvalEventHandler eventHandler = new EvalEventHandler();
- Expression expr = Parser.parseExpression(inputSource, eventHandler);
+ Statement statement = Parser.parseStatement(input, eventHandler, ParsingLevel.LOCAL_LEVEL);
if (!eventHandler.messages.isEmpty()) {
- throw new EvalException(expr.getLocation(), eventHandler.messages.get(0));
+ throw new EvalException(statement.getLocation(), eventHandler.messages.get(0));
}
- return expr.eval(this);
+ // TODO(bazel-team): move statement handling code to Eval
+ // deal with the most common case first
+ if (statement.kind() == Statement.Kind.EXPRESSION) {
+ return ((ExpressionStatement) statement).getExpression().doEval(this);
+ }
+ // all other statement types are executed directly
+ Eval.fromEnvironment(this).exec(statement);
+ switch (statement.kind()) {
+ case ASSIGNMENT:
+ case AUGMENTED_ASSIGNMENT:
+ return ((AssignmentStatement) statement).getLValue().getExpression().doEval(this);
+ case RETURN:
+ Expression expr = ((ReturnStatement) statement).getReturnExpression();
+ return expr != null ? expr.doEval(this) : Runtime.NONE;
+ default:
+ return Runtime.NONE;
+ }
}
@Override
