Various changes in support of system macros

* Add a definition of `default`
* Use qualified module reference for system macros where relevant
* Support use of system macros in TDL
* Updating system symbols to match spec as of now
* Associate SystemMacro values directly with SystemSymbols_1_1 values
* Add SystemSymbol lookup by name

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

@@ -458,6 +458,14 @@ class IonRawTextWriter_1_1 internal constructor(
         isPendingSeparator = true
     }
 
+    override fun stepInTdlSystemMacroInvocation(systemSymbol: SystemSymbols_1_1) {
+        startSexp {
+            output.appendAscii("(.\$ion::")
+            output.appendAscii(systemSymbol.text)
+        }
+        isPendingSeparator = true
+    }
+
     override fun writeTdlVariableExpansion(variableName: String) {
         writeScalar {
             output.appendAscii("(%")

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

@@ -2473,7 +2473,8 @@ protected final SymbolToken getSystemSymbolToken(Marker marker) {
         if (id == 0) {
             return _Private_Utils.SYMBOL_0;
         }
-        return SystemSymbols_1_1.get((int) id).getToken();
+        SystemSymbols_1_1 systemSymbol = SystemSymbols_1_1.get((int) id);
+        return (systemSymbol == null) ? new SymbolTokenImpl(-1) : systemSymbol.getToken();
     }
 
     @Override

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

@@ -27,6 +27,11 @@ internal interface PrivateIonRawWriter_1_1 : IonRawWriter_1_1 {
      */
     fun stepInTdlMacroInvocation(macroRef: String)
 
+    /**
+     * Steps into a TDL System Macro Invocation—an s-expression, followed by `.` and then the qualified macro name
+     */
+    fun stepInTdlSystemMacroInvocation(systemSymbol: SystemSymbols_1_1)
+
     /**
      * Steps in s-expression, writes `%` symbol, variable name, and then closes the s-expression.
      */

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

@@ -24,9 +24,9 @@ enum class SystemSymbols_1_1(val id: Int, val text: String) {
     MACRO_TABLE( /*             */ 14, "macro_table"),
     SYMBOL_TABLE( /*            */ 15, "symbol_table"),
     MODULE( /*                  */ 16, "module"),
-    RETAIN( /*                  */ 17, "retain"),
+    RETAIN( /*                  */ 17, "retain"), // https://github.com/amazon-ion/ion-docs/issues/345
     EXPORT( /*                  */ 18, "export"),
-    CATALOG_KEY( /*             */ 19, "catalog_key"),
+    CATALOG_KEY( /*             */ 19, "catalog_key"), // https://github.com/amazon-ion/ion-docs/issues/345
     IMPORT( /*                  */ 20, "import"),
     THE_EMPTY_SYMBOL( /*        */ 21, ""),
     LITERAL( /*                 */ 22, "literal"),
@@ -35,7 +35,7 @@ enum class SystemSymbols_1_1(val id: Int, val text: String) {
     IF_SINGLE( /*               */ 25, "if_single"),
     IF_MULTI( /*                */ 26, "if_multi"),
     FOR( /*                     */ 27, "for"),
-    FAIL( /*                    */ 28, "fail"),
+    DEFAULT( /*                 */ 28, "default"),
     VALUES( /*                  */ 29, "values"),
     ANNOTATE( /*                */ 30, "annotate"),
     MAKE_STRING( /*             */ 31, "make_string"),
@@ -51,23 +51,28 @@ enum class SystemSymbols_1_1(val id: Int, val text: String) {
     DELTA( /*                   */ 41, "delta"),
     FLATTEN( /*                 */ 42, "flatten"),
     SUM( /*                     */ 43, "sum"),
-    ADD_SYMBOLS( /*             */ 44, "add_symbols"),
-    ADD_MACROS( /*              */ 45, "add_macros"),
-    COMMENT( /*                 */ 46, "comment"),
-    FLEX_SYMBOL( /*             */ 47, "flex_symbol"),
-    FLEX_INT( /*                */ 48, "flex_int"),
-    FLEX_UINT( /*               */ 49, "flex_uint"),
-    UINT8( /*                   */ 50, "uint8"),
-    UINT16( /*                  */ 51, "uint16"),
-    UINT32( /*                  */ 52, "uint32"),
-    UINT64( /*                  */ 53, "uint64"),
-    INT8( /*                    */ 54, "int8"),
-    INT16( /*                   */ 55, "int16"),
-    INT32( /*                   */ 56, "int32"),
-    INT64( /*                   */ 57, "int64"),
-    FLOAT16( /*                 */ 58, "float16"),
-    FLOAT32( /*                 */ 59, "float32"),
-    FLOAT64( /*                 */ 60, "float64"),
+    SET_SYMBOLS( /*             */ 44, "set_symbols"),
+    ADD_SYMBOLS( /*             */ 45, "add_symbols"),
+    SET_MACROS( /*              */ 46, "set_macros"),
+    ADD_MACROS( /*              */ 47, "add_macros"),
+    USE( /*                     */ 48, "use"),
+    META( /*                    */ 49, "meta"),
+    FLEX_SYMBOL( /*             */ 50, "flex_symbol"),
+    FLEX_INT( /*                */ 51, "flex_int"),
+    FLEX_UINT( /*               */ 52, "flex_uint"),
+    UINT8( /*                   */ 53, "uint8"),
+    UINT16( /*                  */ 54, "uint16"),
+    UINT32( /*                  */ 55, "uint32"),
+    UINT64( /*                  */ 56, "uint64"),
+    INT8( /*                    */ 57, "int8"),
+    INT16( /*                   */ 58, "int16"),
+    INT32( /*                   */ 59, "int32"),
+    INT64( /*                   */ 60, "int64"),
+    FLOAT16( /*                 */ 61, "float16"),
+    FLOAT32( /*                 */ 62, "float32"),
+    FLOAT64( /*                 */ 63, "float64"),
+    NONE( /*                    */ 64, "none"),
+    MAKE_FIELD( /*              */ 65, "make_field"),
     ;
 
     val utf8Bytes = text.encodeToByteArray()
@@ -91,6 +96,12 @@ enum class SystemSymbols_1_1(val id: Int, val text: String) {
                 }
         }
 
+        @JvmStatic
+        private val BY_NAME = ALL_VALUES.fold(HashMap<String, SystemSymbols_1_1>(ALL_VALUES.size)) { map, s ->
+            check(map.put(s.text, s) == null) { "Duplicate system symbol text: ${s.id}=${s.text}" }
+            map
+        }
+
         @JvmStatic
         fun size() = ALL_VALUES.size
 
@@ -102,19 +113,26 @@ enum class SystemSymbols_1_1(val id: Int, val text: String) {
         fun allSymbolTexts() = ALL_SYMBOL_TEXTS
 
         /**
-         * Returns true if the [id] is a valid system symbol ID.
-         */
+         * Returns true if the [id] is a valid system symbol ID.*/
         @JvmStatic
         operator fun contains(id: Int): Boolean {
             return id > 0 && id <= SystemSymbols_1_1.ALL_VALUES.size
         }
 
         /**
-         * Returns the text of the given system symbol ID, or null if not a valid system symbol ID.
-         */
+         * Returns the system symbol corresponding to the given system symbol ID,
+         * or `null` if not a valid system symbol ID.*/
+        @JvmStatic
+        operator fun get(id: Int): SystemSymbols_1_1? {
+            return if (contains(id)) { SystemSymbols_1_1.ALL_VALUES[id - 1] } else { null }
+        }
+
+        /**
+         * Returns the system symbol corresponding to the given system symbol text,
+         * or `null` if not a valid system symbol text.*/
         @JvmStatic
-        operator fun get(id: Int): SystemSymbols_1_1 {
-            return SystemSymbols_1_1.ALL_VALUES[id - 1]
+        operator fun get(text: String): SystemSymbols_1_1? {
+            return BY_NAME[text]
         }
     }
 }

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

@@ -163,7 +163,7 @@ internal class IonManagedWriter_1_1(
      */
     private fun addMacroDependencies(macro: Macro) {
         macro.dependencies.forEach {
-            if (it !in macroTable && it !in newMacros) {
+            if (it !is SystemMacro && it !in macroTable && it !in newMacros) {
                 addMacroDependencies(it)
                 assignMacroAddress(it)
             }
@@ -351,7 +351,12 @@ internal class IonManagedWriter_1_1(
                 val name = macroNames[address]
                 when (macro) {
                     is TemplateMacro -> writeMacroDefinition(name, macro)
-                    is SystemMacro -> exportSystemMacro(macro, name)
+                    is SystemMacro -> {
+                        if (name != macro.macroName) {
+                            exportSystemMacro(macro, name)
+                        }
+                        // Else, no need to export the macro since it's already known by the desired name
+                    }
                 }
             }
             forceNoNewlines(true)
@@ -479,8 +484,12 @@ internal class IonManagedWriter_1_1(
                         numberOfTimesToStepOut[expression.endExclusive]++
                     }
                     is Expression.MacroInvocation -> {
-                        val invokedAddress = macroTable[expression.macro]
-                            ?: newMacros[expression.macro]
+                        val invokedMacro = expression.macro
+                        if (invokedMacro is SystemMacro) {
+                            stepInTdlSystemMacroInvocation(invokedMacro.systemSymbol)
+                        }
+                        val invokedAddress = macroTable[invokedMacro]
+                            ?: newMacros[invokedMacro]
                             ?: throw IllegalStateException("A macro in the macro table is missing a dependency")
                         val invokedName = macroNames[invokedAddress]
                         if (options.invokeTdlMacrosByName && invokedName != null) {
@@ -843,23 +852,7 @@ 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)
-        }
-    }
+    private fun startSystemMacro(macro: SystemMacro) = 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

@@ -960,6 +960,13 @@ class IonRawBinaryWriter_1_1 internal constructor(
         writeSymbol(macroRef)
     }
 
+    override fun stepInTdlSystemMacroInvocation(systemSymbol: SystemSymbols_1_1) {
+        stepInSExp(usingLengthPrefix = false)
+        writeSymbol(".")
+        writeAnnotations(SystemSymbols_1_1.ION)
+        writeSymbol(systemSymbol)
+    }
+
     override fun writeTdlVariableExpansion(variableName: String) {
         stepInSExp(usingLengthPrefix = false)
         writeSymbol("%")

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

@@ -6,7 +6,7 @@ import com.amazon.ion.*
 import com.amazon.ion.impl.*
 import com.amazon.ion.impl.bin.Ion_1_1_Constants.*
 import com.amazon.ion.impl.macro.Expression.*
-import com.amazon.ion.util.confirm
+import com.amazon.ion.util.*
 
 /**
  * [MacroCompiler] wraps an Ion reader. When directed to do so, it will take over advancing and getting values from the
@@ -40,6 +40,7 @@ internal class MacroCompiler(
             confirm(reader.encodingType() == IonType.SYMBOL && reader.stringValue() == "macro") { "macro compilation expects a sexp starting with the keyword `macro`" }
 
             reader.nextAndCheckType(IonType.SYMBOL, IonType.NULL, "macro name")
+
             reader.confirmNoAnnotations("macro name")
             if (reader.encodingType() != IonType.NULL) {
                 macroName = reader.stringValue().also { confirm(isIdentifierSymbol(it)) { "invalid macro name: '$it'" } }
@@ -239,17 +240,26 @@ internal class MacroCompiler(
      * Must be positioned on the (expected) macro reference.
      */
     private fun ReaderAdapter.readMacroReference(): Macro {
+
+        val annotations = getTypeAnnotationSymbols()
+        val isSystemMacro = annotations.size == 1 && SystemSymbols_1_1.ION.text == annotations[0].getText()
+
         val macroRef = when (encodingType()) {
             IonType.SYMBOL -> {
                 val macroName = stringValue()
                 // TODO: Come up with a consistent strategy for handling special forms.
                 MacroRef.ByName(macroName)
             }
 
-            IonType.INT -> MacroRef.ById(intValue())
+            IonType.INT -> {
+                val sid = intValue()
+                if (sid < 0) throw IonException("Macro ID must be non-negative: $sid")
+                MacroRef.ById(intValue())
+            }
             else -> throw IonException("macro invocation must start with an id (int) or identifier (symbol); found ${encodingType() ?: "nothing"}\"")
         }
-        return getMacro(macroRef) ?: throw IonException("Unrecognized macro: $macroRef")
+        val m = if (isSystemMacro) SystemMacro.get(macroRef) else getMacro(macroRef)
+        return m ?: throw IonException("Unrecognized macro: $macroRef")
     }
 
     private fun ReaderAdapter.compileVariableExpansion(placeholderIndex: Int) {

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

@@ -14,22 +14,27 @@ 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: Byte, val macroName: String, override val signature: List<Macro.Parameter>, override val body: List<Expression.TemplateBodyExpression>? = null) : Macro {
+enum class SystemMacro(
+    val id: Byte,
+    val systemSymbol: SystemSymbols_1_1,
+    override val signature: List<Macro.Parameter>,
+    override val body: List<Expression.TemplateBodyExpression>? = null
+) : 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(-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"))),
 
     // The real macros
-    None(0, "none", emptyList()),
-    Values(1, "values", listOf(zeroToManyTagged("values"))),
-    Annotate(2, "annotate", listOf(zeroToManyTagged("ann"), exactlyOneTagged("value"))),
-    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"))),
+    None(0, NONE, emptyList()),
+    Values(1, VALUES, listOf(zeroToManyTagged("values"))),
+    Annotate(2, ANNOTATE, listOf(zeroToManyTagged("ann"), exactlyOneTagged("value"))),
+    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"))),
 
     /**
      * ```ion
@@ -41,7 +46,7 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu
      * ```
      */
     SetSymbols(
-        11, "set_symbols", listOf(zeroToManyTagged("symbols")),
+        11, SET_SYMBOLS, listOf(zeroToManyTagged("symbols")),
         templateBody {
             annotated(ION_ENCODING, ::sexp) {
                 sexp {
@@ -66,7 +71,7 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu
      * ```
      */
     AddSymbols(
-        12, "add_symbols", listOf(zeroToManyTagged("symbols")),
+        12, ADD_SYMBOLS, listOf(zeroToManyTagged("symbols")),
         templateBody {
             annotated(ION_ENCODING, ::sexp) {
                 sexp {
@@ -92,7 +97,7 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu
      * ```
      */
     SetMacros(
-        13, "set_macros", listOf(zeroToManyTagged("macros")),
+        13, SET_MACROS, listOf(zeroToManyTagged("macros")),
         templateBody {
             annotated(ION_ENCODING, ::sexp) {
                 sexp {
@@ -117,7 +122,7 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu
      * ```
      */
     AddMacros(
-        14, "add_macros", listOf(zeroToManyTagged("macros")),
+        14, ADD_MACROS, listOf(zeroToManyTagged("macros")),
         templateBody {
             annotated(ION_ENCODING, ::sexp) {
                 sexp {
@@ -144,7 +149,7 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu
      * ```
      */
     Use(
-        15, "use", listOf(exactlyOneTagged("catalog_key"), zeroOrOneTagged("version")),
+        15, USE, listOf(exactlyOneTagged("catalog_key"), zeroOrOneTagged("version")),
         templateBody {
             val theModule = _Private_Utils.newSymbolToken("the_module")
             annotated(ION_ENCODING, ::sexp) {
@@ -172,19 +177,32 @@ enum class SystemMacro(val id: Byte, val macroName: String, override val signatu
         }
     ),
 
-    Repeat(17, "repeat", listOf(exactlyOneTagged("n"), oneToManyTagged("value"))),
+    Repeat(17, REPEAT, listOf(exactlyOneTagged("n"), oneToManyTagged("value"))),
 
-    Comment(21, "comment", listOf(zeroToManyTagged("values")), templateBody { macro(None) {} }),
+    Comment(21, META, listOf(zeroToManyTagged("values")), templateBody { macro(None) {} }),
     MakeField(
-        22, "make_field",
+        22, MAKE_FIELD,
         listOf(
             Macro.Parameter("field_name", Macro.ParameterEncoding.CompactSymbol, Macro.ParameterCardinality.ExactlyOne), exactlyOneTagged("value")
         )
     ),
 
+    Default(
+        23, DEFAULT, listOf(zeroToManyTagged("expr"), zeroToManyTagged("default_expr")),
+        templateBody {
+            macro(IfNone) {
+                variable(0)
+                variable(1)
+                variable(0)
+            }
+        }
+    ),
+
     // TODO: Other system macros
     ;
 
+    val macroName: String get() = this.systemSymbol.text
+
     override val dependencies: List<Macro>
         get() = body
             ?.filterIsInstance<Expression.MacroInvocation>()