-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Macro implementations and conformance test fixes #1003
Changes from 4 commits
07b01e1
a563da1
a1debeb
67ad344
0745be3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ This was a bug I discovered that was causing |
||
readGroupExpression(parameter, expressions, false); | ||
} else { | ||
throw new IonException(String.format( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)); | ||
Comment on lines
+1240
to
+1242
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ When there's nothing there, and we're expecting a parameter, we need to create a "nothing" expression. (In this case, it's an empty expression group, but I think that there's probably a good justification for introducing a |
||
return; | ||
} | ||
readValueAsExpression(isTrailing && parameter.getCardinality().canBeMulti, expressions); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,8 @@ internal class IonManagedWriter_1_1( | |
private val onClose: () -> Unit, | ||
) : _Private_IonWriter, MacroAwareIonWriter { | ||
|
||
internal fun getRawUserWriter(): IonRawWriter_1_1 = userData | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ Hack so that I can convert mangled e-expressions in a |
||
|
||
companion object { | ||
private val ION_VERSION_MARKER_REGEX = Regex("^\\\$ion_\\d+_\\d+$") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,7 @@ internal class MacroCompiler( | |
val encoding = Macro.ParameterEncoding.entries.singleOrNull { it.ionTextName == encodingText } | ||
if (encoding == null) { | ||
// TODO: Check for macro-shaped parameter encodings, and only if it's still null, we throw. | ||
throw IonException("unsupported parameter encoding $annotations") | ||
throw IonException("Unknown parameter encoding: $encodingText") | ||
} | ||
encoding | ||
} | ||
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ This fixes a bug that prevented the macro compiler from recognizing special forms. |
||
return m ?: throw IonException("Unrecognized macro: $macroRef") | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
Comment on lines
-19
to
-20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗺️ This is now unused now that all system macros use tagged parameters. |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗺️ Minor improvement for ease of debugging.