Macro implementations and conformance test fixes (#1003)

Author: popematt

ion-tests

@@ -2348,7 +2348,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.");

src/main/java/com/amazon/ion/impl/IonCursorBinary.java

@@ -1638,8 +1638,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(

src/main/java/com/amazon/ion/impl/IonReaderContinuableCoreBinary.java

@@ -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);

src/main/java/com/amazon/ion/impl/IonReaderTextSystemX.java

@@ -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+$")
 

src/main/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1.kt

@@ -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 {

src/main/java/com/amazon/ion/impl/macro/Expression.kt

@@ -258,7 +258,7 @@ internal class MacroCompiler(
             }
             else -> throw IonException("macro invocation must start with an id (int) or identifier (symbol); found ${encodingType() ?: "nothing"}\"")
         }
-        val m = if (isQualifiedSystemMacro) SystemMacro.get(macroRef) else getMacro(macroRef)
+        val m = if (isQualifiedSystemMacro) SystemMacro.getMacroOrSpecialForm(macroRef) else getMacro(macroRef)
         return m ?: throw IonException("Unrecognized macro: $macroRef")
     }
 

src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt

@@ -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
@@ -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),
@@ -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--
@@ -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.
@@ -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),
@@ -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.")
+                    }
                 }
             }
         }

src/main/java/com/amazon/ion/impl/macro/MacroEvaluator.kt

@@ -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)
 }

src/main/java/com/amazon/ion/impl/macro/ParameterFactory.kt

@@ -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
 
@@ -34,7 +32,22 @@ 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
@@ -176,8 +189,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,
@@ -196,8 +212,6 @@ enum class SystemMacro(
             }
         }
     ),
-
-    // TODO: Other system macros
     ;
 
     val macroName: String get() = this.systemSymbol.text
@@ -239,7 +253,12 @@ enum class SystemMacro(
 
         /** Gets a [SystemMacro] by name, including those which are not in the system table (i.e. special forms) */
         @JvmStatic
-        fun getMacroOrSpecialForm(name: String): SystemMacro? = MACROS_BY_NAME[name]
+        fun getMacroOrSpecialForm(ref: MacroRef): SystemMacro? {
+            return when (ref) {
+                is MacroRef.ById -> get(ref.id)
+                is MacroRef.ByName -> MACROS_BY_NAME[ref.name]
+            }
+        }
 
         @JvmStatic
         val SYSTEM_MACRO_TABLE = this