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

Update symbolTablesUnknownText.ion #85

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update symbolTablesUnknownText.ion #85

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
47 changes: 36 additions & 11 deletions iontestdata_1_0/good/non-equivs/symbolTablesUnknownText.ion
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,50 @@
// import declaration, with max_id > 0
// verifies that imported symbols with undefined text are not equivalent to:
// - other imported symbols with undefined text with different SIds
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// - other imported symbols with undefined text with different SIds
// - other imported symbols with undefined text with different SIDs

The symbols documentation generally says "SID"

// - local symbols with undefined text
// - symbols with defined text
embedded_documents::[
'''
$ion_1_0
$ion_symbol_table::{
symbols:[ "foo", "bar", "baz" ]
imports:[ { name: "com.amazon.ion.tests",
version: 1,
max_id: 2 }
],
symbols:[ "foo", 31 ]
Copy link
Contributor

Choose a reason for hiding this comment

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

From https://amazon-ion.github.io/ion-docs/docs/symbols.html#local-symbol-tables

The symbols field should be a list of strings. If the field is missing or has any other type, it is treated as if it were an empty list.

Null elements in the symbols list declare unknown symbol text (“gaps”) for its SID within the sequence. Any element of the list that is not a string must be interpreted as if it were null. Any SIDs that refer to null slots in a local symbol table are equivalent to symbol zero.

What is this doing, why put a number here instead of an explicit null? Why 31?

Copy link
Contributor

Choose a reason for hiding this comment

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

This could be any non-string (or null), testing the case where"Any element of the list that is not a string must be interpreted as if it were null."

I'm sure we have tests elsewhere that ensure the implementation treats all non-strings as null slots.

}
$10 // first imported symbol of undefined text
''',
'''
$ion_1_0
$ion_symbol_table::{
imports:[ { name: "com.amazon.ion.tests",
version: 1,
max_id: 2 }
],
symbols:[ "foo", 31 ]
}
$11 // second imported symbol of undefined text
''',
'''
$ion_1_0
$ion_symbol_table::{
imports:[ { name: "com.amazon.ion.tests",
version: 1,
max_id: 2 }
],
symbols:[ "foo", 31 ]
}
$10
$11
$12
$12 // local symbol with text "foo"
''',
'''
$ion_1_0
$ion_symbol_table::{
imports:[ { name: "com.amazon.ion.tests",
version: 1,
max_id: 1 }
max_id: 2 }
],
symbols:[ "foo", "bar", "baz" ]
symbols:[ "foo", 31 ]
}
$10
$11
$12
$13 // local symbol with undefined text
''',
]
]