Skip to content

Commit

Permalink
Fixes a bug that caused the binary reader to stop consuming from the …
Browse files Browse the repository at this point in the history
…stream after evaluating an e-expression that produced no values.
  • Loading branch information
tgregg committed Oct 11, 2024
1 parent 86c2bce commit f7c5a43
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1606,17 +1606,17 @@ protected void stepOutOfEExpression() {
}

/**
* Consumes the next value (if any) from the MacroEvaluator, setting `event` based on the result. If this call
* causes the evaluator to reach the end of the current invocation, positions the reader on the value that follows
* the invocation.
* Consumes the next value (if any) from the MacroEvaluator, setting `event` based on the result.
* @return true if evaluation of the current invocation has completed; otherwise, false.
*/
private void evaluateNext() {
private boolean evaluateNext() {
IonType type = macroEvaluatorIonReader.next();
if (type == null) {
if (macroEvaluatorIonReader.getDepth() == 0) {
// Evaluation of this macro is complete. Resume reading from the stream.
isEvaluatingEExpression = false;
event = super.nextValue();
event = Event.NEEDS_INSTRUCTION;
return true;
} else {
event = Event.END_CONTAINER;
}
Expand All @@ -1627,13 +1627,14 @@ private void evaluateNext() {
event = Event.START_SCALAR;
}
}
return false;
}

@Override
public Event nextValue() {
lobBytesRead = 0;
if (parent == null || state != State.READING_VALUE) {
while (true) {
while (true) {
if (parent == null || state != State.READING_VALUE) {
if (state != State.READING_VALUE && state != State.COMPILING_MACRO) {
encodingDirectiveReader.readEncodingDirective();
if (state != State.READING_VALUE) {
Expand All @@ -1642,7 +1643,9 @@ public Event nextValue() {
}
}
if (isEvaluatingEExpression) {
evaluateNext();
if (evaluateNext()) {
continue;
}
} else {
event = super.nextValue();
}
Expand All @@ -1651,27 +1654,31 @@ public Event nextValue() {
state = State.ON_ION_ENCODING_SEXP;
continue;
}
break;
}
} else if (isEvaluatingEExpression) {
evaluateNext();
} else {
event = super.nextValue();
}
if (valueTid != null && valueTid.isMacroInvocation) {
if (encodingContext == null && !isSystemInvocation()) {
// If the macro evaluator is null, it means there is no active macro table. Do not attempt evaluation,
// but allow the user to do a raw read of the parameters if this is a core-level reader.
// TODO this is used in the tests for the core binary reader. If it cannot be useful elsewhere, remove
// and refactor the tests.
if (this instanceof IonReaderContinuableApplicationBinary) {
throw new IonException("The user-level binary reader encountered a macro invocation without an active macro table.");
} else if (isEvaluatingEExpression) {
if (evaluateNext()) {
continue;
}
} else {
expressionArgsReader.beginEvaluatingMacroInvocation(macroEvaluator);
isEvaluatingEExpression = true;
evaluateNext();
event = super.nextValue();
}
if (valueTid != null && valueTid.isMacroInvocation) {
if (encodingContext == null && !isSystemInvocation()) {
// If the macro evaluator is null, it means there is no active macro table. Do not attempt evaluation,
// but allow the user to do a raw read of the parameters if this is a core-level reader.
// TODO this is used in the tests for the core binary reader. If it cannot be useful elsewhere, remove
// and refactor the tests.
if (this instanceof IonReaderContinuableApplicationBinary) {
throw new IonException("The user-level binary reader encountered a macro invocation without an active macro table.");
}
} else {
expressionArgsReader.beginEvaluatingMacroInvocation(macroEvaluator);
isEvaluatingEExpression = true;
if (evaluateNext()) {
continue;
}
}
}
break;
}
return event;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5943,10 +5943,10 @@ public void prefixedAnnotatedContainerInsideDelimitedAnnotatedContainerPreserves
"EF 01 01 EF 01 01 60 61 01",
// (:values) 0 (:values) 1
"EF 01 00 60 EF 01 00 61 01",
// TODO: Determine why the state isn't being properly set after invoking a macro that produces nothing
// See https://github.com/amazon-ion/ion-java/issues/962
// (:values) (:values 0) 1
// "EF 01 00 EF 01 01 60 61 01",
"EF 01 00 EF 01 01 60 61 01",
// (:values) (:values) (:values 0) 1
"EF 01 00 EF 01 00 EF 01 01 60 61 01",
})
public void invokeValuesUsingSystemMacroOpcode(String bytes) throws Exception {
invokeValuesUsingSystemMacroOpcodeHelper(true, bytes);
Expand Down

0 comments on commit f7c5a43

Please sign in to comment.