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.

Sign up for GitHub

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

Jump to bottom

Make AssocOp more like ExprKind #136846

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
Contributor

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
Contributor

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
Contributor

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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