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

fix #108495, postfix decrement and prefix decrement has no warning #108496

Merged
merged 5 commits into from
Mar 1, 2023
Merged

fix #108495, postfix decrement and prefix decrement has no warning #108496

merged 5 commits into from
Mar 1, 2023

Conversation

nx2k3
Copy link
Contributor

@nx2k3 nx2k3 commented Feb 26, 2023

Fixes #108495

@rustbot
Copy link
Collaborator

rustbot commented Feb 26, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @WaffleLapkin (or someone else) soon.

Please see the contribution instructions for more information.

@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 Feb 26, 2023
Copy link
Member

@Nilstrieb Nilstrieb left a comment

Choose a reason for hiding this comment

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

Thank you for the PR, but we cannot do this recovery this way.

let _ = i--i;

and

let _ = --i;

compile today (double negation). The only case we would be able to handle is the case of something like i--+i, which is not allowed today. You'd have to be careful to make sure that whatever comes after the second - is not allowed to start an expression (there should be a helper function for that). I think it should work then, but I'm not entirely sure.

I see your issues mentions a "warning". I think emitting a warning on double negation is a great idea!

@fmease
Copy link
Member

fmease commented Feb 26, 2023

I see your issues mentions a "warning". I think emitting a warning on double negation is a great idea!

Fyi, Clippy already emits a warning for this by default:

warning: `--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op
 --> e.rs:3:9
  |
3 | let _ = --i;
  |         ^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#double_neg
  = note: `#[warn(clippy::double_neg)]` on by default

Not sure if it's worth moving this Clippy lint into rustc (this sometimes does happen though, although I don't know what the process is usually like).

@WaffleLapkin
Copy link
Member

this sometimes does happen though, although I don't know what the process is usually like

The process is more or less "ask T-compiler for approval, if they give a green light, make a PR that adds (to rustc) and deprecates (in clippy) the lint". See #99272 and #99696 as an example.

Although I don't think it's worth it in the case of clippy::double_neg lint.

compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved
Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
(I've committed some small fmt changes so that I don't bother you with them)

@WaffleLapkin
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 28, 2023

📌 Commit 031206b has been approved by WaffleLapkin

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 Feb 28, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2023
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#108376 (compiler/rustc_session: fix sysroot detection logic)
 - rust-lang#108400 (add llvm cgu instructions stats to perf)
 - rust-lang#108496 (fix rust-lang#108495, postfix decrement and prefix decrement has no warning)
 - rust-lang#108505 (Further unify validity intrinsics)
 - rust-lang#108520 (Small cleanup to `one_bound_for_assoc_type`)
 - rust-lang#108560 (Some `infer/mod.rs` cleanups)
 - rust-lang#108563 (Make mailmap more correct)
 - rust-lang#108564 (Fix `x clean` with specific paths)
 - rust-lang#108571 (Add contains_key to SortedIndexMultiMap)
 - rust-lang#108578 (Update Fuchsia platform team members)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1c3cc8b into rust-lang:master Mar 1, 2023
@rustbot rustbot added this to the 1.69.0 milestone Mar 1, 2023
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.

postfix decrement and prefix decrement has no warning
6 participants