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

Ambguity in the Smithy IDL grammar #1249

Closed
hlbarber opened this issue Jun 1, 2022 · 1 comment
Closed

Ambguity in the Smithy IDL grammar #1249

hlbarber opened this issue Jun 1, 2022 · 1 comment

Comments

@hlbarber
Copy link
Contributor

hlbarber commented Jun 1, 2022

One example of an ambiguity in the grammar is within shape_section given in the Smithy IDL specification under "Smithy IDL ABNF" -> "Shapes".

shape_section : [`namespace_statement` [`use_section`] [`shape_statements`]]
use_section : *(`use_statement`)

Is [`use_section`] empty or is it populated with an empty *(`use_statement`)?

This places Smithy IDL outside of a large class of grammars (see page 4 of Miscellaneous Parsing).

An example of where this is a problem is when trying to implement the grammar in lalrpop which will complain when we try to implement the following

use_section = use_statement*;
pub shape_section = use_section?;

A similar conflict is described in this issue.

The ambiguity around [`use_section`] can be resolved by making it non-optional or changing use_section to 1*(`use_statement`).

  • Should we attempt to resolve syntax ambiguities?
  • Should we document how to resolve conflicts?
  • Is it permissible to change the grammar if it does not change the language/semantics?
  • Should we document the formal properties of the grammar?

Disclaimer: I'm not an expert in language theory, please be skeptical of my claims and correct me where needed.

@mtdowling
Copy link
Member

mtdowling commented Jun 2, 2022

I don't think this is ambiguous, but sure the [ and ] around use_section is unnecessary since *(use_statement) is already 0 or more.

And yes, we can change the grammar if it's a bug fix. Please feel free to send us a PR!

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

No branches or pull requests

2 participants