Adds text descriptions for intended system macro test cases #128
Conversation
| The inner clauses should align with the formal data model in the denotational | ||
| semantics, which would (more or less) reduce the components to integers and | ||
| strings. |
There was a problem hiding this comment.
The denotational semantics are not yet available on GitHub, so I wonder if this will be confusing than enlightening.
EDIT: I see now that this section was relocated from below.
| // parse_ion can read Ion 1.0 and Ion 1.1, text and binary | ||
|
|
||
| // parse_ion always produces user values | ||
| // - but if the result passes through other macros it could turn it into a system value...? |
There was a problem hiding this comment.
Yes. And that's ok because the other macros are allowed to affect the current encoding context while the values in the nested stream are not.
There was a problem hiding this comment.
But what about this?
$ion_1_1
$4 // name
(:values (:parse_ion '''$ion_1_1 $ion_literal::$ion_symbol_table::{symbols:["a", "b", "c", "d"]}''' ))
$4 // d
Are we okay with letting system values leak from parse_ion when it's wrapped in another macro/e-expression, and with the fact that values is no longer an identity function in this scenario?
There was a problem hiding this comment.
I don't think that use case works. Values coming out of parse_ion cannot be system values; if they "look like" a system value, parse_ion will escape them in the output.
There was a problem hiding this comment.
So, parse_ion actually prepends $ion_literal:: to anything that could be a system value? (Or retains, depending on how you look at it.)
There was a problem hiding this comment.
I've created amazon-ion/ion-docs#365 to clarify this so as not to delay this PR.
| // Test Cases: | ||
| // set_macros can be invoked by name, address, qualified address, qualified name, binary address, and binary system e-expression op code. | ||
| // the argument values may be zero or more macro definitions | ||
| // TODO: Could it also be a module or an export? |
There was a problem hiding this comment.
Off-hand I think yes for both this and add_macros? mod_a::* and macro_foo both seem reasonable.
Though there is something to be said for the charm of macro-shaped parameters here:
(:set_macros
(foo () ...)
(bar () ...)
(baz () ...)
)
Probably not worth precluding the other options, though. I suppose we could have a separate def_macros that only took macro definitions.
Co-authored-by: Zack Slayton <[email protected]>
Co-authored-by: Tyler Gregg <[email protected]> Co-authored-by: Zack Slayton <[email protected]>
Issue #, if available:
#88
Description of changes:
This adds text descriptions of test cases that will be added. I have chosen to start with this as a way to try to quickly align on the behavior of the macros and ensure there is no unspecified behavior before I start writing full test cases for all of these.
When reviewing, of course call out anything that might be incorrect, but also consider if there's anything that is missing or unspecified. That being said, you can assume that the system macro signatures in the specification are also going to be tested even though I have not called each of them out explicitly in the test descriptions.
I've also added a copyright header to the
grammar.island rearranged + added an overview in the README.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.