From 2b260b8ae1b0a730cab868b69f8844419b90d555 Mon Sep 17 00:00:00 2001 From: Matthew Pope Date: Wed, 2 Oct 2024 17:03:53 -0700 Subject: [PATCH] Updates TDL syntax for macro invocations, expression groups, and variable expansions --- .../amazon/ion/impl/IonRawTextWriter_1_1.kt | 35 ++++- .../ion/impl/PrivateIonRawWriter_1_1.kt | 20 +++ .../ion/impl/bin/IonManagedWriter_1_1.kt | 101 +++----------- .../ion/impl/bin/IonRawBinaryWriter_1_1.kt | 25 ++++ .../ion/impl/bin/Ion_1_1_Constants.java | 4 + .../amazon/ion/impl/macro/MacroCompiler.kt | 129 ++++++++++-------- .../EncodingDirectiveCompilationTest.java | 29 +++- .../ion/impl/bin/IonManagedWriter_1_1_Test.kt | 58 +++++--- .../ion/impl/macro/MacroCompilerTest.kt | 44 +++--- 9 files changed, 261 insertions(+), 184 deletions(-) diff --git a/src/main/java/com/amazon/ion/impl/IonRawTextWriter_1_1.kt b/src/main/java/com/amazon/ion/impl/IonRawTextWriter_1_1.kt index 040e53986..bc3f1e4ae 100644 --- a/src/main/java/com/amazon/ion/impl/IonRawTextWriter_1_1.kt +++ b/src/main/java/com/amazon/ion/impl/IonRawTextWriter_1_1.kt @@ -327,7 +327,11 @@ class IonRawTextWriter_1_1 internal constructor( } override fun stepInSExp(usingLengthPrefix: Boolean) { - openValue { output.appendAscii("(") } + startSexp { output.appendAscii("(") } + } + + private inline fun startSexp(openingTokens: () -> Unit) { + openValue(openingTokens) ancestorContainersStack.add(currentContainer) currentContainer = SExp currentContainerHasValues = false @@ -404,4 +408,33 @@ class IonRawTextWriter_1_1 internal constructor( override fun writeMacroParameterCardinality(cardinality: Macro.ParameterCardinality) { output.appendAscii(cardinality.sigil) } + + override fun stepInTdlExpressionGroup() { + startSexp { output.appendAscii("(..") } + isPendingSeparator = true + } + + override fun stepInTdlMacroInvocation(macroRef: Int) { + startSexp { + output.appendAscii("(.") + output.printInt(macroRef.toLong()) + } + isPendingSeparator = true + } + + override fun stepInTdlMacroInvocation(macroRef: String) { + startSexp { + output.appendAscii("(.") + output.appendAscii(macroRef) + } + isPendingSeparator = true + } + + override fun writeTdlVariableExpansion(variableName: String) { + writeScalar { + output.appendAscii("(%") + output.appendAscii(variableName) + output.appendAscii(")") + } + } } diff --git a/src/main/java/com/amazon/ion/impl/PrivateIonRawWriter_1_1.kt b/src/main/java/com/amazon/ion/impl/PrivateIonRawWriter_1_1.kt index fd75926ec..37a2c159d 100644 --- a/src/main/java/com/amazon/ion/impl/PrivateIonRawWriter_1_1.kt +++ b/src/main/java/com/amazon/ion/impl/PrivateIonRawWriter_1_1.kt @@ -17,6 +17,26 @@ internal interface PrivateIonRawWriter_1_1 : IonRawWriter_1_1 { */ fun writeMacroParameterCardinality(cardinality: Macro.ParameterCardinality) + /** + * Steps into a TDL Macro Invocation—an s-expression, followed by `.` and then the macro address + */ + fun stepInTdlMacroInvocation(macroRef: Int) + + /** + * Steps into a TDL Macro Invocation—an s-expression, followed by `.` and then the macro name + */ + fun stepInTdlMacroInvocation(macroRef: String) + + /** + * Steps in s-expression, writes `%` symbol, variable name, and then closes the s-expression. + */ + fun writeTdlVariableExpansion(variableName: String) + + /** + * Steps in s-expression and writes `..` symbol. + */ + fun stepInTdlExpressionGroup() + /** * Sets a flag that can override the newlines that are normally inserted by a pretty printer. * 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 8b0e3be78..4d8b9440f 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 @@ -42,8 +42,6 @@ internal class IonManagedWriter_1_1( companion object { private val ION_VERSION_MARKER_REGEX = Regex("^\\\$ion_\\d+_\\d+$") - private const val TDL_EXPRESSION_GROUP_START = ";" - // These are chosen subjectively to be neither too big nor too small. private const val MAX_PARAMETERS_IN_ONE_LINE_SIGNATURE = 4 private const val MAX_SYMBOLS_IN_SINGLE_LINE_SYMBOL_TABLE = 10 @@ -403,17 +401,13 @@ internal class IonManagedWriter_1_1( when (expression) { is Expression.DataModelValue -> { - // If we need to write a sexp, we must use the annotate macro. - // If we're writing a symbol, we can put them inside the "literal" special form. - if (expression.type != IonType.SEXP && expression.type != IonType.SYMBOL) { - expression.annotations.forEach { - if (it.text != null) { - // TODO: If it's already in the symbol table we could check the - // symbol-inline strategy and possibly write a SID. - writeAnnotations(it.text) - } else { - writeAnnotations(it.sid) - } + expression.annotations.forEach { + if (it.text != null) { + // TODO: If it's already in the symbol table we could check the + // symbol-inline strategy and possibly write a SID. + writeAnnotations(it.text) + } else { + writeAnnotations(it.sid) } } @@ -432,25 +426,13 @@ internal class IonManagedWriter_1_1( IonType.DECIMAL -> writeDecimal((expression as Expression.DecimalValue).value) IonType.TIMESTAMP -> writeTimestamp((expression as Expression.TimestampValue).value) IonType.SYMBOL -> { - writeSystemSexp { - writeSymbol(SystemSymbols_1_1.LITERAL) - expression.annotations.forEach { - if (it.text != null) { - // TODO: If it's already in the symbol table we could check the - // symbol-inline strategy and possibly write a SID. - writeAnnotations(it.text) - } else { - writeAnnotations(it.sid) - } - } - val symbolToken = (expression as Expression.SymbolValue).value - if (symbolToken.text != null) { - // TODO: If it's already in the symbol table we could check the - // symbol-inline strategy and possibly write a SID. - writeSymbol(symbolToken.text) - } else { - writeSymbol(symbolToken.sid) - } + val symbolToken = (expression as Expression.SymbolValue).value + if (symbolToken.text != null) { + // TODO: If it's already in the symbol table we could check the + // symbol-inline strategy and possibly write a SID. + writeSymbol(symbolToken.text) + } else { + writeSymbol(symbolToken.sid) } } IonType.STRING -> writeString((expression as Expression.StringValue).value) @@ -463,41 +445,8 @@ internal class IonManagedWriter_1_1( } IonType.SEXP -> { expression as Expression.HasStartAndEnd - if (expression.annotations.isNotEmpty()) { - stepInSExp(usingLengthPrefix = false) - numberOfTimesToStepOut[expression.endExclusive]++ - writeSymbol(SystemSymbols_1_1.ANNOTATE) - - // Write the annotations as symbols within an expression group - writeSystemSexp { - writeSymbol(TDL_EXPRESSION_GROUP_START) - expression.annotations.forEach { - if (it.text != null) { - // TODO: If it's already in the symbol table we could check the - // symbol-inline strategy and possibly write a SID. - - // Write the annotation as a string so that we don't have to use - // `literal` to prevent it from being interpreted as a variable - writeString(it.text) - } else { - // TODO: See if there is a less verbose way to use SIDs in TDL - writeSystemSexp { - writeSymbol(SystemSymbols_1_1.LITERAL) - writeSymbol(it.sid) - } - } - } - } - } - // Start a `(make_sexp [ ...` invocation stepInSExp(usingLengthPrefix = false) numberOfTimesToStepOut[expression.endExclusive]++ - writeSymbol(SystemSymbols_1_1.MAKE_SEXP) - - if (expression.startInclusive != expression.endExclusive) { - stepInList(usingLengthPrefix = false) - numberOfTimesToStepOut[expression.endExclusive]++ - } } IonType.STRUCT -> { expression as Expression.HasStartAndEnd @@ -517,28 +466,22 @@ internal class IonManagedWriter_1_1( } } is Expression.ExpressionGroup -> { - stepInSExp(usingLengthPrefix = false) + stepInTdlExpressionGroup() numberOfTimesToStepOut[expression.endExclusive]++ - writeSymbol(TDL_EXPRESSION_GROUP_START) } is Expression.MacroInvocation -> { - stepInSExp(usingLengthPrefix = false) - numberOfTimesToStepOut[expression.endExclusive]++ - val invokedAddress = macroTable[expression.macro] ?: newMacros[expression.macro] ?: throw IllegalStateException("A macro in the macro table is missing a dependency") - - if (options.invokeTdlMacrosByName) { - val invokedName = macroNames[invokedAddress] - if (invokedName != null) { - writeSymbol(invokedName) - return@forEachIndexed - } + val invokedName = macroNames[invokedAddress] + if (options.invokeTdlMacrosByName && invokedName != null) { + stepInTdlMacroInvocation(invokedName) + } else { + stepInTdlMacroInvocation(invokedAddress) } - writeInt(invokedAddress.toLong()) + numberOfTimesToStepOut[expression.endExclusive]++ } - is Expression.VariableRef -> writeSymbol(macro.signature[expression.signatureIndex].variableName) + is Expression.VariableRef -> writeTdlVariableExpansion(macro.signature[expression.signatureIndex].variableName) else -> error("Unreachable") } } diff --git a/src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter_1_1.kt b/src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter_1_1.kt index b0b73d6ab..34bb8503c 100644 --- a/src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter_1_1.kt +++ b/src/main/java/com/amazon/ion/impl/bin/IonRawBinaryWriter_1_1.kt @@ -917,4 +917,29 @@ class IonRawBinaryWriter_1_1 internal constructor( // TODO: Write as a system symbol writeSymbol(cardinality.sigil.toString()) } + + override fun stepInTdlMacroInvocation(macroRef: Int) { + stepInSExp(usingLengthPrefix = false) + writeSymbol(".") + writeInt(macroRef.toLong()) + } + + override fun stepInTdlMacroInvocation(macroRef: String) { + stepInSExp(usingLengthPrefix = false) + writeSymbol(".") + writeSymbol(macroRef) + } + + override fun writeTdlVariableExpansion(variableName: String) { + stepInSExp(usingLengthPrefix = false) + writeSymbol("%") + writeSymbol(variableName) + stepOut() + } + + override fun stepInTdlExpressionGroup() { + stepInSExp(usingLengthPrefix = false) + // TODO: Write as a system symbol + writeSymbol("..") + } } diff --git a/src/main/java/com/amazon/ion/impl/bin/Ion_1_1_Constants.java b/src/main/java/com/amazon/ion/impl/bin/Ion_1_1_Constants.java index 5c233dafc..14305e383 100644 --- a/src/main/java/com/amazon/ion/impl/bin/Ion_1_1_Constants.java +++ b/src/main/java/com/amazon/ion/impl/bin/Ion_1_1_Constants.java @@ -12,6 +12,10 @@ public class Ion_1_1_Constants { private Ion_1_1_Constants() {} + public static final String TDL_MACRO_INVOCATION_SIGIL = "."; + public static final String TDL_EXPRESSION_GROUP_SIGIL = ".."; + public static final String TDL_VARIABLE_EXPANSION_SIGIL = "%"; + // When writing system symbols (or $0) in a flex sym, the SID must be offset to // avoid clashing with E-Expression op codes. public static final int FLEX_SYM_SYSTEM_SYMBOL_OFFSET = 0x60; diff --git a/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt b/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt index a8577d12c..055300930 100644 --- a/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt +++ b/src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt @@ -4,6 +4,7 @@ package com.amazon.ion.impl.macro 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 java.math.BigDecimal @@ -48,7 +49,7 @@ abstract class MacroCompiler( confirmNoAnnotations("macro signature") readSignature() confirm(nextValue()) { "Macro definition is missing a template body expression." } - compileTemplateBodyExpression(isQuoted = false) + compileTemplateBodyExpression() confirm(!nextValue()) { "Unexpected ${encodingType()} after template body expression." } } return TemplateMacro(signature.toList(), expressions.toList()) @@ -112,7 +113,7 @@ abstract class MacroCompiler( * * If called when the reader is not positioned on any value, throws [IllegalStateException]. */ - private fun compileTemplateBodyExpression(isQuoted: Boolean) { + private fun compileTemplateBodyExpression() { // NOTE: `toList()` does not allocate for an empty list. val annotations: List = getTypeAnnotationSymbols() @@ -133,27 +134,10 @@ abstract class MacroCompiler( IonType.STRING -> expressions.add(StringValue(annotations, stringValue())) IonType.BLOB -> expressions.add(BlobValue(annotations, newBytes())) IonType.CLOB -> expressions.add(ClobValue(annotations, newBytes())) - IonType.SYMBOL -> { - if (isQuoted) { - expressions.add(SymbolValue(annotations, symbolValue())) - } else { - val name = stringValue() - confirmNoAnnotations("on variable reference '$name'") - val index = signature.indexOfFirst { it.variableName == name } - confirm(index >= 0) { "variable '$name' is not recognized" } - expressions.add(VariableRef(index)) - } - } - IonType.LIST -> compileSequence(isQuoted) { start, end -> ListValue(annotations, start, end) } - IonType.SEXP -> { - if (isQuoted) { - compileSequence(isQuoted = true) { start, end -> SExpValue(annotations, start, end) } - } else { - confirmNoAnnotations(location = "a macro invocation") - compileMacroInvocation() - } - } - IonType.STRUCT -> compileStruct(annotations, isQuoted) + IonType.SYMBOL -> expressions.add(SymbolValue(annotations, symbolValue())) + IonType.LIST -> compileList(annotations) + IonType.SEXP -> compileSExpression(annotations) + IonType.STRUCT -> compileStruct(annotations) // IonType.NULL, IonType.DATAGRAM, null else -> throw IllegalStateException("Found ${encodingType()}; this should be unreachable.") } @@ -165,7 +149,7 @@ abstract class MacroCompiler( * If this function returns normally, it will be stepped out of the struct. * Caller will need to call [IonReader.next] to get the next value. */ - private fun compileStruct(annotations: List, isQuoted: Boolean) { + private fun compileStruct(annotations: List) { val start = expressions.size expressions.add(Placeholder) val templateStructIndex = mutableMapOf>() @@ -177,7 +161,7 @@ abstract class MacroCompiler( // Default is an array list with capacity of 1, since the most common case is that a field name occurs once. templateStructIndex.getOrPut(it) { ArrayList(1) } += valueIndex } - compileTemplateBodyExpression(isQuoted) + compileTemplateBodyExpression() } val end = expressions.size expressions[start] = StructValue(annotations, start, end, templateStructIndex) @@ -189,58 +173,89 @@ abstract class MacroCompiler( * If this function returns normally, it will be stepped out of the sequence. * Caller will need to call [IonReader.next] to get the next value. */ - private inline fun compileSequence(isQuoted: Boolean, newTemplateBodySequence: (Int, Int) -> TemplateBodyExpression) { - val seqStart = expressions.size + private fun compileList(annotations: List) { + val start = expressions.size + stepIntoContainer() expressions.add(Placeholder) - forEachInContainer { compileTemplateBodyExpression(isQuoted) } - val seqEnd = expressions.size - expressions[seqStart] = newTemplateBodySequence(seqStart, seqEnd) + compileExpressionTail(start) { end -> ListValue(annotations, start, end) } } /** - * Compiles a macro invocation in a macro template. + * Compiles an unclassified S-Expression in a template body expression. * When calling, the reader should be positioned at the sexp, but not stepped into it. * If this function returns normally, it will be stepped out of the sexp. * Caller will need to call [IonReader.next] to get the next value. */ - private fun compileMacroInvocation() { + private fun compileSExpression(sexpAnnotations: List) { + val start = expressions.size stepIntoContainer() - nextValue() - val macroRef = when (encodingType()) { - IonType.SYMBOL -> { - val macroName = stringValue() - // TODO: Come up with a consistent strategy for handling special forms. - when (macroName) { - "literal" -> { - // It's the "literal" special form; skip compiling a macro invocation and just treat all contents as literals - forEachRemaining { compileTemplateBodyExpression(isQuoted = true) } - stepOutOfContainer() + expressions.add(Placeholder) + if (nextValue()) { + if (encodingType() == IonType.SYMBOL) { + when (stringValue()) { + TDL_VARIABLE_EXPANSION_SIGIL -> { + confirm(sexpAnnotations.isEmpty()) { "Variable expansion may not be annotated" } + confirmNoAnnotations("Variable expansion operator") + compileVariableExpansion(start) return } - ";" -> { - val macroStart = expressions.size - expressions.add(Placeholder) - forEachRemaining { compileTemplateBodyExpression(isQuoted = false) } - val macroEnd = expressions.size - expressions[macroStart] = ExpressionGroup(macroStart, macroEnd) - stepOutOfContainer() + + TDL_EXPRESSION_GROUP_SIGIL -> { + confirm(sexpAnnotations.isEmpty()) { "Expression group may not be annotated" } + confirmNoAnnotations("Expression group operator") + compileExpressionTail(start) { end -> ExpressionGroup(start, end) } + return + } + + TDL_MACRO_INVOCATION_SIGIL -> { + confirm(sexpAnnotations.isEmpty()) { "Macro invocation may not be annotated" } + confirmNoAnnotations("Macro invocation operator") + nextValue() + val macro = readMacroReference() + compileExpressionTail(start) { end -> MacroInvocation(macro, start, end) } return } - else -> MacroRef.ByName(macroName) } } + // Compile the value we're already positioned on before compiling the rest of the s-expression + compileTemplateBodyExpression() + } + compileExpressionTail(start) { end -> SExpValue(sexpAnnotations, start, end) } + } + + /** + * Must be positioned on the (expected) macro reference. + */ + private fun readMacroReference(): Macro { + 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()) 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 macro = getMacro(macroRef) ?: throw IonException("Unrecognized macro: $macroRef") - - val macroStart = expressions.size - expressions.add(Placeholder) - forEachRemaining { compileTemplateBodyExpression(isQuoted = false) } - val macroEnd = expressions.size - expressions[macroStart] = MacroInvocation(macro, macroStart, macroEnd) + private fun compileVariableExpansion(placeholderIndex: Int) { + nextValue() + confirm(encodingType() == IonType.SYMBOL) { "Variable names must be symbols" } + val name = stringValue() + confirmNoAnnotations("on variable reference '$name'") + val index = signature.indexOfFirst { it.variableName == name } + confirm(index >= 0) { "variable '$name' is not recognized" } + expressions[placeholderIndex] = VariableRef(index) + confirm(!nextValue()) { "Variable expansion should contain only the variable name." } + stepOutOfContainer() + } + private inline fun compileExpressionTail(seqStart: Int, constructor: (Int) -> TemplateBodyExpression) { + forEachRemaining { compileTemplateBodyExpression() } + val seqEnd = expressions.size + expressions[seqStart] = constructor(seqEnd) stepOutOfContainer() } diff --git a/src/test/java/com/amazon/ion/impl/EncodingDirectiveCompilationTest.java b/src/test/java/com/amazon/ion/impl/EncodingDirectiveCompilationTest.java index 7951a7913..88991956e 100644 --- a/src/test/java/com/amazon/ion/impl/EncodingDirectiveCompilationTest.java +++ b/src/test/java/com/amazon/ion/impl/EncodingDirectiveCompilationTest.java @@ -30,6 +30,8 @@ /** * Tests that Ion 1.1 encoding directives are correctly compiled from streams of Ion data. + * + * Don't attempt fix this without ensuring that MacroCompilerTest and IonManagesWriter_1_1_Test are working first. */ public class EncodingDirectiveCompilationTest { @@ -108,9 +110,22 @@ private static void writeMacroSignature(IonRawWriter_1_1 writer, Map symbols, String fieldName, String variableName) { writer.writeFieldName(symbols.get(fieldName)); - writer.writeSymbol(symbols.get(variableName)); + writeVariableExpansion(writer, symbols.get(variableName)); } private static byte[] getBytes(IonRawWriter_1_1 writer, ByteArrayOutputStream out) { @@ -475,7 +490,8 @@ private Macro writeSimonSaysMacro(IonRawWriter_1_1 writer) { startMacroTable(writer); startMacro(writer, symbols, "SimonSays"); writeMacroSignature(writer, symbols, "anything"); - writer.writeSymbol(symbols.get("anything")); // The body: a variable + // The body + writeVariableExpansion(writer, symbols.get("anything")); endMacro(writer); endMacroTable(writer); endEncodingDirective(writer); @@ -644,10 +660,10 @@ public void twoArgumentGroups() throws Exception { writeMacroSignature(writer, symbols, "these", "*", "those", "+"); writer.stepInList(true); writer.stepInList(true); - writer.writeSymbol(symbols.get("those")); // The body: a variable + writeVariableExpansion(writer, symbols.get("those")); writer.stepOut(); writer.stepInList(true); - writer.writeSymbol(symbols.get("these")); + writeVariableExpansion(writer, symbols.get("these")); writer.stepOut(); writer.stepOut(); endMacro(writer); @@ -714,12 +730,11 @@ public void macroInvocationInMacroDefinition() throws Exception { startMacroTable(writer); startMacro(writer, symbols, "SimonSays"); writeMacroSignature(writer, symbols, "anything"); - writer.writeSymbol(symbols.get("anything")); // The body: a variable + writeVariableExpansion(writer, symbols.get("anything")); // The body: a variable endMacro(writer); startMacro(writer, symbols, "Echo"); writeMacroSignature(writer, symbols); // empty signature - writer.stepInSExp(true); // A macro invocation in TDL - writer.writeInt(0); // Macro ID 0 ("SimonSays") + stepInTdlMacroInvocation(writer, 0); // Macro ID 0 ("SimonSays") writer.writeInt(123); // The argument to SimonSays writer.stepOut(); endMacro(writer); diff --git a/src/test/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1_Test.kt b/src/test/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1_Test.kt index 0b148e09b..52c642c59 100644 --- a/src/test/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1_Test.kt +++ b/src/test/java/com/amazon/ion/impl/bin/IonManagedWriter_1_1_Test.kt @@ -493,26 +493,26 @@ internal class IonManagedWriter_1_1_Test { case( "symbol", body = { symbol(FakeSymbolToken("foo", -1)) }, - expectedBody = "(literal foo)" + expectedBody = "foo" ), case( "unknown symbol", body = { symbol(FakeSymbolToken(null, 0)) }, - expectedBody = "(literal $0)" + expectedBody = "$0" ), case( "annotated symbol", body = { annotated(listOf(fooSymbolToken), ::symbol, barSymbolToken) }, - expectedBody = "(literal foo::bar)" + expectedBody = "foo::bar" ), case( "symbol annotated with $0", body = { annotated(listOf(FakeSymbolToken(null, 0)), ::symbol, barSymbolToken) }, - expectedBody = "(literal $0::bar)" + expectedBody = "$0::bar" ), case( "string", @@ -537,22 +537,22 @@ internal class IonManagedWriter_1_1_Test { case( "sexp", body = { sexp { int(1) } }, - expectedBody = "(make_sexp [1])" + expectedBody = "(1)" ), case( "empty sexp", body = { sexp { } }, - expectedBody = "(make_sexp)" + expectedBody = "()" ), case( "annotated sexp", body = { annotated(listOf(fooSymbolToken), ::sexp) { int(1) } }, - expectedBody = "(annotate (; \"foo\") (make_sexp [1]))" + expectedBody = "foo::(1)" ), case( "sexp with $0 annotation", body = { annotated(listOf(FakeSymbolToken(null, 0)), ::sexp) { int(1) } }, - expectedBody = "(annotate (; (literal $0)) (make_sexp [1]))" + expectedBody = "$0::(1)" ), case( "struct", @@ -567,22 +567,22 @@ internal class IonManagedWriter_1_1_Test { case( "macro invoked by id", body = { macro(barMacro) {} }, - expectedBody = "(1)" + expectedBody = "(.1)" ), case( "macro invoked by name", body = { macro(fooMacro) {} }, - expectedBody = "(foo)" + expectedBody = "(.foo)" ), case( "macro with an argument", body = { macro(fooMacro) { int(1) } }, - expectedBody = "(foo 1)" + expectedBody = "(.foo 1)" ), case( "macro with an empty argument group", body = { macro(fooMacro) { expressionGroup { } } }, - expectedBody = "(foo (;))" + expectedBody = "(.foo (..))" ), case( "macro with a non-empty argument group", @@ -595,7 +595,7 @@ internal class IonManagedWriter_1_1_Test { } } }, - expectedBody = "(foo (; 1 2 3))" + expectedBody = "(.foo (.. 1 2 3))" ), case( "variable", @@ -604,7 +604,7 @@ internal class IonManagedWriter_1_1_Test { body = { variable(0) }, - expectedBody = "x" + expectedBody = "(%x)" ), case( "multiple variables", @@ -617,7 +617,7 @@ internal class IonManagedWriter_1_1_Test { variable(2) } }, - expectedBody = "[x,y,z]" + expectedBody = "[(%x),(%y),(%z)]" ), case( "nested expressions in body", @@ -630,7 +630,7 @@ internal class IonManagedWriter_1_1_Test { } } }, - expectedBody = "[(make_sexp [1]),{foo:2}]" + expectedBody = "[(1),{foo:2}]" ), ) @@ -664,7 +664,7 @@ internal class IonManagedWriter_1_1_Test { (symbol_table ["foo","bar","baz"]) (macro_table (macro null () "foo") - (macro null () "bar")) + (macro null (x) (.0 (%x) "bar" (..) (.. "baz")))) ) $1 $2 @@ -685,13 +685,29 @@ internal class IonManagedWriter_1_1_Test { (:2) """.trimIndent() + val fooMacro = constantMacro { string("foo") } + val actual = write(symbolInliningStrategy = SymbolInliningStrategy.NEVER_INLINE, pretty = true) { writeSymbol("foo") writeSymbol("bar") writeSymbol("baz") - startMacro(constantMacro { string("foo") }) + startMacro(fooMacro) endMacro() - startMacro(constantMacro { string("bar") }) + startMacro( + TemplateMacro( + listOf(Parameter.exactlyOneTagged("x")), + templateBody { + macro(fooMacro) { + variable(0) + string("bar") + expressionGroup { } + expressionGroup { + string("baz") + } + } + } + ) + ) endMacro() flush() writeSymbol("a") @@ -708,7 +724,7 @@ internal class IonManagedWriter_1_1_Test { fun `writeObject() should write something with a macro representation`() { val expected = """ $ion_1_1 - $ion_encoding::((macro_table (macro Point2D (x y) {x:x,y:y}))) + $ion_encoding::((macro_table (macro Point2D (x y) {x:(%x),y:(%y)}))) (:Point2D 2 4) """.trimIndent() @@ -743,7 +759,7 @@ internal class IonManagedWriter_1_1_Test { fun `writeObject() should write something with nested macro representation`() { val expected = """ $ion_1_1 - $ion_encoding::((macro_table (macro null (x*) x) (macro Polygon (vertices+ compact_symbol::fill?) {vertices:[vertices],fill:(0 fill)}) (macro Point2D (x y) {x:x,y:y}))) + $ion_encoding::((macro_table (macro null (x*) (%x)) (macro Polygon (vertices+ compact_symbol::fill?) {vertices:[(%vertices)],fill:(.0 (%fill))}) (macro Point2D (x y) {x:(%x),y:(%y)}))) (:Polygon (: (:Point2D 0 0) (:Point2D 0 1) (:Point2D 1 1) (:Point2D 1 0)) Blue) """.trimIndent() diff --git a/src/test/java/com/amazon/ion/impl/macro/MacroCompilerTest.kt b/src/test/java/com/amazon/ion/impl/macro/MacroCompilerTest.kt index 61dde0174..1ac9f5b03 100644 --- a/src/test/java/com/amazon/ion/impl/macro/MacroCompilerTest.kt +++ b/src/test/java/com/amazon/ion/impl/macro/MacroCompilerTest.kt @@ -44,11 +44,11 @@ class MacroCompilerTest { private infix fun String.shouldCompileTo(macro: TemplateMacro) = MacroSourceAndTemplate(this, macro) private fun testCases() = listOf( - "(macro identity (x) x)" shouldCompileTo TemplateMacro( + "(macro identity (x) (%x))" shouldCompileTo TemplateMacro( listOf(Parameter("x", Tagged, ParameterCardinality.ExactlyOne)), listOf(VariableRef(0)), ), - "(macro identity (any::x) x)" shouldCompileTo TemplateMacro( + "(macro identity (any::x) (%x))" shouldCompileTo TemplateMacro( listOf(Parameter("x", Tagged, ParameterCardinality.ExactlyOne)), listOf(VariableRef(0)), ), @@ -56,26 +56,26 @@ class MacroCompilerTest { signature = emptyList(), body = listOf(DecimalValue(emptyList(), BigDecimal("3.141592653589793"))) ), - "(macro cardinality_test (x?) x)" shouldCompileTo TemplateMacro( + "(macro cardinality_test (x?) (%x))" shouldCompileTo TemplateMacro( signature = listOf(Parameter("x", Tagged, ParameterCardinality.ZeroOrOne)), body = listOf(VariableRef(0)) ), - "(macro cardinality_test (x!) x)" shouldCompileTo TemplateMacro( + "(macro cardinality_test (x!) (%x))" shouldCompileTo TemplateMacro( signature = listOf(Parameter("x", Tagged, ParameterCardinality.ExactlyOne)), body = listOf(VariableRef(0)) ), - "(macro cardinality_test (x+) x)" shouldCompileTo TemplateMacro( + "(macro cardinality_test (x+) (%x))" shouldCompileTo TemplateMacro( signature = listOf(Parameter("x", Tagged, ParameterCardinality.OneOrMore)), body = listOf(VariableRef(0)) ), - "(macro cardinality_test (x*) x)" shouldCompileTo TemplateMacro( + "(macro cardinality_test (x*) (%x))" shouldCompileTo TemplateMacro( signature = listOf(Parameter("x", Tagged, ParameterCardinality.ZeroOrMore)), body = listOf(VariableRef(0)) ), - // Outer 'values' call allows multiple expressions in the body - // The second `values` is a macro call that has a single argument: the variable `x` - // The `literal` call causes the third (inner) `(values x)` to be an uninterpreted s-expression. - """(macro literal_test (x) (values (values x) (literal (values x))))""" shouldCompileTo TemplateMacro( + // Outer '.values' call allows multiple expressions in the body + // The second `.values` is a macro call that has a single argument: the variable `x` + // The third `(values x)` is an uninterpreted s-expression. + """(macro literal_test (x) (.values (.values (%x)) (values x)))""" shouldCompileTo TemplateMacro( signature = listOf(Parameter("x", Tagged, ParameterCardinality.ExactlyOne)), body = listOf( MacroInvocation(SystemMacro.Values, selfIndex = 0, endExclusive = 6), @@ -86,7 +86,7 @@ class MacroCompilerTest { SymbolValue(emptyList(), FakeSymbolToken("x", -1)), ), ), - "(macro each_type () (values null true 1 ${"9".repeat(50)} 1e0 1d0 2024-01-16T \"foo\" (literal bar) [] (literal ()) {} {{}} {{\"\"}} ))" shouldCompileTo TemplateMacro( + "(macro each_type () (.values null true 1 ${"9".repeat(50)} 1e0 1d0 2024-01-16T \"foo\" bar [] () {} {{}} {{\"\"}} ))" shouldCompileTo TemplateMacro( signature = emptyList(), body = listOf( MacroInvocation(SystemMacro.Values, 0, 15), @@ -106,7 +106,7 @@ class MacroCompilerTest { ClobValue(emptyList(), ByteArray(0)) ) ), - """(macro foo () (values 42 "hello" false))""" shouldCompileTo TemplateMacro( + """(macro foo () (.values 42 "hello" false))""" shouldCompileTo TemplateMacro( signature = emptyList(), body = listOf( MacroInvocation(SystemMacro.Values, selfIndex = 0, endExclusive = 4), @@ -115,7 +115,7 @@ class MacroCompilerTest { BoolValue(emptyList(), false), ) ), - """(macro using_expr_group () (values (; 42 "hello" false)))""" shouldCompileTo TemplateMacro( + """(macro using_expr_group () (.values (.. 42 "hello" false)))""" shouldCompileTo TemplateMacro( signature = emptyList(), body = listOf( MacroInvocation(SystemMacro.Values, selfIndex = 0, endExclusive = 5), @@ -125,7 +125,7 @@ class MacroCompilerTest { BoolValue(emptyList(), false), ) ), - """(macro invoke_by_id () (12 true false))""" shouldCompileTo TemplateMacro( + """(macro invoke_by_id () (.12 true false))""" shouldCompileTo TemplateMacro( signature = emptyList(), body = listOf( MacroInvocation(SystemMacro.Values, selfIndex = 0, endExclusive = 3), @@ -137,7 +137,7 @@ class MacroCompilerTest { signature = emptyList(), body = listOf(StringValue(emptyList(), "abc")) ), - "(macro foo (x y z) [100, [200, a::b::300], x, {y: [true, false, z]}])" shouldCompileTo TemplateMacro( + "(macro foo (x y z) [100, [200, a::b::300], (%x), {y: [true, false, (%z)]}])" shouldCompileTo TemplateMacro( signature = listOf( Parameter("x", Tagged, ParameterCardinality.ExactlyOne), Parameter("y", Tagged, ParameterCardinality.ExactlyOne), @@ -284,10 +284,16 @@ class MacroCompilerTest { // Problems in the body "(macro empty ())", // No body expression - "(macro transform (x) y)", // Unknown variable - "(macro transform (x) foo::x)", // VariableReference cannot be annotated - "(macro transform (x) foo::(literal x))", // Macro invocation cannot be annotated - """(macro transform (x) ("literal" x))""", // Macro invocation must start with a symbol or integer id + "(macro transform (x) (%y))", // Unknown variable + "(macro transform (x) foo::(%x))", // Variable expansion cannot be annotated + "(macro transform (x) (foo::%x))", // Variable expansion operator cannot be annotated + "(macro transform (x) (%foo::x))", // Variable name cannot be annotated + "(macro transform (x) foo::(.values x))", // Macro invocation cannot be annotated + "(macro transform (x) (foo::.values x))", // Macro invocation operator cannot be annotated + "(macro transform (x) (.))", // Macro invocation operator must be followed by macro reference + """(macro transform (x) (."values" x))""", // Macro invocation must start with a symbol or integer id + """(macro transform (x) (.values foo::(..)))""", // Expression group may not be annotated + """(macro transform (x) (.values (foo::..)))""", // Expression group operator may not be annotated "(macro transform (x) 1 2)", // Template body must be one expression )