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.

Sign up for GitHub

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

Add tests for $ion_literal. #97

Merged
merged 3 commits into from
Jun 12, 2024
Merged

Add tests for $ion_literal. #97

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
db10310
Add tests for `$ion_literal`.
toddjonker Jun 3, 2024
c1fa96f
Apply suggestions from code review
toddjonker Jun 11, 2024
fc6d4dc
Update conformance/ion_literal.ion
toddjonker Jun 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions conformance/ion_literal.ion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0


(ion_1_1
(each (toplevel '#$11' '#$11'::'#$11' '#$11'::1)
(toplevel $ion_literal $ion_literal::$ion_literal $ion_literal::1 $ion_literal::$ion_literal::1)
(produces $ion_literal $ion_literal 1 $ion_literal::1)))

// But the annotation is only recognized at top level.
(ion_1_1
(toplevel ['#$11', '#$11'::'#$11', $ion_literal::1, $ion_literal::$ion_literal::1])
(produces [$ion_literal, $ion_literal::$ion_literal, $ion_literal::1, $ion_literal::$ion_literal::1]))

// No such effects in 1.0, even with the same SID.
(ion_1_0
(toplevel $ion_symbol_table::{symbols:["$ion_literal"]})
(toplevel '#$10' '#$10'::'#$10')
(produces $ion_literal $ion_literal::$ion_literal))


// Top-level `$ion_literal::$ion_symbol_table::{}` is an error.
// TODO Is this correct? What about '$ion_1_0' etc?
Copy link
Contributor

Choose a reason for hiding this comment

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

Any IVM-like symbol with an annotation is already always a symbol, and doesn't need an annotation, right?

Also, how does this affect text Ion—specifically, a quoted, unannotated IVM is actually a no-op. Does resolving $ion_literal happen before or after determining whether it's a no-op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any IVM-like symbol with an annotation is already always a symbol, and doesn't need an annotation, right?

True, but that doesn't help when you want to represent the unannotated symbol with text "$ion_1_0".

Does resolving $ion_literal happen before or after determining whether it's a no-op?

They would have to happen at the same phase. That no-op behavior of (eg) '$ion_1_0' means that either:

  1. the token is absorbed/discarded at the parsing level, as if it were a comment; or
  2. the parser emits it as some new kind of syntactic entity (AST node type), to be discarded by the expansion phase; or
  3. the parser emits it as a normal symbol, and the expansion phase discards it (at the same it would've detected any $ion_literal annotation).

I think (1) would be easiest to implement but surprising in terms of round-tripping. (2) is obnoxious. So I'm inclined to go with (2) above as a clearer specification of the 1.0 handling.

For 1.1 I currently think we should dump the no-op behavior of such quoted forms, making them normal symbols; let the unquoted form be "escaped" via $ion_literal, and do the same thing for local symtabs $ion_literal::$ion_symbol_table::{. The outcome is that Ion 1.1 allows everything in the data model to be written at top level as user data.

It wouldn't surprise me if Ion 1.0 implementations are full of bugs in their handling of this space. We didn't think it through carefully enough, and then back-filled specification years later to try to match the observed behavior of the primary implementations.

This is worth a separate issue, especially since it's not even covered by this CR. 😉

// https://github.com/amazon-ion/ion-docs/issues/322
(ion_1_1
(toplevel $ion_literal::$ion_symbol_table::{})
(signals "Cannot use literal $ion_symbol_table struct as application data"))

(ion_1_1
(toplevel [$ion_literal::$ion_symbol_table::{}])
(produces [$ion_literal::$ion_symbol_table::{}]))