Skip to content

suggest parenthesis around ExprWithBlock BinOp ExprWithBlock #105223

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Dec 6, 2022

Conversation

lukas-code
Copy link
Member

fix #105179
fix #102171

@rustbot
Copy link
Collaborator

rustbot commented Dec 3, 2022

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 3, 2022
err.span_suggestion_short(
span.shrink_to_hi(),
"consider using a semicolon here",
";",
Applicability::MachineApplicable,
Applicability::MaybeIncorrect,
Copy link
Member Author

Choose a reason for hiding this comment

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

This was already wrong on nightly and isn't caused by this PR.

@robinmoussu
Copy link

Is this a temporary fix? Shouldn’t ExprWithBlock BinOp ExprWithBlock compiles?

I was expecting

match () { 
    () => 1
} + match () {
    () => 2
}

to be valid Rust and equals to 3.

@lukas-code
Copy link
Member Author

Is this a temporary fix? Shouldn’t ExprWithBlock BinOp ExprWithBlock compiles?

This is an unfortunate side effect of the current Rust grammar. When a statement starts with an ExpressionWithBlock (such as a match expression), the statement ends immediately after the closing }.

Changing this will probably require an RFC and a nontrivial amount of work, since this would introduce parsing ambiguities based on types, for example:

fn foo() -> impl FnOnce() -> bool {
    match () { () => () }
    || match () { () => true } // closure
}

fn bar() -> bool {
    // (match () { () => true }) || match () { () => true } // logical or
    match () { () => true } || match () { () => true } // logical or
}

fn baz() -> &'static &'static bool {
    match () { () => () }
    &&match () { () => true } // reference to reference
}

fn quux() -> bool {
    // (match () { () => true }) && match () { () => true } // logical and
    match () { () => true } && match () { () => true } // logical and
}

Furthermore, this would break resolution of unary vs binary - and should probably be gated by an edition. Example:

use std::ops;

struct Bar;
impl ops::Neg for Bar {
    type Output = ();

    fn neg(self) {
        println!("neg");
    }
}
impl ops::Sub<Bar> for () {
    type Output = ();

    fn sub(self, _: Bar) {
        println!("sub");
    }
}

fn main() {
    {
        match () {
            () => (),
        } - match () {
            () => Bar,
        }
    }

    {
        (match () {
            () => (),
        }) - match () {
            () => Bar,
        }
    }
}

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 5, 2022

@bors r+

1 similar comment
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 5, 2022

📌 Commit c808d0b has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 5, 2022
…ochenkov

suggest parenthesis around ExprWithBlock BinOp ExprWithBlock

fix rust-lang#105179
fix rust-lang#102171
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 5, 2022
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#104912 (PartialEq: PERs are homogeneous)
 - rust-lang#104952 (Streamline the user experience for `x.py setup`)
 - rust-lang#104953 (Ensure required submodules at the same time as updating existing submodules)
 - rust-lang#105180 (Use proper HirId for async track_caller attribute check)
 - rust-lang#105222 (std update libc version and freebsd image build dependencies)
 - rust-lang#105223 (suggest parenthesis around ExprWithBlock BinOp ExprWithBlock)
 - rust-lang#105230 (Skip recording resolution for duplicated generic params.)
 - rust-lang#105301 (update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 66a4cb5 into rust-lang:master Dec 6, 2022
@rustbot rustbot added this to the 1.67.0 milestone Dec 6, 2022
@robinmoussu
Copy link

This is an unfortunate side effect of the current Rust grammar. When a statement starts with an ExpressionWithBlock (such as a match expression), the statement ends immediately after the closing }.

Basically, this would require to have significant newline to distinguish them. Or to require a ; at the end of each statement, include match, if and similar statement that have a pair of braces. Yeah, it doesn’t sound great.

1 similar comment
@robinmoussu
Copy link

This is an unfortunate side effect of the current Rust grammar. When a statement starts with an ExpressionWithBlock (such as a match expression), the statement ends immediately after the closing }.

Basically, this would require to have significant newline to distinguish them. Or to require a ; at the end of each statement, include match, if and similar statement that have a pair of braces. Yeah, it doesn’t sound great.

@lukas-code lukas-code deleted the (ExprWithBlock) branch December 6, 2022 17:31
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…ochenkov

suggest parenthesis around ExprWithBlock BinOp ExprWithBlock

fix rust-lang#105179
fix rust-lang#102171
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#104912 (PartialEq: PERs are homogeneous)
 - rust-lang#104952 (Streamline the user experience for `x.py setup`)
 - rust-lang#104953 (Ensure required submodules at the same time as updating existing submodules)
 - rust-lang#105180 (Use proper HirId for async track_caller attribute check)
 - rust-lang#105222 (std update libc version and freebsd image build dependencies)
 - rust-lang#105223 (suggest parenthesis around ExprWithBlock BinOp ExprWithBlock)
 - rust-lang#105230 (Skip recording resolution for duplicated generic params.)
 - rust-lang#105301 (update Miri)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

match + match compilation error Missing "parentheses are required to parse this as an expression" hint with multiple unsafe blocks
5 participants