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

3 files changed, 9 insertions, 8 deletions

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 a1066c3e7f..cc0de17909 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 @@ message ContinueExecutionRequest {
   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 @@ message ContinueExecutionResponse {
 
 // 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 @@ message Breakpoint {
   }
   // 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 93e833db03..433f4ef4cc 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 @@ public final class SkylarkDebugServer implements DebugServer {
       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 585d581ed2..6f92949e74 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 @@ final class ThreadHandler {
     }
   }
 
-  SkylarkDebuggingProtos.Value evaluate(long threadId, String expression)
+  SkylarkDebuggingProtos.Value evaluate(long threadId, String statement)
       throws DebugRequestException {
     Debuggable debuggable;
     synchronized (this) {
@@ -250,7 +250,7 @@ final class ThreadHandler {
     // 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());