From 92560f5dfd6e37983b2324e8f5a6258d1338ddcd Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Mon, 16 Dec 2024 16:29:48 -0800 Subject: [PATCH] Even moar cleanup --- .../ion/impl/bin/IonManagedWriter_1_1.kt | 2 +- .../com/amazon/ion/impl/macro/Environment.kt | 2 +- .../amazon/ion/impl/macro/MacroEvaluator.kt | 629 +++++++++--------- .../com/amazon/ion/impl/macro/SystemMacro.kt | 14 +- .../ion/impl/IonRawTextWriterTest_1_1.kt | 6 +- 5 files changed, 349 insertions(+), 304 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt b/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt index bd6bfcbad..7848944fc 100644 --- a/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt +++ b/src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt @@ -628,7 +628,7 @@ internal class IonManagedWriter_1_1( is Expression.MacroInvocation -> { val invokedMacro = expression.macro if (invokedMacro is SystemMacro) { - stepInTdlSystemMacroInvocation(invokedMacro.systemSymbol!!) + stepInTdlSystemMacroInvocation(invokedMacro.systemSymbol) } else { val invokedAddress = macroTable[invokedMacro] ?: newMacros[invokedMacro] diff --git a/src/main/java/com/amazon/ion/impl/macro/Environment.kt b/src/main/java/com/amazon/ion/impl/macro/Environment.kt index 18c854c89..d348612a1 100644 --- a/src/main/java/com/amazon/ion/impl/macro/Environment.kt +++ b/src/main/java/com/amazon/ion/impl/macro/Environment.kt @@ -29,7 +29,7 @@ data class Environment private constructor( | ], | parent: ${parentEnvironment.toString().lines().joinToString("\n| ")}, |) - """.trimMargin() + """.trimMargin() companion object { @JvmStatic diff --git a/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt b/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt index b3ccfc5fc..af3ecf589 100644 --- a/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt +++ b/src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt @@ -6,7 +6,8 @@ import com.amazon.ion.* import com.amazon.ion.impl._Private_RecyclingStack import com.amazon.ion.impl._Private_Utils.* import com.amazon.ion.impl.macro.Expression.* -import com.amazon.ion.impl.macro.MacroEvaluator.ExpanderKind.* +import com.amazon.ion.impl.macro.MacroEvaluator.ExpanderKind.Stream +import com.amazon.ion.impl.macro.MacroEvaluator.ExpanderKind.Uninitialized import com.amazon.ion.util.* import java.io.ByteArrayOutputStream import java.lang.StringBuilder @@ -23,24 +24,49 @@ import java.math.BigInteger * if the end of the container or end of expansion has been reached. * - Call [stepIn] when positioned on a container to step into that container. * - Call [stepOut] to step out of the current container. + * + * TODO: Make expansion limit configurable. + * + * ### Implementation Overview: + * + * The macro evaluator can be thought of as a stack of containers where each container has a stack of expansion frames + * (i.e. [ExpansionInfo]). To get the next value at the current depth, the macro evaluator starts with the bottom frame + * of the top container. Expansion frames may delegate to and/or intercept other expansions that are further up the stack. + * + * One might visualize it like this: + * ``` + * 3. List : Stream --> Delta --> Variable + * 2. List : Stream --> Flatten --> Stream + * 1. Struct : Stream --> Variable --> TemplateBody --> Stream --> TemplateBody + * 0. TopLevel : Stream --> TemplateBody --> TemplateBody + * ``` + * + * When calling [expandNext], the evaluator looks at the first expansion frame of the top container in the stack. + * Then it calls `produceNext` for the first expansion in that container. That expansion may produce a result all on its + * own, or it may call the next expansion in the chain and return that value (with or without further modification). + * + * In practice, it is a little more complex. A single expansion frame may hold more than one child frame sequentially + * (e.g. `repeat`, `annotate`) or concurrently (e.g. `for`, `flatten`). */ class MacroEvaluator { - private var numExpandedExpressions = 0 - private val expansionLimit: Int = 1_000_000 - private val expanderPool: ArrayList = ArrayList(32) - - // TODO(PERF): Does it improve performance if we make this an `inner` class and remove the evaluator field? - class MacroEvaluationSession(private val evaluator: MacroEvaluator) { - - fun getExpander( - expanderKind: ExpanderKind, - expressions: List, - startInclusive: Int, - endExclusive: Int, - environment: Environment, - ): ExpansionFrame { - val expansion = evaluator.expanderPool.removeLastOrNull() ?: ExpansionFrame(this) + /** + * Holds state that is shared across all macro evaluations that are part of this evaluator. + * This state pertains to a single "session" of the macro evaluator, and is reset every time [initExpansion] is called. + * For now, this includes managing the pool of [ExpansionInfo] and tracking the expansion step limit. + */ + private class Session( + /** Number of expansion steps at which the evaluation session should be aborted. */ + private val expansionLimit: Int = 1_000_000 + ) { + /** Internal state for tracking the number of expansion steps. */ + private var numExpandedExpressions = 0 + /** Pool of [ExpansionInfo] to minimize allocation and garbage collection. */ + private val expanderPool: ArrayList = ArrayList(32) + + /** Gets an [ExpansionInfo] from the pool (or allocates a new one if necessary), initializing it with the provided values. */ + fun getExpander(expanderKind: ExpanderKind, expressions: List, startInclusive: Int, endExclusive: Int, environment: Environment): ExpansionInfo { + val expansion = expanderPool.removeLastOrNull() ?: ExpansionInfo(this) expansion.expanderKind = expanderKind expansion.expressions = expressions expansion.i = startInclusive @@ -51,32 +77,40 @@ class MacroEvaluator { return expansion } - fun returnExpander(ex: ExpansionFrame) { - // TODO: This check is O(n). Remove this when confident. - check(ex !in evaluator.expanderPool) - evaluator.expanderPool.add(ex) + /** Reclaims an [ExpansionInfo] to the available pool. */ + fun reclaimExpander(ex: ExpansionInfo) { + // TODO: This check is O(n). Consider removing this when confident there are no double frees. + check(ex !in expanderPool) + expanderPool.add(ex) } fun incrementStepCounter() { - evaluator.numExpandedExpressions++ - if (evaluator.numExpandedExpressions > evaluator.expansionLimit) { + numExpandedExpressions++ + if (numExpandedExpressions > expansionLimit) { // Technically, we are not counting "steps" because we don't have a true definition of what a "step" is, // but this is probably a more user-friendly message than trying to explain what we're actually counting. - throw IonException("Macro expansion exceeded limit of ${evaluator.expansionLimit} steps.") + throw IonException("Macro expansion exceeded limit of $expansionLimit steps.") } } + + fun reset() { + numExpandedExpressions = 0 + } } - private data class ContainerInfo(var type: Type = Type.Uninitialized, private var _expansion: ExpansionFrame? = null) { + /** + * A container in the macro evaluator's [containerStack]. + */ + private data class ContainerInfo(var type: Type = Type.Uninitialized, private var _expansion: ExpansionInfo? = null) { enum class Type { TopLevel, List, Sexp, Struct, Uninitialized } - fun releaseResources() { - _expansion?.drop() + fun close() { + _expansion?.close() _expansion = null type = Type.Uninitialized } - var expansion: ExpansionFrame + var expansion: ExpansionInfo get() = _expansion!! set(value) { _expansion = value } @@ -85,122 +119,20 @@ class MacroEvaluator { } } - private val session = MacroEvaluationSession(this) - private val containerStack = _Private_RecyclingStack(8) { ContainerInfo() } - private var currentExpr: DataModelExpression? = null - - private fun resetSession() { this.numExpandedExpressions = 0 } - - fun getArguments(): List { - return containerStack.iterator().next().expansion.expressions - } - /** - * Initialize the macro evaluator with an E-Expression. + * Stateless functions that operate on the expansion frames (i.e. [ExpansionInfo]). */ - fun initExpansion(encodingExpressions: List) { - resetSession() - containerStack.push { ci -> - ci.type = ContainerInfo.Type.TopLevel - ci.expansion = session.getExpander(Stream, encodingExpressions, 0, encodingExpressions.size, Environment.EMPTY) - } - } - - /** - * Evaluate the macro expansion until the next [DataModelExpression] can be returned. - * Returns null if at the end of a container or at the end of the expansion. - */ - fun expandNext(): DataModelExpression? { - currentExpr = null - while (currentExpr == null && !containerStack.isEmpty()) { - val currentContainer = containerStack.peek() - - when (val nextExpansionOutput = currentContainer.produceNext()) { - is DataModelExpression -> currentExpr = nextExpansionOutput - EndOfExpansion -> { - if (currentContainer.type == ContainerInfo.Type.TopLevel) { - currentContainer.releaseResources() - containerStack.pop() - } - return null - } - } - } - return currentExpr - } - - /** - * Steps out of the current [DataModelContainer]. - */ - fun stepOut() { - // TODO: We should be able to step out of a "TopLevel" container and/or we need some way to close the evaluation. - if (containerStack.size() <= 1) throw IonException("Nothing to step out of.") - val popped = containerStack.pop() - popped.releaseResources() - } - - /** - * Steps in to the current [DataModelContainer]. - * Throws [IonException] if not positioned on a container. - */ - fun stepIn() { - val expression = requireNotNull(currentExpr) { "Not positioned on a value" } - if (expression is DataModelContainer) { - val currentContainer = containerStack.peek() - val topExpansion = currentContainer.expansion.top() - containerStack.push { ci -> - ci.type = when (expression.type) { - IonType.LIST -> ContainerInfo.Type.List - IonType.SEXP -> ContainerInfo.Type.Sexp - IonType.STRUCT -> ContainerInfo.Type.Struct - else -> unreachable() - } - ci.expansion = session.getExpander( - expanderKind = Stream, - expressions = topExpansion.expressions, - startInclusive = expression.startInclusive, - endExclusive = expression.endExclusive, - environment = topExpansion.environment, - ) - } - currentExpr = null - } else { - throw IonException("Not positioned on a container.") - } - } - // TODO(PERF): It might be possible to optimize this by changing it to an enum without any methods (or even a set of // integer constants) and converting all their implementations to static methods. - enum class ExpanderKind { + private enum class ExpanderKind { Uninitialized { - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - throw IllegalStateException("ExpansionInfo not initialized.") - } + override fun produceNext(thisExpansion: ExpansionInfo): Nothing = throw IllegalStateException("ExpansionInfo not initialized.") }, Empty { - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue = EndOfExpansion + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue = EndOfExpansion }, Stream { - private fun invokeMacro(thisExpansion: ExpansionFrame, macro: Macro, next: HasStartAndEnd): ContinueExpansion { - val argIndices = macro.calculateArgumentIndices( - encodingExpressions = thisExpansion.expressions, - argsStartInclusive = next.startInclusive, - argsEndExclusive = next.endExclusive, - ) - val newEnvironment = thisExpansion.environment.createChild(thisExpansion.expressions, argIndices) - val expanderKind = if (macro.body != null) Stream else getExpanderKindForSystemMacro(macro as SystemMacro) - thisExpansion.expansionDelegate = thisExpansion.session.getExpander( - expanderKind = expanderKind, - expressions = macro.body ?: emptyList(), - startInclusive = 0, - endExclusive = macro.body?.size ?: 0, - environment = newEnvironment, - ) - - return ContinueExpansion - } - - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { // If there's a delegate, we'll try that first. val delegate = thisExpansion.expansionDelegate check(thisExpansion != delegate) @@ -208,7 +140,7 @@ class MacroEvaluator { return when (val result = delegate.produceNext()) { is DataModelExpression -> result EndOfExpansion -> { - delegate.drop() + delegate.close() thisExpansion.expansionDelegate = null ContinueExpansion } @@ -226,11 +158,23 @@ class MacroEvaluator { return when (next) { is DataModelExpression -> next - is EExpression -> invokeMacro(thisExpansion, next.macro, next) - is MacroInvocation -> invokeMacro(thisExpansion, next.macro, next) + is InvokableExpression -> { + val macro = next.macro + val argIndices = calculateArgumentIndices(macro, thisExpansion.expressions, next.startInclusive, next.endExclusive) + val newEnvironment = thisExpansion.environment.createChild(thisExpansion.expressions, argIndices) + val expanderKind = ExpanderKind.forMacro(macro) + thisExpansion.expansionDelegate = thisExpansion.session.getExpander( + expanderKind = expanderKind, + expressions = macro.body ?: emptyList(), + startInclusive = 0, + endExclusive = macro.body?.size ?: 0, + environment = newEnvironment, + ) + ContinueExpansion + } is ExpressionGroup -> { thisExpansion.expansionDelegate = thisExpansion.session.getExpander( - expanderKind = Stream, + expanderKind = ExprGroup, expressions = thisExpansion.expressions, startInclusive = next.startInclusive, endExclusive = next.endExclusive, @@ -248,41 +192,26 @@ class MacroEvaluator { } } }, - // TODO: Move this into the variable expansion? - ExactlyOneValueStream { - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - if (thisExpansion.additionalState != 1) { - return when (val firstValue = Stream.produceNext(thisExpansion)) { - is DataModelExpression -> { - thisExpansion.additionalState = 1 - firstValue - } - ContinueExpansion -> ContinueExpansion - EndOfExpansion -> throw IonException("Expected one value, found 0") - } - } else { - return when (val secondValue = Stream.produceNext(thisExpansion)) { - is DataModelExpression -> throw IonException("Expected one value, found multiple") - ContinueExpansion -> ContinueExpansion - EndOfExpansion -> secondValue - } - } + /** Alias of [Stream] to aid in debugging */ + Variable { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + return Stream.produceNext(thisExpansion) } }, - NonEmptyStream { - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - return when (val firstValue = Stream.produceNext(thisExpansion)) { - EndOfExpansion -> throw IonException("Expected at least one value, found 0") - ContinueExpansion -> ContinueExpansion - is DataModelExpression -> { - thisExpansion.expanderKind = Stream - firstValue - } - } + /** Alias of [Stream] to aid in debugging */ + TemplateBody { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + return Stream.produceNext(thisExpansion) } }, - AtMostOneValueStream { - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + /** Alias of [Stream] to aid in debugging */ + ExprGroup { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + return Stream.produceNext(thisExpansion) + } + }, + ExactlyOneValueStream { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { if (thisExpansion.additionalState != 1) { return when (val firstValue = Stream.produceNext(thisExpansion)) { is DataModelExpression -> { @@ -290,7 +219,7 @@ class MacroEvaluator { firstValue } ContinueExpansion -> ContinueExpansion - EndOfExpansion -> EndOfExpansion + EndOfExpansion -> throw IonException("Expected one value, found 0") } } else { return when (val secondValue = Stream.produceNext(thisExpansion)) { @@ -303,23 +232,23 @@ class MacroEvaluator { }, IfNone { - override fun produceNext(thisExpansion: ExpansionFrame) = checkExpansionSize(thisExpansion) { it == 0 } + override fun produceNext(thisExpansion: ExpansionInfo) = thisExpansion.branchIf { it == 0 } }, IfSome { - override fun produceNext(thisExpansion: ExpansionFrame) = checkExpansionSize(thisExpansion) { it > 0 } + override fun produceNext(thisExpansion: ExpansionInfo) = thisExpansion.branchIf { it > 0 } }, IfSingle { - override fun produceNext(thisExpansion: ExpansionFrame) = checkExpansionSize(thisExpansion) { it == 1 } + override fun produceNext(thisExpansion: ExpansionInfo) = thisExpansion.branchIf { it == 1 } }, IfMulti { - override fun produceNext(thisExpansion: ExpansionFrame) = checkExpansionSize(thisExpansion) { it > 1 } + override fun produceNext(thisExpansion: ExpansionInfo) = thisExpansion.branchIf { it > 1 } }, Annotate { private val ANNOTATIONS_ARG = VariableRef(0) private val VALUE_TO_ANNOTATE_ARG = VariableRef(1) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { val annotations = thisExpansion.map(ANNOTATIONS_ARG) { when (it) { is StringValue -> newSymbolToken(it.value) @@ -335,17 +264,19 @@ class MacroEvaluator { it as? DataModelValue ?: throw IonException("Required at least one value.") it.withAnnotations(annotations + it.annotations) } + if (valueToAnnotateExpansion.produceNext() != EndOfExpansion) { + throw IonException("Can only annotate exactly one value") + } return annotatedExpression.also { thisExpansion.tailCall(valueToAnnotateExpansion) - thisExpansion.expanderKind = ExactlyOneValueStream } } }, MakeString { private val STRINGS_ARG = VariableRef(0) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { val sb = StringBuilder() thisExpansion.forEach(STRINGS_ARG) { when (it) { @@ -362,7 +293,7 @@ class MacroEvaluator { MakeSymbol { private val STRINGS_ARG = VariableRef(0) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { if (thisExpansion.additionalState != null) return EndOfExpansion thisExpansion.additionalState = Unit @@ -381,10 +312,7 @@ class MacroEvaluator { MakeBlob { private val LOB_ARG = VariableRef(0) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - if (thisExpansion.additionalState != null) return EndOfExpansion - thisExpansion.additionalState = Unit - + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { val baos = ByteArrayOutputStream() thisExpansion.forEach(LOB_ARG) { when (it) { @@ -393,6 +321,7 @@ class MacroEvaluator { is FieldName -> unreachable() } } + thisExpansion.expanderKind = Empty return BlobValue(value = baos.toByteArray()) } }, @@ -400,12 +329,10 @@ class MacroEvaluator { private val COEFFICIENT_ARG = VariableRef(0) private val EXPONENT_ARG = VariableRef(1) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - if (thisExpansion.additionalState != null) return EndOfExpansion - thisExpansion.additionalState = Unit - + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { val coefficient = thisExpansion.readExactlyOneArgument(COEFFICIENT_ARG).bigIntegerValue val exponent = thisExpansion.readExactlyOneArgument(EXPONENT_ARG).bigIntegerValue + thisExpansion.expanderKind = Empty return DecimalValue(value = BigDecimal(coefficient, -1 * exponent.intValueExact())) } }, @@ -418,7 +345,7 @@ class MacroEvaluator { private val SECOND_ARG = VariableRef(5) private val OFFSET_ARG = VariableRef(6) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { val year = thisExpansion.readExactlyOneArgument(YEAR_ARG).longValue.toInt() val month = thisExpansion.readZeroOrOneArgument(MONTH_ARG)?.longValue?.toInt() val day = thisExpansion.readZeroOrOneArgument(DAY_ARG)?.longValue?.toInt() @@ -470,7 +397,7 @@ class MacroEvaluator { private val FIELD_NAME = VariableRef(0) private val FIELD_VALUE = VariableRef(1) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { val fieldName = thisExpansion.readExactlyOneArgument(FIELD_NAME) val fieldNameExpression = when (fieldName) { is SymbolValue -> FieldName(fieldName.value) @@ -491,8 +418,8 @@ class MacroEvaluator { _Private_FlattenStruct { private val STRUCTS = VariableRef(0) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - var argumentExpansion: ExpansionFrame? = thisExpansion.additionalState as ExpansionFrame? + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + var argumentExpansion: ExpansionInfo? = thisExpansion.additionalState as ExpansionInfo? if (argumentExpansion == null) { argumentExpansion = thisExpansion.readArgument(STRUCTS) thisExpansion.additionalState = argumentExpansion @@ -502,7 +429,7 @@ class MacroEvaluator { return when (val next = currentChildExpansion?.produceNext()) { is DataModelExpression -> next - EndOfExpansion -> thisExpansion.dropDelegateAndContinue() + EndOfExpansion -> thisExpansion.closeDelegateAndContinue() // Only possible if expansionDelegate is null null -> when (val nextSequence = argumentExpansion.produceNext()) { is StructValue -> { @@ -522,11 +449,16 @@ class MacroEvaluator { } }, + /** + * Iterates over the sequences, returning the values contained in the sequences. + * The expansion for the sequences argument is stored in [ExpansionInfo.additionalState]. + * When + */ Flatten { private val SEQUENCES = VariableRef(0) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - var argumentExpansion: ExpansionFrame? = thisExpansion.additionalState as ExpansionFrame? + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + var argumentExpansion: ExpansionInfo? = thisExpansion.additionalState as ExpansionInfo? if (argumentExpansion == null) { argumentExpansion = thisExpansion.readArgument(SEQUENCES) thisExpansion.additionalState = argumentExpansion @@ -536,7 +468,7 @@ class MacroEvaluator { return when (val next = currentChildExpansion?.produceNext()) { is DataModelExpression -> next - EndOfExpansion -> thisExpansion.dropDelegateAndContinue() + EndOfExpansion -> thisExpansion.closeDelegateAndContinue() // Only possible if expansionDelegate is null null -> when (val nextSequence = argumentExpansion.produceNext()) { is StructValue -> throw IonException("invalid argument; flatten expects sequences") @@ -561,21 +493,19 @@ class MacroEvaluator { private val ARG_A = VariableRef(0) private val ARG_B = VariableRef(1) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - if (thisExpansion.additionalState != null) return EndOfExpansion - thisExpansion.additionalState = Unit - + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + // TODO(PERF): consider checking whether the value would fit in a long and returning a `LongIntValue`. val a = thisExpansion.readExactlyOneArgument(ARG_A).bigIntegerValue val b = thisExpansion.readExactlyOneArgument(ARG_B).bigIntegerValue + thisExpansion.expanderKind = Empty return BigIntValue(value = a + b) } }, Delta { private val ARGS = VariableRef(0) - // Initial value = 0 - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { - // TODO: Optimize to use LongIntValue when possible + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { + // TODO(PERF): Optimize to use LongIntValue when possible var delegate = thisExpansion.expansionDelegate val runningTotal = thisExpansion.additionalState as? BigInteger ?: BigInteger.ZERO if (delegate == null) { @@ -590,9 +520,8 @@ class MacroEvaluator { thisExpansion.additionalState = nextOutput return BigIntValue(value = nextOutput) } - EndOfExpansion -> return nextExpandedArg - is DataModelValue -> throw IonException("delta arguments must be integers") - is FieldName -> unreachable() + EndOfExpansion -> return EndOfExpansion + else -> throw IonException("delta arguments must be integers") } } }, @@ -600,7 +529,7 @@ class MacroEvaluator { private val COUNT_ARG = VariableRef(0) private val THING_TO_REPEAT = VariableRef(1) - override fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue { + override fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue { var n = thisExpansion.additionalState as Long? if (n == null) { n = thisExpansion.readExactlyOneArgument(COUNT_ARG).longValue @@ -620,52 +549,42 @@ class MacroEvaluator { val repeated = thisExpansion.expansionDelegate!! return when (val maybeNext = repeated.produceNext()) { is DataModelExpression -> maybeNext - EndOfExpansion -> thisExpansion.dropDelegateAndContinue() + EndOfExpansion -> thisExpansion.closeDelegateAndContinue() } } }, ; - abstract fun produceNext(thisExpansion: ExpansionFrame): ExpansionOutputExpressionOrContinue + /** + * Produces the next value, [EndOfExpansion], or [ContinueExpansion]. + * Each enum variant must implement this method. + */ + abstract fun produceNext(thisExpansion: ExpansionInfo): ExpansionOutputExpressionOrContinue - internal inline fun checkExpansionSize(thisExpansion: ExpansionFrame, condition: (Int) -> Boolean): ContinueExpansion { + /** Helper function for the `if_*` macros */ + inline fun ExpansionInfo.branchIf(condition: (Int) -> Boolean): ContinueExpansion { val argToTest = VariableRef(0) val trueBranch = VariableRef(1) val falseBranch = VariableRef(2) - val testArg = thisExpansion.readArgument(argToTest) + val testArg = readArgument(argToTest) var n = 0 while (n < 2) { if (testArg.produceNext() is EndOfExpansion) break n++ } - testArg.drop() + testArg.close() val branch = if (condition(n)) trueBranch else falseBranch - val branchExpansion = thisExpansion.readArgument(branch) - thisExpansion.tailCall(branchExpansion) + tailCall(readArgument(branch)) return ContinueExpansion } - internal fun VariableRef.readFrom(environment: Environment, session: MacroEvaluationSession): ExpansionFrame { - val argIndex = environment.argumentIndices[signatureIndex] - if (argIndex < 0) { - // Argument was elided. - return session.getExpander(Empty, emptyList(), 0, 0, Environment.EMPTY) - } - val firstArgExpression = environment.arguments[argIndex] - - return session.getExpander( - expanderKind = Stream, - expressions = environment.arguments, - startInclusive = if (firstArgExpression is ExpressionGroup) firstArgExpression.startInclusive else argIndex, - endExclusive = if (firstArgExpression is HasStartAndEnd) firstArgExpression.endExclusive else argIndex + 1, - environment = environment.parentEnvironment!! - ) - } - - internal fun ExpansionFrame.readArgument(variableRef: VariableRef): ExpansionFrame { + /** + * Returns an expansion for the given variable. + */ + fun ExpansionInfo.readArgument(variableRef: VariableRef): ExpansionInfo { val argIndex = environment.argumentIndices[variableRef.signatureIndex] if (argIndex < 0) { // Argument was elided. @@ -673,7 +592,7 @@ class MacroEvaluator { } val firstArgExpression = environment.arguments[argIndex] return session.getExpander( - expanderKind = Stream, + expanderKind = Variable, expressions = environment.arguments, startInclusive = if (firstArgExpression is ExpressionGroup) firstArgExpression.startInclusive else argIndex, endExclusive = if (firstArgExpression is HasStartAndEnd) firstArgExpression.endExclusive else argIndex + 1, @@ -681,7 +600,10 @@ class MacroEvaluator { ) } - internal inline fun ExpansionFrame.forEach(variableRef: VariableRef, action: (DataModelExpression) -> Unit) { + /** + * Performs the given [action] for each value produced by the expansion of [variableRef]. + */ + inline fun ExpansionInfo.forEach(variableRef: VariableRef, action: (DataModelExpression) -> Unit) { val variableExpansion = readArgument(variableRef) while (true) { when (val next = variableExpansion.produceNext()) { @@ -691,18 +613,27 @@ class MacroEvaluator { } } - internal inline fun ExpansionFrame.map(variableRef: VariableRef, action: (DataModelExpression) -> T): List { + /** + * Performs the given [transform] on each value produced by the expansion of [variableRef], returning a list + * of the results. + */ + inline fun ExpansionInfo.map(variableRef: VariableRef, transform: (DataModelExpression) -> T): List { val variableExpansion = readArgument(variableRef) val result = mutableListOf() while (true) { when (val next = variableExpansion.produceNext()) { EndOfExpansion -> return result - is DataModelExpression -> result.add(action(next)) + is DataModelExpression -> result.add(transform(next)) } } } - internal inline fun ExpansionFrame.readZeroOrOneArgument(variableRef: VariableRef): T? { + /** + * Reads and returns zero or one values from the expansion of the given [variableRef]. + * Throws an [IonException] if more than one value is present in the variable expansion. + * Throws an [IonException] if the value is not the expected type [T]. + */ + inline fun ExpansionInfo.readZeroOrOneArgument(variableRef: VariableRef): T? { val argExpansion = readArgument(variableRef) var argValue: T? = null while (true) { @@ -717,43 +648,58 @@ class MacroEvaluator { is FieldName -> unreachable("Unreachable without stepping into a container") } } + argExpansion.close() return argValue } - internal inline fun ExpansionFrame.readExactlyOneArgument(variableRef: VariableRef): T { + /** + * Reads and returns exactly one value from the expansion of the given [variableRef]. + * Throws an [IonException] if the expansion of [variableRef] does not produce exactly one value. + * Throws an [IonException] if the value is not the expected type [T]. + */ + inline fun ExpansionInfo.readExactlyOneArgument(variableRef: VariableRef): T { return readZeroOrOneArgument(variableRef) ?: throw IonException("invalid argument; no value when one is expected") } companion object { + /** + * Gets the [ExpanderKind] for the given [macro]. + */ @JvmStatic - fun getExpanderKindForSystemMacro(systemMacro: SystemMacro) = when (systemMacro) { - SystemMacro.Annotate -> Annotate - SystemMacro.MakeString -> MakeString - SystemMacro.MakeSymbol -> MakeSymbol - SystemMacro.MakeDecimal -> MakeDecimal - SystemMacro.Repeat -> Repeat - SystemMacro.Sum -> Sum - SystemMacro.Delta -> Delta - SystemMacro.MakeBlob -> MakeBlob - SystemMacro.Flatten -> Flatten - SystemMacro._Private_FlattenStruct -> _Private_FlattenStruct - SystemMacro.MakeTimestamp -> MakeTimestamp - SystemMacro._Private_MakeFieldNameAndValue -> _Private_MakeFieldNameAndValue - SystemMacro.IfNone -> IfNone - SystemMacro.IfSome -> IfSome - SystemMacro.IfSingle -> IfSingle - SystemMacro.IfMulti -> IfMulti - else -> if (systemMacro.body != null) { - throw IllegalStateException("SystemMacro ${systemMacro.name} should be using its template body.") - } else { - TODO("Not implemented yet: ${systemMacro.name}") + fun forMacro(macro: Macro): ExpanderKind { + return if (macro.body != null) { + TemplateBody + } else when (macro as SystemMacro) { + SystemMacro.Annotate -> Annotate + SystemMacro.MakeString -> MakeString + SystemMacro.MakeSymbol -> MakeSymbol + SystemMacro.MakeDecimal -> MakeDecimal + SystemMacro.Repeat -> Repeat + SystemMacro.Sum -> Sum + SystemMacro.Delta -> Delta + SystemMacro.MakeBlob -> MakeBlob + SystemMacro.Flatten -> Flatten + SystemMacro._Private_FlattenStruct -> _Private_FlattenStruct + SystemMacro.MakeTimestamp -> MakeTimestamp + SystemMacro._Private_MakeFieldNameAndValue -> _Private_MakeFieldNameAndValue + SystemMacro.IfNone -> IfNone + SystemMacro.IfSome -> IfSome + SystemMacro.IfSingle -> IfSingle + SystemMacro.IfMulti -> IfMulti + else -> TODO("Not implemented yet: ${macro.name}") } } } } - class ExpansionFrame( - @JvmField val session: MacroEvaluationSession, + /** + * Represents a frame in the expansion stack for a particular container. + * + * TODO: "info" is very non-specific; rename to ExpansionFrame next time there's a + * non-functional refactoring in this class. + */ + private class ExpansionInfo( + @JvmField val session: Session, @JvmField var expanderKind: ExpanderKind = Uninitialized, /** * The [Expression]s being expanded. This MUST be the original list, not a sublist because @@ -763,56 +709,56 @@ class MacroEvaluator { @JvmField var expressions: List = emptyList(), /** Current position within [expressions] of this expansion */ @JvmField var i: Int = 0, - /** End of [expressions] that are applicable for this [ExpansionFrame] */ + /** End of [expressions] that are applicable for this [ExpansionInfo] */ @JvmField var endExclusive: Int = 0, /** The evaluation [Environment]—i.e. variable bindings. */ @JvmField var environment: Environment = Environment.EMPTY, - @JvmField var _expansionDelegate: ExpansionFrame? = null, + _expansionDelegate: ExpansionInfo? = null, @JvmField var additionalState: Any? = null, ) { - var expansionDelegate: ExpansionFrame? - get() = _expansionDelegate + // TODO: if expansionDelegate == this, it will cause an infinite loop or stack overflow somewhere. + // In practice, it should never happen, so we may wish to remove the custom setter to avoid any performance impact. + var expansionDelegate: ExpansionInfo? = _expansionDelegate set(value) { check(value != this) - _expansionDelegate = value + field = value } - fun dropDelegateAndContinue(): ContinueExpansion { - expansionDelegate?.drop() + /** + * Convenience function to close the [expansionDelegate] and return it to the pool. + */ + fun closeDelegateAndContinue(): ContinueExpansion { + expansionDelegate?.close() expansionDelegate = null return ContinueExpansion } - fun top(): ExpansionFrame = expansionDelegate?.top() ?: this + /** + * Gets the [ExpansionInfo] at the top of the stack of [expansionDelegate]s. + */ + fun top(): ExpansionInfo = expansionDelegate?.top() ?: this - fun drop() { + /** + * Returns this [ExpansionInfo] to the expander pool, recursively closing [expansionDelegate]s in the process. + * Could also be thought of as a `free` function. + */ + fun close() { expanderKind = Uninitialized - additionalState = null environment = Environment.EMPTY expressions = emptyList() - expansionDelegate?.drop() - expansionDelegate = null - session.returnExpander(this) - } - - fun initExpansion( - expanderKind: ExpanderKind, - expressions: List, - startInclusive: Int, - endExclusive: Int, - environment: Environment, - ) { - this.expanderKind = expanderKind - this.expressions = expressions - this.i = startInclusive - this.endExclusive = endExclusive - this.environment = environment + additionalState?.let { if (it is ExpansionInfo) it.close() } additionalState = null + expansionDelegate?.close() expansionDelegate = null + session.reclaimExpander(this) } - fun tailCall(other: ExpansionFrame) { + /** + * Replaces the state of `this` [ExpansionInfo] with the state of [other]—effectively a tail-call optimization. + * After transferring the state, `other` is returned to the expansion pool. + */ + fun tailCall(other: ExpansionInfo) { this.expanderKind = other.expanderKind this.expressions = other.expressions this.i = other.i @@ -820,26 +766,35 @@ class MacroEvaluator { this.expansionDelegate = other.expansionDelegate this.additionalState = other.additionalState this.environment = other.environment - // Drop `other` + // Close `other` other.expansionDelegate = null - other.drop() + other.close() } + /** + * Produces the next value from this expansion. + */ fun produceNext(): ExpansionOutputExpression { while (true) { val next = expanderKind.produceNext(this) if (next is ContinueExpansion) continue + // This the only place where we count the expansion steps. + // It is theoretically possible to have macro expansions that are millions of levels deep because this + // only counts macro invocations at the end of their expansion, but this will still work to catch things + // like a billion laughs attack because it does place a limit on the number of _values_ produced. + // This counts every value _at every level_, so most values will be counted multiple times. If possible + // without impacting performance, count values only once in order to have more predictable behavior. session.incrementStepCounter() return next as ExpansionOutputExpression } } override fun toString() = """ - |ExpansionFrame( + |ExpansionInfo( | expansionKind: $expanderKind, | environment: ${environment.toString().lines().joinToString("\n| ")}, | expressions: [ - | ${expressions.mapIndexed { index, expression -> "$index. $expression" }.joinToString(",\n| ") { it.toString() } } + | ${expressions.mapIndexed { i, expr -> "$i. $expr" }.joinToString(",\n| ") } | ], | endExclusive: $endExclusive, | i: $i, @@ -848,6 +803,87 @@ class MacroEvaluator { |) """.trimMargin() } + + private val session = Session(expansionLimit = 1_000_000) + private val containerStack = _Private_RecyclingStack(8) { ContainerInfo() } + private var currentExpr: DataModelExpression? = null + + /** + * Returns the e-expression argument expressions that this MacroEvaluator would evaluate. + */ + fun getArguments(): List { + return containerStack.iterator().next().expansion.expressions + } + + /** + * Initialize the macro evaluator with an E-Expression. + */ + fun initExpansion(encodingExpressions: List) { + session.reset() + containerStack.push { ci -> + ci.type = ContainerInfo.Type.TopLevel + ci.expansion = session.getExpander(Stream, encodingExpressions, 0, encodingExpressions.size, Environment.EMPTY) + } + } + + /** + * Evaluate the macro expansion until the next [DataModelExpression] can be returned. + * Returns null if at the end of a container or at the end of the expansion. + */ + fun expandNext(): DataModelExpression? { + currentExpr = null + val currentContainer = containerStack.peek() + when (val nextExpansionOutput = currentContainer.produceNext()) { + is DataModelExpression -> currentExpr = nextExpansionOutput + EndOfExpansion -> { + if (currentContainer.type == ContainerInfo.Type.TopLevel) { + currentContainer.close() + containerStack.pop() + } + } + } + return currentExpr + } + + /** + * Steps out of the current [DataModelContainer]. + */ + fun stepOut() { + // TODO: We should be able to step out of a "TopLevel" container and/or we need some way to close the evaluation early. + if (containerStack.size() <= 1) throw IonException("Nothing to step out of.") + val popped = containerStack.pop() + popped.close() + } + + /** + * Steps in to the current [DataModelContainer]. + * Throws [IonException] if not positioned on a container. + */ + fun stepIn() { + val expression = requireNotNull(currentExpr) { "Not positioned on a value" } + if (expression is DataModelContainer) { + val currentContainer = containerStack.peek() + val topExpansion = currentContainer.expansion.top() + containerStack.push { ci -> + ci.type = when (expression.type) { + IonType.LIST -> ContainerInfo.Type.List + IonType.SEXP -> ContainerInfo.Type.Sexp + IonType.STRUCT -> ContainerInfo.Type.Struct + else -> unreachable() + } + ci.expansion = session.getExpander( + expanderKind = Stream, + expressions = topExpansion.expressions, + startInclusive = expression.startInclusive, + endExclusive = expression.endExclusive, + environment = topExpansion.environment, + ) + } + currentExpr = null + } else { + throw IonException("Not positioned on a container.") + } + } } /** @@ -859,17 +895,18 @@ class MacroEvaluator { * This function also validates that the correct number of parameters are present. If there are * too many parameters or too few parameters, this will throw [IonException]. */ -private fun Macro.calculateArgumentIndices( +private fun calculateArgumentIndices( + macro: Macro, encodingExpressions: List, argsStartInclusive: Int, argsEndExclusive: Int ): List { // TODO: For TDL macro invocations, see if we can calculate this during the "compile" step. var numArgs = 0 - val argsIndices = IntArray(signature.size) + val argsIndices = IntArray(macro.signature.size) var currentArgIndex = argsStartInclusive - for (p in signature) { + for (p in macro.signature) { if (currentArgIndex >= argsEndExclusive) { if (!p.cardinality.canBeVoid) throw IonException("No value provided for parameter ${p.variableName}") // Elided rest parameter. @@ -890,8 +927,8 @@ private fun Macro.calculateArgumentIndices( } numArgs++ } - if (numArgs > signature.size) { - throw IonException("Too many arguments. Expected ${signature.size}, but found $numArgs") + if (numArgs > macro.signature.size) { + throw IonException("Too many arguments. Expected ${macro.signature.size}, but found $numArgs") } return argsIndices.toList() } diff --git a/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt b/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt index 67cead5b3..8d3c997a4 100644 --- a/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt +++ b/src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt @@ -15,7 +15,7 @@ import com.amazon.ion.impl.macro.ParameterFactory.zeroToManyTagged */ enum class SystemMacro( val id: Byte, - val systemSymbol: SystemSymbols_1_1?, + private val _systemSymbol: SystemSymbols_1_1?, override val signature: List, override val body: List? = null ) : Macro { @@ -27,8 +27,9 @@ enum class SystemMacro( IfMulti(-1, IF_MULTI, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))), // Unnameable, unaddressable macros used for the internals of certain other system macros - _Private_FlattenStruct(-1, systemSymbol = null, listOf(zeroToManyTagged("structs"))), - _Private_MakeFieldNameAndValue(-1, systemSymbol = null, listOf(exactlyOneTagged("fieldName"), exactlyOneTagged("value"))), + // TODO: See if we can move these somewhere else so that they are not visible + _Private_FlattenStruct(-1, _systemSymbol = null, listOf(zeroToManyTagged("structs"))), + _Private_MakeFieldNameAndValue(-1, _systemSymbol = null, listOf(exactlyOneTagged("fieldName"), exactlyOneTagged("value"))), // The real macros Values(1, VALUES, listOf(zeroToManyTagged("values")), templateBody { variable(0) }), @@ -260,7 +261,10 @@ enum class SystemMacro( ), ; - val macroName: String get() = this.systemSymbol?.text ?: throw IllegalStateException("Attempt to get name for unaddressable macro $name") + val systemSymbol: SystemSymbols_1_1 + get() = _systemSymbol ?: throw IllegalStateException("Attempted to get name for unaddressable macro $name") + + val macroName: String get() = this.systemSymbol.text override val dependencies: List get() = body @@ -272,7 +276,7 @@ enum class SystemMacro( companion object : MacroTable { private val MACROS_BY_NAME: Map = SystemMacro.entries - .filter { it.systemSymbol != null } + .filter { it._systemSymbol != null } .associateBy { it.macroName } // TODO: Once all of the macros are implemented, replace this with an array as in SystemSymbols_1_1 diff --git a/src/test/java/com/amazon/ion/impl/IonRawTextWriterTest_1_1.kt b/src/test/java/com/amazon/ion/impl/IonRawTextWriterTest_1_1.kt index 4a9d68ecb..ec5092817 100644 --- a/src/test/java/com/amazon/ion/impl/IonRawTextWriterTest_1_1.kt +++ b/src/test/java/com/amazon/ion/impl/IonRawTextWriterTest_1_1.kt @@ -713,7 +713,11 @@ class IonRawTextWriterTest_1_1 { @ParameterizedTest @EnumSource(SystemMacro::class) fun `write system macro E-expression by name`(systemMacro: SystemMacro) { - if (systemMacro.systemSymbol == null) throw TestAbortedException("Skip this test for unaddressable macros") + try { + systemMacro.systemSymbol + } catch (e: IllegalStateException) { + throw TestAbortedException("Skip this test for unaddressable macros") + } assertWriterOutputEquals("(:\$ion::${systemMacro.macroName})") { stepInEExp(systemMacro) stepOut()