Skip to content

Make AssocOp more like ExprKind #136846

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

Jump to bottom
Merged
merged 4 commits into from
Feb 28, 2025
Merged

Make AssocOp more like ExprKind #136846

merged 4 commits into from
Feb 28, 2025

Conversation

nnethercote
Copy link
Contributor

This is step 1 of MCP 831.

r? @estebank

@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 11, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 11, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@bors
Copy link
Collaborator

bors commented Feb 12, 2025

☔ The latest upstream changes (presumably #136878) made this pull request unmergeable. Please resolve the merge conflicts.

@nnethercote
Copy link
Contributor Author

The conflicts are trivial and won't affect review, so I will wait before fixing them.

@nnethercote nnethercote force-pushed the make-AssocOp-more-like-ExprKind branch from 1ec80bc to 22c1d74 Compare February 17, 2025 00:07
@nnethercote
Copy link
Contributor Author

It has been two weeks, let's try a different reviewer.

r? @spastorino

@rustbot rustbot assigned spastorino and unassigned estebank Feb 23, 2025
@bors
Copy link
Collaborator

bors commented Feb 25, 2025

☔ The latest upstream changes (presumably #135726) made this pull request unmergeable. Please resolve the merge conflicts.

@nnethercote
In AssocOp::AssignOp, use BinOpKind instead of BinOpToken
a8364f3 
`AssocOp::AssignOp` contains a `BinOpToken`. `ExprKind::AssignOp`
contains a `BinOpKind`. Given that `AssocOp` is basically a cut-down
version of `ExprKind`, it makes sense to make `AssocOp` more like
`ExprKind`. Especially given that `AssocOp` and `BinOpKind` use semantic
operation names (e.g. `Mul`, `Div`), but `BinOpToken` uses syntactic
names (e.g. `Star`, `Slash`).

This results in more concise code, and removes the need for various
conversions. (Note that the removed functions `hirbinop2assignop` and
`astbinop2assignop` are semantically identical, because `hir::BinOp` is
just a synonum for `ast::BinOp`!)

The only downside to this is that it allows the possibility of some
nonsensical combinations, such as `AssocOp::AssignOp(BinOpKind::Lt)`.
But `ExprKind::AssignOp` already has that problem. The problem can be
fixed for both types in the future with some effort, by introducing an
`AssignOpKind` type.
@nnethercote
Introduce AssocOp::Binary.
ceafbad 
It mirrors `ExprKind::Binary`, and contains a `BinOpKind`. This makes
`AssocOp` more like `ExprKind`. Note that the variants removed from
`AssocOp` are all named differently to `BinOpToken`, e.g. `Multiply`
instead of `Mul`, so that's an inconsistency removed.

The commit adds `precedence` and `fixity` methods to `BinOpKind`, and
calls them from the corresponding methods in `AssocOp`. This avoids the
need to create an `AssocOp` from a `BinOpKind` in a bunch of places, and
`AssocOp::from_ast_binop` is removed.

`AssocOp::to_ast_binop` is also no longer needed.

Overall things are shorter and nicer.
@nnethercote
Replace AssocOp::DotDot{,Eq} with AssocOp::Range.
fc8e87b 
It makes `AssocOp` more similar to `ExprKind` and makes things a little
simpler. And the semantic names make more sense here than the syntactic
names.
@nnethercote
Rename AssocOp::As as AssocOp::Cast.
2ac46f6 
To match `ExprKind::Cast`, and because a semantic name makes more sense
here than a syntactic name.
@nnethercote nnethercote force-pushed the make-AssocOp-more-like-ExprKind branch from 22c1d74 to 2ac46f6 Compare February 26, 2025 23:01
@spastorino
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 27, 2025

📌 Commit 2ac46f6 has been approved by spastorino

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 27, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 27, 2025
@matthiaskrgr
Rollup merge of rust-lang#136846 - nnethercote:make-AssocOp-more-like…
4f7de02 
…-ExprKind, r=spastorino

Make `AssocOp` more like `ExprKind`

This is step 1 of [MCP 831](rust-lang/compiler-team#831).

r? `@estebank`
This was referenced Feb 27, 2025
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2025
@bors
Auto merge of rust-lang#137710 - matthiaskrgr:rollup-3vmxxu9, r=matth…
8346456 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
 - rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
 - rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
 - rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
 - rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)
 - rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
 - rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
 - rust-lang#137694 (Spruce up `AttributeKind` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 27, 2025
@matthiaskrgr
Rollup merge of rust-lang#136846 - nnethercote:make-AssocOp-more-like…
fabf069 
…-ExprKind, r=spastorino

Make `AssocOp` more like `ExprKind`

This is step 1 of [MCP 831](rust-lang/compiler-team#831).

r? ```@estebank```
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 27, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2025
@bors
Auto merge of rust-lang#137716 - matthiaskrgr:rollup-k2rjlu6, r=<try>
4109d74 
Rollup of 4 pull requests

Successful merges:

 - rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
 - rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
 - rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
 - rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)

r? `@ghost`
`@rustbot` modify labels: rollup

try-job: i686-msvc-1
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 27, 2025
@bors
Auto merge of rust-lang#137710 - matthiaskrgr:rollup-3vmxxu9, r=matth…
b4657c0 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
 - rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
 - rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
 - rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
 - rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)
 - rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
 - rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
 - rust-lang#137694 (Spruce up `AttributeKind` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
@bors
Auto merge of rust-lang#137710 - matthiaskrgr:rollup-3vmxxu9, r=matth…
cb08599 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#136542 ([`compiletest`-related cleanups 4/7] Make the distinction between root build directory vs test suite specific build directory in compiletest less confusing)
 - rust-lang#136579 (Fix UB in ThinVec::flat_map_in_place)
 - rust-lang#136688 (require trait impls to have matching const stabilities as the traits)
 - rust-lang#136846 (Make `AssocOp` more like `ExprKind`)
 - rust-lang#137304 (add `IntoBounds::intersect` and `RangeBounds::is_empty`)
 - rust-lang#137455 (Reuse machinery from `tail_expr_drop_order` for `if_let_rescope`)
 - rust-lang#137480 (Return unexpected termination error instead of panicing in `Thread::join`)
 - rust-lang#137694 (Spruce up `AttributeKind` docs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4ecca4c into rust-lang:master Feb 28, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Feb 28, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2025
@rust-timer
Unrolled build for rust-lang#136846
352987e 
Rollup merge of rust-lang#136846 - nnethercote:make-AssocOp-more-like-ExprKind, r=spastorino

Make `AssocOp` more like `ExprKind`

This is step 1 of [MCP 831](rust-lang/compiler-team#831).

r? ``@estebank``
@nnethercote nnethercote deleted the make-AssocOp-more-like-ExprKind branch February 28, 2025 08:03
@nnethercote nnethercote mentioned this pull request Mar 4, 2025
Closed
@dtolnay dtolnay mentioned this pull request Apr 5, 2025
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@spastorino spastorino

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
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nnethercote @rustbot @bors @spastorino @estebank