Skip to content
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

Adds support for macro-aware transcoding from text. #1010

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

tgregg
Copy link
Contributor

@tgregg tgregg commented Dec 9, 2024

Description of changes:
Builds on #1000 and #1005.

Supports macro-aware transcoding from text, preserving symbol tables, encoding directives, and e-expression invocations. This will be useful for:

  • Allowing human-authored macros to be transcoded directly to binary for size/performance testing.
  • Allow text Ion 1.1 to be re-written with pretty printing without evaluating e-expressions.

This change required moving a few symbol table-related methods from the user reader to the system reader, as we had to do in binary. That's because Ion 1.1 e-expressions need to be interpreted at the system level (because they may expand to system values), and may therefore affect the encoding context (including the symbol table).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -237,7 +237,7 @@ protected void readValueAsExpression(boolean isImplicitRest, List<Expression.EEx
}
IonType type = reader.encodingType();
List<SymbolToken> annotations = getAnnotations();
if (isImplicitRest) {
if (isImplicitRest && !isContainerAnExpressionGroup()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug fix. Without this, an expression group as the final parameter to a rest-compatible argument would be parsed into a nested expression group, which is illegal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the bugs that I noticed yesterday. Thanks for catching and fixing this.

* @return true if the reader is positioned on an Ion 1.0 symbol table; otherwise, false. Note: the caller must
* ensure this is called only at the top level.
*/
boolean isPositionedOnSymbolTable() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The green from here through handlePossibleIonVersionMarker was moved from the user-level reader.

Base automatically changed from ion-11-encoding-by-value-transcode to ion-11-encoding December 12, 2024 18:53
@tgregg tgregg force-pushed the ion-11-encoding-by-value-transcode-text branch from db7014a to b9df158 Compare December 12, 2024 19:11
@tgregg tgregg merged commit 5c35d1a into ion-11-encoding Dec 12, 2024
16 checks passed
@tgregg tgregg deleted the ion-11-encoding-by-value-transcode-text branch December 12, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants