From ab1e0418049e2aae94c52ac409c1a6079eefa7b1 Mon Sep 17 00:00:00 2001 From: Matthew Pope <81593196+popematt@users.noreply.github.com> Date: Tue, 8 Oct 2024 10:05:31 -0700 Subject: [PATCH] Add support for reading system e-expressions (#961) --- .../impl/IonReaderContinuableCoreBinary.java | 43 ++++++++-------- ...onReaderContinuableTopLevelBinaryTest.java | 51 +++++++++++++++++++ 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java index ca0297801..7fcf19e4a 100644 --- a/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java +++ b/src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java @@ -24,6 +24,7 @@ import com.amazon.ion.impl.macro.MacroEvaluator; import com.amazon.ion.impl.macro.MacroEvaluatorAsIonReader; import com.amazon.ion.impl.macro.MacroRef; +import com.amazon.ion.impl.macro.SystemMacro; import java.io.InputStream; import java.math.BigDecimal; @@ -112,11 +113,11 @@ class IonReaderContinuableCoreBinary extends IonCursorBinary implements IonReade // The symbol IDs for the annotations on the current value. private final IntList annotationSids; - // The IonReader-like MacroEvaluator that this core reader delegates to when evaluating a macro invocation. - protected MacroEvaluatorAsIonReader macroEvaluatorIonReader = null; - // The core MacroEvaluator that this core reader delegates to when evaluating a macro invocation. - private MacroEvaluator macroEvaluator = null; + private final MacroEvaluator macroEvaluator = new MacroEvaluator(); + + // The IonReader-like MacroEvaluator that this core reader delegates to when evaluating a macro invocation. + protected MacroEvaluatorAsIonReader macroEvaluatorIonReader = new MacroEvaluatorAsIonReader(macroEvaluator); // The encoding context (macro table) that is currently active. private EncodingContext encodingContext = null; @@ -1225,8 +1226,6 @@ private void stepOutOfSexpWithinEncodingDirective() { */ private void installMacros() { encodingContext = new EncodingContext(newMacros); - macroEvaluator = new MacroEvaluator(); - macroEvaluatorIonReader = new MacroEvaluatorAsIonReader(macroEvaluator); } /** @@ -1529,9 +1528,6 @@ private void readContainerValueAsExpression( */ private void readValueAsExpression(List expressions) { if (valueTid.isMacroInvocation) { - if (isSystemInvocation()) { - throw new UnsupportedOperationException("TODO: system macros"); - } collectEExpressionArgs(expressions); // TODO avoid recursion return; } @@ -1664,17 +1660,24 @@ private void readParameter(Macro.Parameter parameter, long parameterPresence, Li * @param expressions receives the expressions as they are materialized. */ private void collectEExpressionArgs(List expressions) { - if (isSystemInvocation()) { - throw new UnsupportedOperationException("System macro invocations not yet supported."); - } + Macro macro; long id = getMacroInvocationId(); - if (id > Integer.MAX_VALUE) { - throw new IonException("Macro addresses larger than 2147483647 are not supported by this implementation."); - } - MacroRef address = MacroRef.byId((int) id); - Macro macro = encodingContext.getMacroTable().get(address); - if (macro == null) { - throw new IonException(String.format("Encountered an unknown macro address: %d.", id)); + if (isSystemInvocation()) { + macro = SystemMacro.get((int) id); + if (macro == null) { + throw new UnsupportedOperationException("System macro " + id + " not yet supported."); + } + } else { + if (id > Integer.MAX_VALUE) { + throw new IonException("Macro addresses larger than 2147483647 are not supported by this implementation."); + } + MacroRef address = MacroRef.byId((int) id); + Map macroTable = encodingContext.getMacroTable(); + macro = macroTable.get(address); + + if (macro == null) { + throw new IonException(String.format("Encountered an unknown macro address: %d.", id)); + } } PresenceBitmap presenceBitmap = new PresenceBitmap(); List signature = macro.getSignature(); @@ -1768,7 +1771,7 @@ public Event nextValue() { event = super.nextValue(); } if (valueTid != null && valueTid.isMacroInvocation) { - if (macroEvaluator == null) { + 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 diff --git a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java index ed62b4c30..f248aca43 100644 --- a/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java +++ b/src/test/java/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java @@ -5934,5 +5934,56 @@ public void prefixedAnnotatedContainerInsideDelimitedAnnotatedContainerPreserves closeAndCount(); } + + @ParameterizedTest(name = "[{index}] {0}") + @CsvSource({ + // (:values 0) (:values 1) + "EF 01 01 60 EF 01 01 61 01", + // (:values (: values 0)) 1 + "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", + }) + public void invokeValuesUsingSystemMacroOpcode(String bytes) throws Exception { + invokeValuesUsingSystemMacroOpcodeHelper(true, bytes); + invokeValuesUsingSystemMacroOpcodeHelper(false, bytes); + } + + @ParameterizedTest(name = "[{index}] {0}") + @CsvSource({ + // (:values (:: ) ) 0 1 + "EF 01 02 01 F0 60 61 01", + // (:values 0 1) // using length-prefixed expression group + "EF 01 02 07 60 61 01", + // (:values 0 1) // using delimited expression group + "EF 01 02 01 60 61 01 F0", + // (:values (:: 0) (:values (:: 1)) + "EF 01 02 03 60 EF 01 02 05 61 01", + // (:values (:values 0 1)) + "EF 01 01 EF 01 02 07 60 61 01", + }) + public void invokeValuesWithExpressionGroupsUsingSystemMacroOpcode(String bytes) throws Exception { + invokeValuesUsingSystemMacroOpcodeHelper(true, bytes); + // TODO: Determine why the checkpoint in the cursor isn't being set correctly before calling slowFillValue() + // specifically while reading from a length-prefixed expression group + // See https://github.com/amazon-ion/ion-java/issues/963 + // invokeValuesUsingSystemMacroOpcodeHelper(false, bytes); + } + + private void invokeValuesUsingSystemMacroOpcodeHelper(boolean constructFromBytes, String bytes) throws Exception { + byte[] input = withIvm(1, hexStringToByteArray(cleanCommentedHexBytes(bytes))); + readerBuilder = readerBuilder.withIncrementalReadingEnabled(false); + reader = readerFor(readerBuilder, constructFromBytes, input); + assertSequence( + next(IonType.INT), intValue(0), + next(IonType.INT), intValue(1), + next(null) + ); + } + // TODO Ion 1.1 symbol tables with all kinds of annotation encodings (opcodes E4 - E9, inline and SID) }