Add support for writing System E-Expression invocations (#965)

src/main/java/com/amazon/ion/impl/IonRawTextWriter_1_1.kt

@@ -386,6 +386,18 @@ class IonRawTextWriter_1_1 internal constructor(
         isPendingSeparator = true // Treat the macro id as if it is a value that needs a separator.
     }
 
+    override fun stepInEExp(systemMacro: SystemMacro) {
+        confirm(numAnnotations == 0) { "Cannot annotate a macro invocation" }
+        openValue {
+            output.appendAscii("(:\$ion::")
+            output.printSymbol(systemMacro.macroName)
+        }
+        ancestorContainersStack.add(currentContainer)
+        currentContainer = EExpression
+        currentContainerHasValues = false
+        isPendingSeparator = true // Treat the macro name as if it is a value that needs a separator.
+    }
+
     override fun stepInExpressionGroup(usingLengthPrefix: Boolean) {
         confirm(numAnnotations == 0) { "Cannot annotate an expression group" }
         confirm(currentContainer == EExpression) { "Can only create an expression group in a macro invocation" }

src/main/java/com/amazon/ion/impl/IonRawWriter_1_1.kt

@@ -172,6 +172,14 @@ interface IonRawWriter_1_1 {
      */
     fun stepInEExp(id: Int, usingLengthPrefix: Boolean, macro: Macro)
 
+    /**
+     * Writes a system macro invocation for the given system macro.
+     * A macro is not a container in the Ion data model, but it is a container from an encoding perspective.
+     *
+     * TODO: Consider adding `usingLengthPrefix: Boolean`.
+     */
+    fun stepInEExp(systemMacro: SystemMacro)
+
     /**
      * Steps out of the current container.
      */

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

@@ -351,21 +351,23 @@ internal class IonManagedWriter_1_1(
                 val name = macroNames[address]
                 when (macro) {
                     is TemplateMacro -> writeMacroDefinition(name, macro)
-                    is SystemMacro -> exportSystemMacro(macro)
+                    is SystemMacro -> exportSystemMacro(macro, name)
                 }
             }
             forceNoNewlines(true)
         }
         systemData.forceNoNewlines(false)
     }
 
-    private fun exportSystemMacro(macro: SystemMacro) {
-        // TODO: Support for aliases
+    private fun exportSystemMacro(macro: SystemMacro, alias: String?) {
         writeSystemSexp {
             forceNoNewlines(true)
             writeSymbol(SystemSymbols_1_1.EXPORT)
             writeAnnotations(SystemSymbols_1_1.ION)
             writeSymbol(macro.macroName)
+            if (alias != null && alias != macro.macroName) {
+                writeSymbol(alias)
+            }
         }
         systemData.forceNoNewlines(false)
     }
@@ -814,13 +816,21 @@ internal class IonManagedWriter_1_1(
     }
 
     override fun startMacro(macro: Macro) {
-        val address = getOrAssignMacroAddress(macro)
-        startMacro(null, address, macro)
+        if (macro is SystemMacro) {
+            startSystemMacro(macro)
+        } else {
+            val address = getOrAssignMacroAddress(macro)
+            startMacro(null, address, macro)
+        }
     }
 
     override fun startMacro(name: String, macro: Macro) {
-        val address = getOrAssignMacroAddressAndName(name, macro)
-        startMacro(name, address, macro)
+        if (macro is SystemMacro && macro.macroName == name) {
+            startSystemMacro(macro)
+        } else {
+            val address = getOrAssignMacroAddressAndName(name, macro)
+            startMacro(name, address, macro)
+        }
     }
 
     private fun startMacro(name: String?, address: Int, definition: Macro) {
@@ -833,6 +843,24 @@ internal class IonManagedWriter_1_1(
         }
     }
 
+    private fun startSystemMacro(macro: SystemMacro) {
+        var macroName = macro.macroName
+        // TODO: Replace `indexOf` with something that is not O(n) time complexity.
+        var id = macroNames.indexOf(macroName)
+        if (id < 0) {
+            // If the name is not in use, put it into the user macro table
+            id = getOrAssignMacroAddressAndName(macroName, macro)
+        }
+        if (macrosById[id] == macro) {
+            // The name and id in the local table refers to the system macro we want to invoke,
+            // so invoke as a local symbol since it's almost always shorter.
+            startMacro(macroName, id, macro)
+        } else {
+            // The name is already being used by something else, so invoke using the system macro syntax.
+            userData.stepInEExp(macro)
+        }
+    }
+
     override fun startExpressionGroup() {
         userData.stepInExpressionGroup(options.writeLengthPrefix(ContainerType.EXPRESSION_GROUP, depth + 1))
     }

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

@@ -700,6 +700,34 @@ class IonRawBinaryWriter_1_1 internal constructor(
         hasFieldName = false
     }
 
+    override fun stepInEExp(systemMacro: SystemMacro) {
+        confirm(numAnnotations == 0) { "Cannot annotate an E-Expression" }
+
+        if (currentContainer.type == STRUCT && !hasFieldName) {
+            // This allows the e-expression to be written in field-name position.
+            // TODO: Confirm that this is still in the spec.
+            if (!currentContainer.usesFlexSym) switchCurrentStructToFlexSym()
+            buffer.writeByte(FlexInt.ZERO)
+            currentContainer.length++
+        }
+
+        currentContainer = containerStack.push { it.reset(EEXP, buffer.position(), isLengthPrefixed = false) }
+
+        buffer.writeByte(OpCodes.SYSTEM_MACRO_INVOCATION)
+        buffer.writeByte(systemMacro.id)
+        currentContainer.metadataOffset += 1 // to account for the macro ID.
+
+        val presenceBits = presenceBitmapStack.push { it.initialize(systemMacro.signature) }
+        if (presenceBits.byteSize > 0) {
+            // Reserve for presence bits
+            buffer.reserve(presenceBits.byteSize)
+            currentContainer.length += presenceBits.byteSize
+        }
+
+        // No need to clear any of the annotation fields because we already asserted that there are no annotations
+        hasFieldName = false
+    }
+
     override fun stepInExpressionGroup(usingLengthPrefix: Boolean) {
         confirm(numAnnotations == 0) { "Cannot annotate an expression group" }
         confirm(currentContainer.type == EEXP) { "Can only create an expression group in a macro invocation" }

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

@@ -9,7 +9,7 @@ import com.amazon.ion.impl.macro.ParameterFactory.zeroToManyTagged
 /**
  * Macros that are built in, rather than being defined by a template.
  */
-enum class SystemMacro(val id: Int, val macroName: String, override val signature: List<Macro.Parameter>) : Macro {
+enum class SystemMacro(val id: Byte, val macroName: String, override val signature: List<Macro.Parameter>) : Macro {
     None(0, "none", emptyList()),
     Values(1, "values", listOf(zeroToManyTagged("values"))),
     Annotate(2, "annotate", listOf(zeroToManyTagged("ann"), exactlyOneTagged("value"))),
@@ -21,10 +21,10 @@ enum class SystemMacro(val id: Int, val macroName: String, override val signatur
 
     // 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, "IfNone", listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
-    IfSome(-1, "IfSome", listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
-    IfSingle(-1, "IfSingle", listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
-    IfMulti(-1, "IfMulti", listOf(zeroToManyTagged("stream"), zeroToManyTagged("true_branch"), zeroToManyTagged("false_branch"))),
+    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"))),
     ;
 
     override val dependencies: List<Macro>
@@ -35,7 +35,7 @@ enum class SystemMacro(val id: Int, val macroName: String, override val signatur
         private val MACROS_BY_NAME: Map<String, SystemMacro> = SystemMacro.entries.associateBy { it.macroName }
 
         // TODO: Once all of the macros are implemented, replace this with an array as in SystemSymbols_1_1
-        private val MACROS_BY_ID: Map<Int, SystemMacro> = SystemMacro.entries
+        private val MACROS_BY_ID: Map<Byte, SystemMacro> = SystemMacro.entries
             .filterNot { it.id < 0 }
             .associateBy { it.id }
 
@@ -44,7 +44,7 @@ enum class SystemMacro(val id: Int, val macroName: String, override val signatur
 
         /** Gets a [SystemMacro] by its address in the system table */
         @JvmStatic
-        operator fun get(id: Int): SystemMacro? = MACROS_BY_ID[id]
+        operator fun get(id: Int): SystemMacro? = MACROS_BY_ID[id.toByte()]
 
         /** Gets, by name, a [SystemMacro] with an address in the system table (i.e. that can be invoked as E-Expressions) */
         @JvmStatic

src/test/java/com/amazon/ion/impl/IonRawTextWriterTest_1_1.kt

@@ -14,6 +14,7 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.CsvSource
+import org.junit.jupiter.params.provider.EnumSource
 
 class IonRawTextWriterTest_1_1 {
 
@@ -708,6 +709,15 @@ class IonRawTextWriterTest_1_1 {
         }
     }
 
+    @ParameterizedTest
+    @EnumSource(SystemMacro::class)
+    fun `write system macro E-expression by name`(systemMacro: SystemMacro) {
+        assertWriterOutputEquals("(:\$ion::${systemMacro.macroName})") {
+            stepInEExp(systemMacro)
+            stepOut()
+        }
+    }
+
     @Test
     fun `write E-expression by id`() {
         assertWriterOutputEquals("(:1)") {

src/test/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1_Test.kt

@@ -245,6 +245,83 @@ internal class IonManagedWriter_1_1_Test {
         assertEquals(expected, actual)
     }
 
+    @Test
+    fun `when a system macro is shadowed, it should be written using the system e-exp syntax`() {
+        val expected = """
+            $ion_1_1
+            $ion_encoding::((macro_table (macro make_string () "make")))
+            (:make_string)
+            (:$ion::make_string (: "a" b))
+        """.trimIndent()
+
+        // Makes the word "make" as a string
+        val makeStringShadow = constantMacro {
+            string("make")
+        }
+
+        val actual = write {
+            startMacro("make_string", makeStringShadow)
+            endMacro()
+            startMacro(SystemMacro.MakeString)
+            startExpressionGroup()
+            writeString("a")
+            writeSymbol("b")
+            endExpressionGroup()
+            endMacro()
+        }
+
+        assertEquals(expected, actual)
+    }
+
+    @Test
+    fun `when a system macro is invoked, it should be added to the user table if there is no name conflict`() {
+        val expected = """
+            $ion_1_1
+            $ion_encoding::((macro_table (export $ion::make_string)))
+            (:make_string (: "a" b))
+        """.trimIndent()
+
+        var actual = write {
+            startMacro(SystemMacro.MakeString)
+            startExpressionGroup()
+            writeString("a")
+            writeSymbol("b")
+            endExpressionGroup()
+            endMacro()
+        }
+        assertEquals(expected, actual)
+
+        // And again, but with using the function that accepts the name of the macro
+        actual = write {
+            startMacro("make_string", SystemMacro.MakeString)
+            startExpressionGroup()
+            writeString("a")
+            writeSymbol("b")
+            endExpressionGroup()
+            endMacro()
+        }
+        assertEquals(expected, actual)
+    }
+
+    @Test
+    fun `it is possible to invoke a system macro using an alias`() {
+        val expected = """
+            $ion_1_1
+            $ion_encoding::((macro_table (export $ion::make_string foo)))
+            (:foo (: "a" b))
+        """.trimIndent()
+
+        val actual = write {
+            startMacro("foo", SystemMacro.MakeString)
+            startExpressionGroup()
+            writeString("a")
+            writeSymbol("b")
+            endExpressionGroup()
+            endMacro()
+        }
+        assertEquals(expected, actual)
+    }
+
     @Test
     fun `write an encoding directive with a non-empty symbol table`() {
         val expected = """

src/test/java/com/amazon/ion/impl/bin/IonRawBinaryWriterTest_1_1.kt

@@ -17,6 +17,7 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.CsvSource
+import org.junit.jupiter.params.provider.EnumSource
 
 class IonRawBinaryWriterTest_1_1 {
 
@@ -1119,6 +1120,22 @@ class IonRawBinaryWriterTest_1_1 {
         }
     }
 
+    @ParameterizedTest
+    @EnumSource(SystemMacro::class)
+    fun `write a system macro e-expression`(systemMacro: SystemMacro) {
+        val numVariadicParameters = systemMacro.signature.count { it.cardinality != ParameterCardinality.ExactlyOne }
+        val signatureBytes = when (numVariadicParameters) {
+            0 -> ""
+            1, 2, 3, 4 -> "00"
+            5, 6, 7, 8 -> "00 00"
+            else -> TODO("There are definitely no system macros with more than 8 variadic parameters")
+        }
+        assertWriterOutputEquals(String.format("EF %02X $signatureBytes", systemMacro.id)) {
+            stepInEExp(systemMacro)
+            stepOut()
+        }
+    }
+
     @Test
     fun `write a delimited e-expression`() {
         assertWriterOutputEquals("00") {