Skip to content

Commit

Permalink
Cleanups for Skylark tracebacks
Browse files Browse the repository at this point in the history
In Skylark.java, fix line numbers to start at 1 instead of 2.

In Eval.java, go through maybeTransformException for consistency with expressions, even though no statement nodes use this feature.

In EvalExceptionWithStackTrace, fix style violation (non-consecutive overloads) and add TODO explaining an open issue.

RELNOTES: None
PiperOrigin-RevId: 169769928
  • Loading branch information
brandjon authored and katre committed Sep 25, 2017
1 parent fefccdb commit 95b0467
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 10 deletions.
8 changes: 8 additions & 0 deletions src/main/java/com/google/devtools/build/lib/syntax/Eval.java
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,14 @@ void execReturn(ReturnStatement node) throws EvalException, InterruptedException
* @throws InterruptedException may be thrown in a sub class.
*/
public void exec(Statement st) throws EvalException, InterruptedException {
try {
execDispatch(st);
} catch (EvalException ex) {
throw st.maybeTransformException(ex);
}
}

void execDispatch(Statement st) throws EvalException, InterruptedException {
switch (st.kind()) {
case ASSIGNMENT:
execAssignment((AssignmentStatement) st);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,24 @@ public void registerPhantomFuncall(
addStackFrame(funcallDescription, location, false);
}

/**
* Adds a line for the given frame.
*/
/** Adds a line for the given frame. */
private void addStackFrame(String label, Location location, boolean canPrint) {
// We have to watch out for duplicate since ExpressionStatements add themselves twice:
// Statement#exec() calls Expression#eval(), both of which call this method.
// TODO(bazel-team): This check was originally created to weed out duplicates in case the same
// node is added twice, but it's not clear if that is still a possibility. In any case, it would
// be better to eliminate the check and not create unwanted duplicates in the first place.
//
// The check is problematic because it suppresses tracebacks in the REPL, where line numbers
// can be reset within a single session.
if (mostRecentElement != null && isSameLocation(location, mostRecentElement.getLocation())) {
return;
}
mostRecentElement = new StackFrame(label, location, mostRecentElement, canPrint);
}

private void addStackFrame(String label, Location location) {
addStackFrame(label, location, true);
}

/**
* Checks two locations for equality in paths and start offsets.
*
Expand All @@ -136,10 +142,6 @@ private boolean isSameLocation(Location first, Location second) {
}
}

private void addStackFrame(String label, Location location) {
addStackFrame(label, location, true);
}

/**
* Returns the exception message without the stack trace.
*/
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/google/devtools/skylark/Skylark.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ private String prompt() {
StringBuilder input = new StringBuilder();
System.out.print(START_PROMPT);
try {
String lineSeparator = "";
while (true) {
String line = reader.readLine();
if (line == null) {
Expand All @@ -72,7 +73,8 @@ private String prompt() {
if (line.isEmpty()) {
return input.toString();
}
input.append("\n").append(line);
input.append(lineSeparator).append(line);
lineSeparator = "\n";
System.out.print(CONTINUATION_PROMPT);
}
} catch (IOException io) {
Expand Down

0 comments on commit 95b0467

Please sign in to comment.