Tighten up assignment operator representations. #138017
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @vakaras Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
|
@spastorino: I'm not 100% certain we should do this. It does make the representation better in the sense that it makes it impossible to express some nonsensical combinations, such as I suggest starting with the third commit, which is the most important one. If you are happy with how it looks, then look at the first and second commits, which are just some preliminary refactorings. |
|
☔ The latest upstream changes (presumably #138464) made this pull request unmergeable. Please resolve the merge conflicts. |
nnethercote
force-pushed
the
tighten-assignment-op
branch
from
74227e1 to
7d1e5d0
Compare
|
I rebased. |
|
☔ The latest upstream changes (presumably #138653) made this pull request unmergeable. Please resolve the merge conflicts. |
nnethercote
force-pushed
the
tighten-assignment-op
branch
from
7d1e5d0 to
e43b280
Compare
|
@spastorino is busy and asked for a different reviewer, so let's try: r? @estebank |
|
From above, for the reviewer:
|
|
☔ The latest upstream changes (presumably #138747) made this pull request unmergeable. Please resolve the merge conflicts. |
compiler/rustc_hir_typeck/src/op.rs
Outdated
| let by_ref_binop = !op.node.is_by_value(); | ||
| if is_assign == IsAssign::Yes || by_ref_binop { | ||
| let by_ref_binop = !op.is_by_value(); | ||
| if op.is_assign() || by_ref_binop { |
There was a problem hiding this comment.
Not sure if I prefer is_assign or just an explicit matches! checking the type. Whatever you prefer :).
There was a problem hiding this comment.
Ok, I switched to matches!/if let.
|
@nnethercote r=me after rebasing. I've made a minor comment but do whatever you feel is better :). |
nnethercote
force-pushed
the
tighten-assignment-op
branch
from
e43b280 to
98b46f5
Compare
First, move the `lang_item_for_op` call from the top of
`lookup_op_method`'s body to its callsites. It makes those callsites a
little more verbose, but also means `lookup_op_method` no longer cares
whether it's handling a binop or unop. This lets us remove `Op` and
split `lang_item_for_op` into `lang_item_for_{bin,un}op`, which is a
little simpler.
This change is a prerequisite for adding the `ast::AssignOpKind` type in
a subsequent commit.
Because it's nice to avoid passing in unnecessary data.
In the AST, currently we use `BinOpKind` within `ExprKind::AssignOp` and `AssocOp::AssignOp`, even though this allows some nonsensical combinations. E.g. there is no `&&=` operator. Likewise for HIR and THIR. This commit introduces `AssignOpKind` which only includes the ten assignable operators, and uses it in `ExprKind::AssignOp` and `AssocOp::AssignOp`. (And does similar things for `hir::ExprKind` and `thir::ExprKind`.) This avoids the possibility of nonsensical combinations, as seen by the removal of the `bug!` case in `lang_item_for_binop`. The commit is mostly plumbing, including: - Adds an `impl From<AssignOpKind> for BinOpKind` (AST) and `impl From<AssignOp> for BinOp` (MIR/THIR). - `BinOpCategory` can now be created from both `BinOpKind` and `AssignOpKind`. - Replaces the `IsAssign` type with `Op`, which has more information and a few methods. - `suggest_swapping_lhs_and_rhs`: moves the condition to the call site, it's easier that way. - `check_expr_inner`: had to factor out some code into a separate method. I'm on the fence about whether avoiding the nonsensical combinations is worth the extra code.
nnethercote
force-pushed
the
tighten-assignment-op
branch
from
98b46f5 to
ddcb370
Compare
|
I rebased. @bors r=spastorino |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#138017 (Tighten up assignment operator representations.) - rust-lang#138462 (Dedup `&mut *` reborrow suggestion in loops) - rust-lang#138610 (impl !PartialOrd for HirId) - rust-lang#138767 (Allow boolean literals in `check-cfg`) - rust-lang#139068 (io: Avoid marking some bytes as uninit) - rust-lang#139255 (Remove unused variables generated in merged doctests) - rust-lang#139270 (Add a mailmap entry for myself) - rust-lang#139303 (Put Noratrieb on vacation) - rust-lang#139312 (add Marco Ieni to mailmap) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#138017 - nnethercote:tighten-assignment-op, r=spastorino Tighten up assignment operator representations. This is step 3 of [MCP 831](rust-lang/compiler-team#831). r? `@spastorino`
This is step 3 of MCP 831.
r? @spastorino