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

Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level #126893

Merged
merged 2 commits into from
Jun 26, 2024

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Jun 24, 2024

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as PREC_POSTFIX (field access, square bracket index, question mark, method call) and PREC_PAREN (loops, if, paths, literals).

There are a bunch of places that look at either prec < PREC_POSTFIX or prec >= PREC_POSTFIX. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

pub const PREC_POSTFIX: i8 = 60;
pub const PREC_PAREN: i8 = 99;

let close_paren = if expr.precedence().order() < PREC_POSTFIX {

let mut sugg = if expr.precedence().order() >= PREC_POSTFIX {

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. x.f() means a different thing than (x.f)(). But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax — you can tell because if it did, then x.f[0].f() wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

fn print_expr_call(&mut self, func: &ast::Expr, args: &[P<ast::Expr>], fixup: FixupContext) {
let prec = match func.kind {
ast::ExprKind::Field(..) => parser::PREC_FORCE_PAREN,
_ => parser::PREC_POSTFIX,
};

@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2024

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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 Jun 24, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 24, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@@ -1160,7 +1160,7 @@ impl<'tcx> Dereferencing<'tcx> {
},
Some(parent) if !parent.span.from_expansion() => {
// Double reference might be needed at this point.
if parent.precedence().order() == PREC_POSTFIX {
if parent.precedence().order() == PREC_UNAMBIGUOUS {
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 line contains the only observable behavior change in this PR.

rust-lang/rust-clippy#12990 has a writeup of what this code is doing. It is using precedence of the parent expression of an expression x to decide whether it's possible to suggest replacing x by &x, or by (&x).

Precedence of the parent expression is the wrong thing for Clippy to be looking at in this code, and causes the false negatives reported in the link. This PR increases the number of cases which encounter that false negative in the short term, such as [x; 2], but does not make the issue harder to fix in Clippy.

@compiler-errors
Copy link
Member

r? compiler-errors @bors r+

@bors
Copy link
Contributor

bors commented Jun 24, 2024

📌 Commit 273447c has been approved by compiler-errors

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 Jun 24, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 24, 2024
Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as `PREC_POSTFIX` (field access, square bracket index, question mark, method call) and `PREC_PAREN` (loops, if, paths, literals).

There are a bunch of places that look at either `prec < PREC_POSTFIX` or `prec >= PREC_POSTFIX`. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast/src/util/parser.rs#L236-L237

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L2829

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L1290

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. `x.f()` means a different thing than `(x.f)()`. But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax &mdash; you can tell because if it did, then `x.f[0].f()` wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L217-L221
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 24, 2024
Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as `PREC_POSTFIX` (field access, square bracket index, question mark, method call) and `PREC_PAREN` (loops, if, paths, literals).

There are a bunch of places that look at either `prec < PREC_POSTFIX` or `prec >= PREC_POSTFIX`. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast/src/util/parser.rs#L236-L237

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L2829

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L1290

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. `x.f()` means a different thing than `(x.f)()`. But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax &mdash; you can tell because if it did, then `x.f[0].f()` wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L217-L221
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#124460 (Show notice about  "never used" of Debug for enum)
 - rust-lang#125740 (transmute size check: properly account for alignment)
 - rust-lang#126413 (compiletest: make the crash test error message abit more informative)
 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126673 (Ensure we don't accidentally succeed when we want to report an error)
 - rust-lang#126804 (On short error format, append primary span label to message)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126899 (Suggest inline const blocks for array initialization)
 - rust-lang#126909 (add `@kobzol` to bootstrap team for triagebot)

r? `@ghost`
`@rustbot` modify labels: rollup
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 25, 2024
Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as `PREC_POSTFIX` (field access, square bracket index, question mark, method call) and `PREC_PAREN` (loops, if, paths, literals).

There are a bunch of places that look at either `prec < PREC_POSTFIX` or `prec >= PREC_POSTFIX`. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast/src/util/parser.rs#L236-L237

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L2829

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L1290

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. `x.f()` means a different thing than `(x.f)()`. But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax &mdash; you can tell because if it did, then `x.f[0].f()` wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L217-L221
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…mpiler-errors

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126907 (Fixes for 32-bit SPARC on Linux)
 - rust-lang#126932 (Tweak `FlatPat::new` to avoid a temporarily-invalid state)
 - rust-lang#126934 (fix broken build cache caused by rustdoc builds)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)
 - rust-lang#126946 (Add missing slash in `const_eval_select` doc comment)
 - rust-lang#126947 (Delegation: ast lowering refactor)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 25, 2024
Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as `PREC_POSTFIX` (field access, square bracket index, question mark, method call) and `PREC_PAREN` (loops, if, paths, literals).

There are a bunch of places that look at either `prec < PREC_POSTFIX` or `prec >= PREC_POSTFIX`. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast/src/util/parser.rs#L236-L237

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L2829

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L1290

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. `x.f()` means a different thing than `(x.f)()`. But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax &mdash; you can tell because if it did, then `x.f[0].f()` wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L217-L221
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…kingjubilee

Rollup of 14 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126885 (Remove internal `PathBuf::as_mut_vec`)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126916 (Specify target specific linker for `riscv64gc-gnu` job)
 - rust-lang#126926 (Tweak a confusing comment in `create_match_candidates`)
 - rust-lang#126927 (core: VaArgSafe is an unsafe trait)
 - rust-lang#126932 (Tweak `FlatPat::new` to avoid a temporarily-invalid state)
 - rust-lang#126941 (Migrate `run-make/llvm-ident` to `rmake.rs`)
 - rust-lang#126946 (Add missing slash in `const_eval_select` doc comment)
 - rust-lang#126947 (Delegation: ast lowering refactor)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#126618 (Mark assoc tys live only if the corresponding trait is live)
 - rust-lang#126746 (Deny `use<>` for RPITITs)
 - rust-lang#126868 (not use offset when there is not ends with brace)
 - rust-lang#126884 (Do not ICE when suggesting dereferencing closure arg)
 - rust-lang#126893 (Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level)
 - rust-lang#126915 (Don't suggest awaiting in closure patterns)
 - rust-lang#126943 (De-duplicate all consecutive native libs regardless of their options)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 709baae into rust-lang:master Jun 26, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 26, 2024
Rollup merge of rust-lang#126893 - dtolnay:prec, r=compiler-errors

Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as `PREC_POSTFIX` (field access, square bracket index, question mark, method call) and `PREC_PAREN` (loops, if, paths, literals).

There are a bunch of places that look at either `prec < PREC_POSTFIX` or `prec >= PREC_POSTFIX`. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast/src/util/parser.rs#L236-L237

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L2829

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L1290

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. `x.f()` means a different thing than `(x.f)()`. But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax &mdash; you can tell because if it did, then `x.f[0].f()` wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L217-L221
@dtolnay dtolnay deleted the prec branch June 26, 2024 04:28
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 27, 2024
Eliminate the distinction between PREC_POSTFIX and PREC_PAREN precedence level

I have been tangling with precedence as part of porting some pretty-printer improvements from syn back to rustc (related to parenthesization of closures, returns, and breaks by the AST pretty-printer).

As far as I have been able to tell, there is no difference between the 2 different precedence levels that rustc identifies as `PREC_POSTFIX` (field access, square bracket index, question mark, method call) and `PREC_PAREN` (loops, if, paths, literals).

There are a bunch of places that look at either `prec < PREC_POSTFIX` or `prec >= PREC_POSTFIX`. But there is nothing that needs to distinguish PREC_POSTFIX and PREC_PAREN from one another.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast/src/util/parser.rs#L236-L237

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L2829

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs#L1290

In the interest of eliminating a distinction without a difference, this PR collapses these 2 levels down to 1.

There is exactly 1 case where an expression with PREC_POSTFIX precedence needs to be parenthesized in a location that an expression with PREC_PAREN would not, and that's when the receiver of ExprKind::MethodCall is ExprKind::Field. `x.f()` means a different thing than `(x.f)()`. But this does not justify having separate precedence levels because this special case in the grammar is not governed by precedence. Field access does not have "lower precedence than" method call syntax &mdash; you can tell because if it did, then `x.f[0].f()` wouldn't be able to have its unparenthesized field access in the receiver of a method call. Because this Field/MethodCall special case is not governed by precedence, it already requires special handling and is not affected by eliminating the PREC_POSTFIX precedence level.

https://github.com/rust-lang/rust/blob/d49994b060684af423339b55769439b2f444a7b9/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L217-L221
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

5 participants