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

Rename adjustment::PointerCast and variants using it to PointerCoercion #113376

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Rename adjustment::PointerCast and variants using it to PointerCoercion #113376

merged 1 commit into from
Jul 8, 2023

Conversation

Noratrieb
Copy link
Member

It makes it sounds like the ExprKind and Rvalue are supposed to represent all pointer related casts, when in reality their just used to share a little enum variants. Make it clear there these are only coercions and that people who see this and think "why are so many pointer related casts not in these variants" aren't insane.

This enum was added in #59987. I'm not sure whether the variant sharing is actually worth it, but this at least makes it less confusing.

r? oli-obk

@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 Jul 5, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jul 5, 2023

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Noratrieb Noratrieb force-pushed the pointer-coercions-are-not-casts-because-that-sounds-way-to-general-aaaa branch from 0164791 to 63e538b Compare July 5, 2023 19:20
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Jul 6, 2023

r=me after clippy compiles again

@bors
Copy link
Contributor

bors commented Jul 6, 2023

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

@Noratrieb Noratrieb force-pushed the pointer-coercions-are-not-casts-because-that-sounds-way-to-general-aaaa branch from 63e538b to 506904a Compare July 6, 2023 17:42
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Noratrieb Noratrieb force-pushed the pointer-coercions-are-not-casts-because-that-sounds-way-to-general-aaaa branch from 506904a to 3ac2825 Compare July 6, 2023 18:03
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Noratrieb Noratrieb force-pushed the pointer-coercions-are-not-casts-because-that-sounds-way-to-general-aaaa branch from 3ac2825 to 7c0f1eb Compare July 6, 2023 18:47
@Noratrieb
Copy link
Member Author

@bors r=oli-obk

@bors
Copy link
Contributor

bors commented Jul 6, 2023

📌 Commit 7c0f1eb6df8f6421ddde3868c0da2891ae5f4a84 has been approved by oli-obk

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 Jul 6, 2023
@Noratrieb
Copy link
Member Author

Already got a conflict since yesterday
@bors p=1

@bors
Copy link
Contributor

bors commented Jul 6, 2023

⌛ Testing commit 7c0f1eb6df8f6421ddde3868c0da2891ae5f4a84 with merge e39b45df87de9d953590c7c1682f4f27ea286221...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Jul 6, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 6, 2023
@bors
Copy link
Contributor

bors commented Jul 7, 2023

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

@Noratrieb
Rename adjustment::PointerCast and variants using it to `PointerCoe…
2beabbb 
…rcion`

It makes it sound like the `ExprKind` and `Rvalue` are supposed to represent all pointer related
casts, when in reality their just used to share a some enum variants. Make it clear there these
are only coercion to make it clear why only some pointer related "casts" are in the enum.
@Noratrieb Noratrieb force-pushed the pointer-coercions-are-not-casts-because-that-sounds-way-to-general-aaaa branch from 7c0f1eb to 2beabbb Compare July 7, 2023 16:17
@Noratrieb
Copy link
Member Author

@bors r=oli-obk
@bors p=2 priority will increase until merge conflicts decrease

@bors
Copy link
Contributor

bors commented Jul 7, 2023

📌 Commit 2beabbb has been approved by oli-obk

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 Jul 7, 2023
@bors
Copy link
Contributor

bors commented Jul 7, 2023

⌛ Testing commit 2beabbb with merge 03c19fc9e9b7ede3bbeaa87858d619159cb1f9c3...

@bors
Copy link
Contributor

bors commented Jul 7, 2023

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 7, 2023
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@Noratrieb
Copy link
Member Author

@bors retry

@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 Jul 8, 2023
@bors
Copy link
Contributor

bors commented Jul 8, 2023

⌛ Testing commit 2beabbb with merge 9bb6fbe...

@bors
Copy link
Contributor

bors commented Jul 8, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 9bb6fbe to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 8, 2023
@bors bors merged commit 9bb6fbe into rust-lang:master Jul 8, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jul 8, 2023
This was referenced Jul 8, 2023
Closed
Draft
Merged
@Noratrieb Noratrieb deleted the pointer-coercions-are-not-casts-because-that-sounds-way-to-general-aaaa branch July 8, 2023 17:20
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9bb6fbe): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 1.7% [1.7%, 1.7%] 1
Improvements ✅
(primary)		 -4.9% [-4.9%, -4.9%] 1
Improvements ✅
(secondary)		 -2.9% [-3.3%, -2.5%] 2
All ❌✅ (primary) -4.9% [-4.9%, -4.9%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.8% [0.8%, 0.8%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.8% [0.8%, 0.8%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 655.094s -> 657.838s (0.42%)

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 14, 2023
@bors
Auto merge of rust-lang#113376 - Nilstrieb:pointer-coercions-are-not-…
1816caa 
…casts-because-that-sounds-way-to-general-aaaa, r=oli-obk

Rename `adjustment::PointerCast` and variants using it to `PointerCoercion`

It makes it sounds like the `ExprKind` and `Rvalue` are supposed to represent all pointer related casts, when in reality their just used to share a little enum variants. Make it clear there these are only coercions and that people who see this and think "why are so many pointer related casts not in these variants" aren't insane.

This enum was added in rust-lang#59987. I'm not sure whether the variant sharing is actually worth it, but this at least makes it less confusing.

r? oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
merged-by-bors This PR was explicitly merged by bors. 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.72.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Noratrieb @rustbot @rust-log-analyzer @oli-obk @bors @rust-timer