Skip to content

Commit

Permalink
Macro implementations and conformance test fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
popematt committed Nov 26, 2024
1 parent 7177234 commit f9c34fd
Show file tree
Hide file tree
Showing 13 changed files with 309 additions and 46 deletions.
2 changes: 1 addition & 1 deletion ion-tests
Submodule ion-tests updated 60 files
+15 −0 .github/workflows/main.yaml
+34 −0 check-syntax
+56 −37 conformance/README.md
+3 −3 conformance/core/empty_document.ion
+0 −20 conformance/core/typed_null.ion
+48 −0 conformance/data_model/annotations.ion
+40 −0 conformance/data_model/boolean.ion
+340 −0 conformance/data_model/float.ion
+162 −162 conformance/data_model/integer.ion
+85 −0 conformance/data_model/null.ion
+4 −4 conformance/eexp/basic_system_macros.ion
+629 −0 conformance/eexp/binary/argument_encoding.ion
+327 −0 conformance/grammar.isl
+2 −2 conformance/ion_encoding/module/macro/cardinality/invoke_cardinality_ee.ion
+2 −2 conformance/ion_encoding/module/macro/trivial/invoke_ee.ion
+1 −1 conformance/ion_encoding/module/macro_table.ion
+3 −3 conformance/ivm.ion
+0 −7 conformance/null.ion
+87 −0 conformance/system_macros/add_macros.ion
+85 −0 conformance/system_macros/add_symbols.ion
+9 −0 conformance/system_macros/annotate.ion
+84 −0 conformance/system_macros/default.ion
+84 −0 conformance/system_macros/delta.ion
+8 −0 conformance/system_macros/flatten.ion
+8 −0 conformance/system_macros/make_blob.ion
+64 −0 conformance/system_macros/make_decimal.ion
+68 −0 conformance/system_macros/make_field.ion
+62 −0 conformance/system_macros/make_list.ion
+62 −0 conformance/system_macros/make_sexp.ion
+74 −0 conformance/system_macros/make_string.ion
+60 −0 conformance/system_macros/make_struct.ion
+74 −0 conformance/system_macros/make_symbol.ion
+305 −0 conformance/system_macros/make_timestamp.ion
+39 −0 conformance/system_macros/meta.ion
+27 −0 conformance/system_macros/none.ion
+25 −0 conformance/system_macros/parse_ion.ion
+72 −0 conformance/system_macros/repeat.ion
+87 −0 conformance/system_macros/set_macros.ion
+82 −0 conformance/system_macros/set_symbols.ion
+84 −0 conformance/system_macros/sum.ion
+12 −0 conformance/system_macros/use.ion
+40 −0 conformance/system_macros/values.ion
+1 −1 iontestdata_1_1/bad/annotationSymbolIDUnmapped.ion
+1 −1 iontestdata_1_1/bad/fieldNameSymbolIDUnmapped.ion
+45 −0 iontestdata_1_1/good/equivs/macros/none.ion
+0 −45 iontestdata_1_1/good/equivs/macros/void.ion
+3 −3 iontestdata_1_1/good/macros/README.md
+1 −1 iontestdata_1_1/good/macros/none.ion
+25 −0 iontestdata_1_1/good/macros/none_invoked_deeply_nested.ion
+6 −0 iontestdata_1_1/good/macros/none_invoked_in_list.ion
+5 −0 iontestdata_1_1/good/macros/none_invoked_in_sexp.ion
+6 −0 iontestdata_1_1/good/macros/none_invoked_in_struct.ion
+4 −0 iontestdata_1_1/good/macros/none_invoked_in_struct_field.ion
+2 −0 iontestdata_1_1/good/macros/none_invoked_in_values_macro.ion
+0 −25 iontestdata_1_1/good/macros/void_invoked_deeply_nested.ion
+0 −6 iontestdata_1_1/good/macros/void_invoked_in_list.ion
+0 −5 iontestdata_1_1/good/macros/void_invoked_in_sexp.ion
+0 −6 iontestdata_1_1/good/macros/void_invoked_in_struct.ion
+0 −4 iontestdata_1_1/good/macros/void_invoked_in_struct_field.ion
+0 −2 iontestdata_1_1/good/macros/void_invoked_in_values_macro.ion
2 changes: 1 addition & 1 deletion src/main/java/com/amazon/ion/impl/IonCursorBinary.java
Original file line number Diff line number Diff line change
Expand Up @@ -2344,7 +2344,7 @@ private void uncheckedReadMacroInvocationHeader(IonTypeID valueTid, Marker marke
private boolean uncheckedReadHeader(final int typeIdByte, final boolean isAnnotated, final Marker markerToSet) {
IonTypeID valueTid = typeIds[typeIdByte];
if (!valueTid.isValid) {
throw new IonException("Invalid type ID.");
throw new IonException("Invalid type ID: " + valueTid.theByte);
} else if (valueTid.type == IonTypeID.ION_TYPE_ANNOTATION_WRAPPER) {
if (isAnnotated) {
throw new IonException("Nested annotation wrappers are invalid.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1625,8 +1625,7 @@ protected void readParameter(Macro.Parameter parameter, long parameterPresence,
case OneOrMore:
if (parameterPresence == PresenceBitmap.EXPRESSION) {
readSingleExpression(parameter, expressions);
}
if (parameterPresence == PresenceBitmap.GROUP) {
} else if (parameterPresence == PresenceBitmap.GROUP) {
readGroupExpression(parameter, expressions, false);
} else {
throw new IonException(String.format(
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java
Original file line number Diff line number Diff line change
Expand Up @@ -1237,6 +1237,9 @@ private class TextEExpressionArgsReader extends EExpressionArgsReader {
@Override
protected void readParameter(Macro.Parameter parameter, long parameterPresence, List<Expression.EExpressionBodyExpression> expressions, boolean isTrailing) {
if (IonReaderTextSystemX.this.nextRaw() == null) {
// Add an empty expression group if nothing present.
int index = expressions.size() + 1;
expressions.add(new Expression.ExpressionGroup(index, index));
return;
}
readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti, expressions);
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ internal class IonManagedWriter_1_1(
private val onClose: () -> Unit,
) : _Private_IonWriter, MacroAwareIonWriter {

internal fun getRawUserWriter() : IonRawWriter_1_1 = userData

companion object {
private val ION_VERSION_MARKER_REGEX = Regex("^\\\$ion_\\d+_\\d+$")

Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/amazon/ion/impl/macro/Expression.kt
Original file line number Diff line number Diff line change
Expand Up @@ -95,18 +95,21 @@ sealed interface Expression {

sealed interface IntValue : DataModelValue {
val bigIntegerValue: BigInteger
val longValue: Long
}

data class LongIntValue(override val annotations: List<SymbolToken> = emptyList(), val value: Long) : IntValue {
override val type: IonType get() = IonType.INT
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
override val bigIntegerValue: BigInteger get() = BigInteger.valueOf(value)
override val longValue: Long get() = value
}

data class BigIntValue(override val annotations: List<SymbolToken> = emptyList(), val value: BigInteger) : IntValue {
override val type: IonType get() = IonType.INT
override fun withAnnotations(annotations: List<SymbolToken>) = copy(annotations = annotations)
override val bigIntegerValue: BigInteger get() = value
override val longValue: Long get() = value.longValueExact()
}

data class FloatValue(override val annotations: List<SymbolToken> = emptyList(), val value: Double) : DataModelValue {
Expand Down
154 changes: 140 additions & 14 deletions src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// SPDX-License-Identifier: Apache-2.0
package com.amazon.ion.impl.macro

import com.amazon.ion.IonException
import com.amazon.ion.SymbolToken
import com.amazon.ion.*
import com.amazon.ion.impl._Private_RecyclingStack
import com.amazon.ion.impl._Private_Utils.newSymbolToken
import com.amazon.ion.impl.macro.Expression.*
import java.io.ByteArrayOutputStream
import java.math.BigDecimal
import java.math.BigInteger

/**
* Evaluates an EExpression from a List of [EExpressionBodyExpression] and the [TemplateBodyExpression]s
Expand Down Expand Up @@ -209,6 +209,119 @@ class MacroEvaluator {
}
}

private object MakeTimestampExpander : Expander {
private fun readOptionalIntArg(
signatureIndex: Int,
expansionInfo: ExpansionInfo,
macroEvaluator: MacroEvaluator
): Int? {
if (expansionInfo.i == expansionInfo.endExclusive) return null
val parameterName = SystemMacro.MakeTimestamp.signature[signatureIndex].variableName
val arg = readZeroOrOneExpandedArgumentValues(expansionInfo, macroEvaluator, parameterName)
return arg?.let {
it as? IntValue ?: throw IonException("$parameterName must be an integer")
it.longValue.toInt()
}
}

override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression {
val year = readExactlyOneExpandedArgumentValue(expansionInfo, macroEvaluator, SystemMacro.MakeTimestamp.signature[0].variableName)
.let { it as? IntValue ?: throw IonException("year must be an integer") }
.longValue.toInt()
val month = readOptionalIntArg(1, expansionInfo, macroEvaluator)
val day = readOptionalIntArg(2, expansionInfo, macroEvaluator)
val hour = readOptionalIntArg(3, expansionInfo, macroEvaluator)
val minute = readOptionalIntArg(4, expansionInfo, macroEvaluator)
val second = if (expansionInfo.i == expansionInfo.endExclusive) {
null
} else when (val arg = readZeroOrOneExpandedArgumentValues(expansionInfo, macroEvaluator, SystemMacro.MakeTimestamp.signature[5].variableName)) {
null -> null
is DecimalValue -> arg.value
is IntValue -> arg.longValue.toBigDecimal()
else -> throw IonException("second must be a decimal")
}
val offsetMinutes = readOptionalIntArg(6, expansionInfo, macroEvaluator)

try {
val ts = if (second != null) {
month ?: throw IonException("make_timestamp: month is required when second is present")
day ?: throw IonException("make_timestamp: day is required when second is present")
hour ?: throw IonException("make_timestamp: hour is required when second is present")
minute ?: throw IonException("make_timestamp: minute is required when second is present")
Timestamp.forSecond(year, month, day, hour, minute, second, offsetMinutes)
} else if (minute != null) {
month ?: throw IonException("make_timestamp: month is required when minute is present")
day ?: throw IonException("make_timestamp: day is required when minute is present")
hour ?: throw IonException("make_timestamp: hour is required when minute is present")
Timestamp.forMinute(year, month, day, hour, minute, offsetMinutes)
} else if (hour != null) {
throw IonException("make_timestamp: minute is required when hour is present")
} else {
if (offsetMinutes != null) throw IonException("make_timestamp: offset_minutes is prohibited when hours and minute are not present")
if (day != null) {
month ?: throw IonException("make_timestamp: month is required when day is present")
Timestamp.forDay(year, month, day)
} else if (month != null) {
Timestamp.forMonth(year, month)
} else {
Timestamp.forYear(year)
}
}
return TimestampValue(value = ts)
} catch (e: IllegalArgumentException) {
throw IonException(e.message)
}
}
}

private object SumExpander : Expander {
override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression {
val a = readExactlyOneExpandedArgumentValue(expansionInfo, macroEvaluator, "a")
val b = readExactlyOneExpandedArgumentValue(expansionInfo, macroEvaluator, "b")
if (a !is IntValue || b !is IntValue) throw IonException("operands of sum must be integers")
// TODO: Use LongIntValue when possible.
return BigIntValue(value = a.bigIntegerValue + b.bigIntegerValue)
}
}

private object DeltaExpander : Expander {
override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression {
// TODO: Optimize to use Long and only fallback to BigInteger if needed.
// TODO: Optimize for lazy evaluation
if (expansionInfo.additionalState == null) {
val position = expansionInfo.i
var runningTotal = BigInteger.ZERO
val values = ArrayDeque<BigInteger>()
readExpandedArgumentValues(expansionInfo, macroEvaluator) {
when (it) {
is IntValue -> {
runningTotal += it.bigIntegerValue
values += runningTotal
}
is DataModelValue -> throw IonException("Invalid argument type for 'delta': ${it.type}")
is FieldName -> TODO("Unreachable. We shouldn't be able to get here without first encountering a StructValue.")
}
true // continue expansion
}

if (values.isEmpty()) {
// Return fake, empty expression group
return ExpressionGroup(position, position)
}

expansionInfo.additionalState = values
expansionInfo.i = position
}

val valueQueue = expansionInfo.additionalState as ArrayDeque<BigInteger>
val nextValue = valueQueue.removeFirst()
if (valueQueue.isEmpty()) {
expansionInfo.i = expansionInfo.endExclusive
}
return BigIntValue(value = nextValue)
}
}

private enum class IfExpander(private val minInclusive: Int, private val maxExclusive: Int) : Expander {
IF_NONE(0, 1),
IF_SOME(1, -1),
Expand Down Expand Up @@ -254,11 +367,10 @@ class MacroEvaluator {
}
nExpression.value.intValueExact()
}
else -> throw IonException("The first argument of repeat must be a positive integer")
else -> throw IonException("The first argument of repeat must be a non-negative integer")
}
if (iterationsRemaining <= 0) {
// TODO: Confirm https://github.com/amazon-ion/ion-docs/issues/350
throw IonException("The first argument of repeat must be a positive integer")
if (iterationsRemaining < 0) {
throw IonException("The first argument of repeat must be a non-negative integer")
}
// Decrement because we're starting the first iteration right away.
iterationsRemaining--
Expand All @@ -267,21 +379,27 @@ class MacroEvaluator {
}

override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression {
val repeatsRemaining = expansionInfo.additionalState as? Int
val repeatsRemainingAfterTheCurrentOne = expansionInfo.additionalState as? Int
?: init(expansionInfo, macroEvaluator)

if (repeatsRemainingAfterTheCurrentOne < 0) {
expansionInfo.nextSourceExpression()
return ExpressionGroup(0, 0)
}

val repeatedExpressionIndex = expansionInfo.i
val next = readFirstExpandedArgumentValue(expansionInfo, macroEvaluator)
next ?: throw IonException("repeat macro requires at least one value for value parameter")
if (repeatsRemaining > 0) {
expansionInfo.additionalState = repeatsRemaining - 1
next ?: return ExpressionGroup(0, 0)
if (repeatsRemainingAfterTheCurrentOne > 0) {
expansionInfo.additionalState = repeatsRemainingAfterTheCurrentOne - 1
expansionInfo.i = repeatedExpressionIndex
}
return next
}
}

private object MakeFieldExpander : Expander {
// This is wrong!
override fun nextExpression(expansionInfo: ExpansionInfo, macroEvaluator: MacroEvaluator): Expression {
/**
* Uses [ExpansionInfo.additionalState] to track whether the expansion is on the field name or value.
Expand Down Expand Up @@ -317,7 +435,10 @@ class MacroEvaluator {
MakeSymbol(MakeSymbolExpander),
MakeBlob(MakeBlobExpander),
MakeDecimal(MakeDecimalExpander),
MakeTimestamp(MakeTimestampExpander),
MakeField(MakeFieldExpander),
Sum(SumExpander),
Delta(DeltaExpander),
IfNone(IfExpander.IF_NONE),
IfSome(IfExpander.IF_SOME),
IfSingle(IfExpander.IF_SINGLE),
Expand All @@ -328,23 +449,28 @@ class MacroEvaluator {
companion object {
@JvmStatic
fun forSystemMacro(macro: SystemMacro): ExpansionKind {
return if (macro.body != null) {
TemplateBody
} else when (macro) {
return when (macro) {
SystemMacro.None -> Values // "none" takes no args, so we can treat it as an empty "values" expansion
SystemMacro.Values -> Values
SystemMacro.Annotate -> Annotate
SystemMacro.MakeString -> MakeString
SystemMacro.MakeSymbol -> MakeSymbol
SystemMacro.MakeBlob -> MakeBlob
SystemMacro.MakeDecimal -> MakeDecimal
SystemMacro.MakeTimestamp -> MakeTimestamp
SystemMacro.IfNone -> IfNone
SystemMacro.IfSome -> IfSome
SystemMacro.IfSingle -> IfSingle
SystemMacro.IfMulti -> IfMulti
SystemMacro.Repeat -> Repeat
SystemMacro.MakeField -> MakeField
else -> throw IllegalStateException("Unreachable. All other macros have a template body.")
SystemMacro.Sum -> Sum
SystemMacro.Delta -> Delta
else -> if (macro.body != null) {
TemplateBody
} else {
TODO("System macro ${macro.macroName} needs either a template body or a hard-coded expander.")
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class MacroEvaluatorAsIonReader(
?: return Collections.emptyIterator()
}

override fun isInStruct(): Boolean = TODO("Not yet implemented")
override fun isInStruct(): Boolean = containerStack.peek().container is Expression.StructValue

override fun getFieldId(): Int = currentFieldName?.value?.sid ?: 0
override fun getFieldName(): String? = currentFieldName?.value?.text
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/com/amazon/ion/impl/macro/ParameterFactory.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,4 @@ object ParameterFactory {
fun oneToManyTagged(name: String) = Parameter(name, ParameterEncoding.Tagged, ParameterCardinality.OneOrMore)
@JvmStatic
fun exactlyOneTagged(name: String) = Parameter(name, ParameterEncoding.Tagged, ParameterCardinality.ExactlyOne)
@JvmStatic
fun exactlyOneFlexInt(name: String) = Parameter(name, ParameterEncoding.FlexInt, ParameterCardinality.ExactlyOne)
}
33 changes: 22 additions & 11 deletions src/main/java/com/amazon/ion/impl/macro/SystemMacro.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ package com.amazon.ion.impl.macro
import com.amazon.ion.impl.*
import com.amazon.ion.impl.SystemSymbols_1_1.*
import com.amazon.ion.impl.macro.ExpressionBuilderDsl.Companion.templateBody
import com.amazon.ion.impl.macro.ParameterFactory.exactlyOneFlexInt
import com.amazon.ion.impl.macro.ParameterFactory.exactlyOneTagged
import com.amazon.ion.impl.macro.ParameterFactory.oneToManyTagged
import com.amazon.ion.impl.macro.ParameterFactory.zeroOrOneTagged
import com.amazon.ion.impl.macro.ParameterFactory.zeroToManyTagged

Expand All @@ -22,10 +20,10 @@ enum class SystemMacro(
) : Macro {
// Technically not system macros, but special forms. However, it's easier to model them as if they are macros in TDL.
// We give them an ID of -1 to distinguish that they are not addressable outside TDL.
IfNone(-1, IF_NONE, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfSome(-1, IF_SOME, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfSingle(-1, IF_SINGLE, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfMulti(-1, IF_MULTI, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfNone(31, IF_NONE, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfSome(32, IF_SOME, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfSingle(33, IF_SINGLE, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
IfMulti(34, IF_MULTI, listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),

// The real macros
None(0, NONE, emptyList()),
Expand All @@ -34,7 +32,19 @@ enum class SystemMacro(
MakeString(3, MAKE_STRING, listOf(zeroToManyTagged("text"))),
MakeSymbol(4, MAKE_SYMBOL, listOf(zeroToManyTagged("text"))),
MakeBlob(5, MAKE_BLOB, listOf(zeroToManyTagged("bytes"))),
MakeDecimal(6, MAKE_DECIMAL, listOf(exactlyOneFlexInt("coefficient"), exactlyOneFlexInt("exponent"))),
MakeDecimal(6, MAKE_DECIMAL, listOf(exactlyOneTagged("coefficient"), exactlyOneTagged("exponent"))),
MakeTimestamp(7, MAKE_TIMESTAMP, listOf(
exactlyOneTagged("year"),
zeroOrOneTagged("month"),
zeroOrOneTagged("day"),
zeroOrOneTagged("hour"),
zeroOrOneTagged("minute"),
zeroOrOneTagged("second"),
zeroOrOneTagged("offset_minutes"),
)),
// TODO: make_list
// TODO: make_sexp
// TODO: make_struct

/**
* ```ion
Expand Down Expand Up @@ -176,8 +186,11 @@ enum class SystemMacro(
}
}
),

Repeat(17, REPEAT, listOf(exactlyOneTagged("n"), oneToManyTagged("value"))),
// TODO: parse_ion
Repeat(17, REPEAT, listOf(exactlyOneTagged("n"), zeroToManyTagged("value"))),
Delta(18, DELTA, listOf(zeroToManyTagged("deltas"))),
// TODO: flatten
Sum(20, SUM, listOf(exactlyOneTagged("a"), exactlyOneTagged("b"))),
Comment(21, META, listOf(zeroToManyTagged("values")), templateBody { macro(None) {} }),
MakeField(
22, MAKE_FIELD,
Expand All @@ -196,8 +209,6 @@ enum class SystemMacro(
}
}
),

// TODO: Other system macros
;

val macroName: String get() = this.systemSymbol.text
Expand Down
Loading

0 comments on commit f9c34fd

Please sign in to comment.