Skip to content

Commit

Permalink
Fix macro parameter cardinality and minor todos
Browse files Browse the repository at this point in the history
  • Loading branch information
@popematt
popematt committed Apr 19, 2024
1 parent 4cd5c75 commit 7d0c8cc
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 47 deletions.
22 changes: 20 additions & 2 deletions src/main/java/com/amazon/ion/impl/macro/Macro.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,30 @@ package com.amazon.ion.impl.macro
sealed interface Macro {
val signature: List<Parameter>

data class Parameter(val variableName: String, val type: ParameterEncoding, val grouped: Boolean)
data class Parameter(val variableName: String, val type: ParameterEncoding, val cardinality: ParameterCardinality)

enum class ParameterEncoding(val ionTextName: String) {
Tagged("any"),
// TODO: List all of the possible tagless encodings
}

enum class ParameterCardinality(val sigil: Char) {
AtMostOne('?'),
One('!'),
AtLeastOne('+'),
Any('*');

companion object {
@JvmStatic
fun fromSigil(sigil: String): ParameterCardinality? = when (sigil.singleOrNull()) {
'?' -> AtMostOne
'!' -> One
'+' -> AtLeastOne
'*' -> Any
else -> null
}
}
}
}

/**
Expand Down Expand Up @@ -40,6 +58,6 @@ enum class SystemMacro(override val signature: List<Macro.Parameter>) : Macro {
// TODO: replace these placeholders
Stream(emptyList()), // A stream is technically not a macro, but we can implement it as a macro that is the identity function.
Annotate(emptyList()),
MakeString(listOf(Macro.Parameter("text", Macro.ParameterEncoding.Tagged, grouped = true))),
MakeString(listOf(Macro.Parameter("text", Macro.ParameterEncoding.Tagged, Macro.ParameterCardinality.Any))),

Check warning on line 61 in src/main/java/com/amazon/ion/impl/macro/Macro.kt

View check run for this annotation

Codecov / codecov/patch

src/main/java/com/amazon/ion/impl/macro/Macro.kt#L61 

Added line #L61 was not covered by tests
// TODO: Other system macros
}
83 changes: 54 additions & 29 deletions src/main/java/com/amazon/ion/impl/macro/MacroCompiler.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@
package com.amazon.ion.impl.macro

import com.amazon.ion.*
import com.amazon.ion.impl.*
import com.amazon.ion.impl.macro.TemplateBodyExpression.*
import com.amazon.ion.util.confirm
import com.amazon.ion.util.*

/**
* [MacroCompiler] wraps an [IonReader]. When directed to do so, it will take over advancing and getting values from the
Expand Down Expand Up @@ -41,12 +42,11 @@ class MacroCompiler(private val reader: IonReader) {
reader.readContainer {
confirm(reader.next() == IonType.SYMBOL && reader.stringValue() == "macro") { "macro compilation expects a sexp starting with the keyword `macro`" }

nextAndCheckType(IonType.SYMBOL, "macro name")
nextAndCheckType(IonType.SYMBOL, IonType.NULL, "macro name")
confirmNoAnnotations("macro name")
// TODO: Enforce 'identifier' syntax subset of symbol
// Possibly add support for macro definitions without names?
macroName = symbolValue().assumeText()

if (type != IonType.NULL) {
macroName = symbolValue().assumeText().also { confirm(isIdentifierSymbol(it)) { "invalid macro name: '$it'" } }
}
nextAndCheckType(IonType.SEXP, "macro signature")
confirmNoAnnotations("macro signature")
readSignature()
Expand All @@ -62,32 +62,51 @@ class MacroCompiler(private val reader: IonReader) {
* Caller is responsible for making sure that the reader is positioned on (but not stepped into) the signature sexp.
*/
private fun readSignature() {
var pendingParameter: Macro.Parameter? = null

reader.forEachInContainer {
when (it) {
IonType.SYMBOL -> addParameter(grouped = false)
IonType.LIST -> {
confirmNoAnnotations(location = "grouped parameter enclosing list")
readContainer {
nextAndCheckType(IonType.SYMBOL, "parameter name")
addParameter(grouped = true)
confirm(next() == null) { "grouped parameter list must enclose only one variable name" }
}
if (type != IonType.SYMBOL) throw IonException("parameter must be a symbol; found $type")

val symbolText = symbolValue().assumeText()

val cardinality = Macro.ParameterCardinality.fromSigil(symbolText)

if (cardinality != null) {
confirmNoAnnotations("cardinality sigil")
// The symbol is a cardinality modifier
if (pendingParameter == null) {
throw IonException("Found an orphaned cardinality in macro signature")
} else {
signature.add(pendingParameter!!.copy(cardinality = cardinality))
pendingParameter = null
return@forEachInContainer
}
else -> throw IonException("parameter must be a symbol or a list; found ${reader.type}")
}

// If we have a pending parameter, add it to the signature before we read the next parameter
if (pendingParameter != null) signature.add(pendingParameter!!)

// Read the next parameter name
val annotations = typeAnnotations
confirm(annotations.isEmptyOr(Macro.ParameterEncoding.Tagged.ionTextName)) { "unsupported parameter encoding ${annotations.toList()}" }
confirm(isIdentifierSymbol(symbolText)) { "invalid parameter name: '$symbolText'" }
confirm(signature.none { it.variableName == symbolText }) { "redeclaration of parameter '$symbolText'" }
pendingParameter = Macro.Parameter(symbolText, Macro.ParameterEncoding.Tagged, Macro.ParameterCardinality.One)
}
// If we have a pending parameter than hasn't been added to the signature, add it here.
if (pendingParameter != null) signature.add(pendingParameter!!)
}

/**
* Adds a parameter to the macro signature.
* Caller is responsible for making sure that the reader is positioned on a parameter name.
*/
private fun addParameter(grouped: Boolean) {
val annotations = reader.typeAnnotations
confirm(annotations.isEmptyOr(Macro.ParameterEncoding.Tagged.ionTextName)) { "unsupported parameter encoding ${annotations.toList()}" }
val parameterName = reader.symbolValue().assumeText()
confirm(signature.none { it.variableName == parameterName }) { "redeclaration of parameter '$parameterName'" }
signature.add(Macro.Parameter(parameterName, Macro.ParameterEncoding.Tagged, grouped))
private fun isIdentifierSymbol(symbol: String): Boolean {
if (symbol.isEmpty()) return false

// If the symbol's text matches an Ion keyword, it's not an identifier symbol.
// Eg, the symbol 'false' must be quoted and is not an identifier symbol.
if (_Private_IonTextAppender.isIdentifierKeyword(symbol)) return false

if (!_Private_IonTextAppender.isIdentifierStart(symbol[0].code)) return false

return symbol.all { c -> _Private_IonTextAppender.isIdentifierPart(c.code) }
}

/**
Expand Down Expand Up @@ -192,16 +211,16 @@ class MacroCompiler(private val reader: IonReader) {
IonType.SYMBOL -> {
val macroName = reader.stringValue()
// TODO: Once we have a macro table, validate name exists in current macro table.
if (macroName == "quote") null else MacroRef.ByName(macroName)
// TODO: Come up with a consistent strategy for handling special forms.
if (macroName == "literal") null else MacroRef.ByName(macroName)
}
// TODO: When we have an ID for the macro "quote", add handling for it here.
// TODO: Once we have a macro table, validate that id exists in current macro table.
IonType.INT -> MacroRef.ById(reader.longValue())
else -> throw IonException("macro invocation must start with an id (int) or identifier (symbol); found ${reader.type ?: "nothing"}\"")
}

if (macroRef == null) {
// It's the "quote" macro; skip compiling a macro invocation and just treat all contents as literals
// It's the "literal" special form; skip compiling a macro invocation and just treat all contents as literals
reader.forEachRemaining { compileTemplateBodyExpression(isQuoted = true) }
} else {
val macroStart = expressions.size
Expand Down Expand Up @@ -229,6 +248,12 @@ class MacroCompiler(private val reader: IonReader) {
confirm(next() == expected) { "$location must be a $expected; found ${type ?: "nothing"}" }
}

/** Moves to the next type and throw [IonException] if it is not the `expected` [IonType]. */
private fun IonReader.nextAndCheckType(expected0: IonType, expected1: IonType, location: String) {
val next = next()
confirm(next == expected0 || next == expected1) { "$location must be a $expected0 or $expected1; found ${type ?: "nothing"}" }
}

/** Steps into a container, executes [block], and steps out. */
private inline fun IonReader.readContainer(block: IonReader.() -> Unit) { stepIn(); block(); stepOut() }

Expand Down
55 changes: 39 additions & 16 deletions src/test/java/com/amazon/ion/impl/macro/MacroCompilerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,38 @@ class MacroCompilerTest {

private fun testCases() = listOf(
"(macro identity (x) x)" shouldCompileTo TemplateMacro(
listOf(Parameter("x", Tagged, grouped = false)),
listOf(Parameter("x", Tagged, ParameterCardinality.One)),
listOf(Variable(0)),
),
"(macro identity (any::x) x)" shouldCompileTo TemplateMacro(
listOf(Parameter("x", Tagged, grouped = false)),
listOf(Parameter("x", Tagged, ParameterCardinality.One)),
listOf(Variable(0)),
),
"(macro pi () 3.141592653589793)" shouldCompileTo TemplateMacro(
signature = emptyList(),
body = listOf(DecimalValue(emptyList(), BigDecimal("3.141592653589793")))
),
"(macro group_test ([x]) x)" shouldCompileTo TemplateMacro(
signature = listOf(Parameter("x", Tagged, grouped = true)),
"(macro cardinality_test (x?) x)" shouldCompileTo TemplateMacro(
signature = listOf(Parameter("x", Tagged, ParameterCardinality.AtMostOne)),
body = listOf(Variable(0))
),
"(macro cardinality_test (x!) x)" shouldCompileTo TemplateMacro(
signature = listOf(Parameter("x", Tagged, ParameterCardinality.One)),
body = listOf(Variable(0))
),
"(macro cardinality_test (x+) x)" shouldCompileTo TemplateMacro(
signature = listOf(Parameter("x", Tagged, ParameterCardinality.AtLeastOne)),
body = listOf(Variable(0))
),
"(macro cardinality_test (x*) x)" shouldCompileTo TemplateMacro(
signature = listOf(Parameter("x", Tagged, ParameterCardinality.Any)),
body = listOf(Variable(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 `quote` call causes the third (inner) `(values x)` to be an uninterpreted s-expression.
"""(macro quote_test (x) (values (values x) (quote (values x))))""" shouldCompileTo TemplateMacro(
signature = listOf(Parameter("x", Tagged, grouped = false)),
// 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(
signature = listOf(Parameter("x", Tagged, ParameterCardinality.One)),
body = listOf(
MacroInvocation(ByName("values"), startInclusive = 0, endInclusive = 5),
MacroInvocation(ByName("values"), startInclusive = 1, endInclusive = 2),
Expand All @@ -65,7 +77,7 @@ class MacroCompilerTest {
SymbolValue(emptyList(), FakeSymbolToken("x", -1)),
),
),
"(macro each_type () (values null true 1 ${"9".repeat(50)} 1e0 1d0 2024-01-16T \"foo\" (quote bar) [] (quote ()) {} {{}} {{\"\"}} ))" shouldCompileTo TemplateMacro(
"(macro each_type () (values null true 1 ${"9".repeat(50)} 1e0 1d0 2024-01-16T \"foo\" (literal bar) [] (literal ()) {} {{}} {{\"\"}} ))" shouldCompileTo TemplateMacro(
signature = emptyList(),
body = listOf(
MacroInvocation(ByName("values"), 0, 14),
Expand Down Expand Up @@ -102,11 +114,15 @@ class MacroCompilerTest {
BoolValue(emptyList(), false),
)
),
"(macro null () \"abc\")" shouldCompileTo TemplateMacro(
signature = emptyList(),
body = listOf(StringValue(emptyList(), "abc"))
),
"(macro foo (x y z) [100, [200, a::b::300], x, {y: [true, false, z]}])" shouldCompileTo TemplateMacro(
signature = listOf(
Parameter("x", Tagged, grouped = false),
Parameter("y", Tagged, grouped = false),
Parameter("z", Tagged, grouped = false)
Parameter("x", Tagged, ParameterCardinality.One),
Parameter("y", Tagged, ParameterCardinality.One),
Parameter("z", Tagged, ParameterCardinality.One)
),
body = listOf(
ListValue(emptyList(), startInclusive = 0, endInclusive = 11),
Expand Down Expand Up @@ -163,6 +179,7 @@ class MacroCompilerTest {
"""
(macro foo (x) 1)
(macro bar (y) 2)
(macro null (z) 3)
"""
)
val compiler = MacroCompiler(reader)
Expand All @@ -173,6 +190,9 @@ class MacroCompilerTest {
reader.next()
compiler.compileMacro()
assertEquals("bar", compiler.macroName)
reader.next()
compiler.compileMacro()
assertNull(compiler.macroName)
}

// macro with invalid variable
Expand All @@ -198,24 +218,27 @@ class MacroCompilerTest {
"(macro 2.5 () 3.141592653589793)", // Macro name is not a symbol
"""(macro "pi"() 3.141592653589793)""", // Macro name is not a symbol
"(macro \$0 () 3.141592653589793)", // Macro name must have known text
"(macro + () 123)", // Macro name cannot be an operator symbol
"(macro 'a.b' () 123)", // Macro name must be a symbol that can be unquoted (i.e. an identifier symbol)
"(macro 'false' () 123)", // Macro name must be a symbol that can be unquoted (i.e. an identifier symbol)

// Problems in the signature
"(macro identity x x)", // Missing sexp around signature
"(macro identity [x] x)", // Using list instead of sexp for signature
"(macro identity any::(x) x)", // Signature sexp should not be annotated
"(macro identity (foo::x) x)", // Unknown type in signature
"(macro identity (any::[x]) x)", // Annotation should be on parameter name, not the grouping indicator
"(macro identity ([]) x)", // Grouping indicator must have one symbol in it
"(macro identity ([x, y]) x)", // Grouping indicator must have one symbol in it
"(macro identity (x any::*) x)", // Annotation should be on parameter name, not the cardinality
"(macro identity (x! !) x)", // Dangling cardinality modifier
"(macro identity (x%) x)", // Not a real cardinality sigil
"(macro identity (x x) x)", // Repeated parameter name
"""(macro identity ("x") x)""", // Parameter name must be a symbol, not a string

// Problems in the body
"(macro empty ())", // No body expression
"(macro transform (x) y)", // Unknown variable
"(macro transform (x) foo::x)", // Variable cannot be annotated
"(macro transform (x) foo::(quote x))", // Macro invocation cannot be annotated
"""(macro transform (x) ("quote" x))""", // Macro invocation must start with a symbol or integer id
"(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) 1 2)", // Template body must be one expression
]
)
Expand Down

0 comments on commit 7d0c8cc

Please sign in to comment.