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

Parsing inconsistencies with semi-statements in expressions #29071

Closed
nagisa opened this issue Oct 15, 2015 · 3 comments
Closed

Parsing inconsistencies with semi-statements in expressions #29071

nagisa opened this issue Oct 15, 2015 · 3 comments

Comments

@nagisa
Copy link
Member

nagisa commented Oct 15, 2015

    x = if true {
        10u32
    } else {
        20u32
    } as ()

parses as

    x = (if true {
        10u32
    } else {
        20u32
    } as ())

but

    if true {
        10u32
    } else {
        20u32
    } as ()

parses as

    if true {
        10u32
    } else {
        20u32
    }; 
    as ()
@nagisa
Copy link
Member Author

nagisa commented Oct 15, 2015

This might showcase it better:

    x&if true {
        10u32
    } else {
        20u32
    }&x

parsed as

    (x & if true {
        10u32
    } else {
        20u32
    } & x)

but

    if true {
        10u32
    } else {
        20u32
    }&x

is parsed as

    if true {
        10u32
    } else {
        20u32
    };
    &x

which is completely different thing.

@nagisa
Copy link
Member Author

nagisa commented Oct 15, 2015

Fixing this, would be a breaking change, but we at least need to ensure we have a test for this.

@arielb1
Copy link
Contributor

arielb1 commented Oct 15, 2015

@nagisa

This looks like a consequence of "semicolon insertion" (the stmt: block_expr rule) being eager.

bors added a commit that referenced this issue Oct 27, 2015
This commit generalises parsing of associative operators from left-associative
only (with some ugly hacks to support right-associative assignment) to properly
left/right-associative operators.

Parsing is still is not general enough to handle non-associative,
non-highest-precedence prefix or non-highest-precedence
postfix operators (e.g. `..` range syntax) and should be made to be.

Lastly, this commit adds support for parsing right-associative `<-` (left arrow)
operator with precedence higher than assignment as the operator for placement-in
feature.

---

This PR still needs various non-parser changes (e.g. src/grammar and tests) and I’m still working on these; the meat of the PR can already be reviewed, though, I think.

Please review carefully. I made sure that quirks I have discovered so far are preserved (see e.g. #29071) and am looking for more corner cases as I continue to work on tests et al, but there may be something I haven’t noticed or accounted for.

EDIT: I’m also not sure I managed to preserve all the semantics with the range operator inside non-trivial expressions since these are a mess at the moment. Crater runs would be nice.
@bors bors closed this as completed in 884b589 Oct 28, 2015
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