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 rustc_middle::Ty::is_unsafe_ptr to is_raw_ptr #135994

Merged
merged 4 commits into from
Feb 13, 2025
Merged

Rename rustc_middle::Ty::is_unsafe_ptr to is_raw_ptr #135994

merged 4 commits into from
Feb 13, 2025

Conversation

1c3t3a
Copy link
Contributor

@1c3t3a 1c3t3a commented Jan 24, 2025

The wording unsafe pointer is less common and not mentioned in a lot of places, instead this is usually called a "raw pointer". For the sake of uniformity, we rename this method.
This came up during the review of
#134424.

r? @Noratrieb

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Could not assign reviewer from: Noratrieb.
User(s) Noratrieb are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

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

rustbot commented Jan 24, 2025

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Noratrieb
Copy link
Member

r? Noratrieb

testing something

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Noratrieb has insufficient capacity to be assigned the pull request at this time. PR assignment has been reverted.

Please choose another assignee.

(see documentation)

@Noratrieb
Copy link
Member

r? oli-obk

more testing

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Could not assign reviewer from: oli-obk.
User(s) oli-obk are either the PR author, already assigned, or on vacation. Please use r? to specify someone else to assign.

@oli-obk oli-obk removed their assignment Jan 24, 2025
@RalfJung
Copy link
Member

coerce_unsafe_ptr should also be renamed.

Clippy also has a lint with unsafe_ptr in the name -- but that would better be changed in a clippy PR (if you want, otherwise filing an issue to record this mismatch is also fine).

@Noratrieb Noratrieb self-assigned this Jan 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Noratrieb has insufficient capacity to be assigned the pull request at this time. PR assignment has been reverted.

Please choose another assignee.

(see documentation)

@Noratrieb Noratrieb assigned oli-obk and 1c3t3a and unassigned oli-obk Jan 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

You have insufficient capacity to be assigned the pull request at this time. PR assignment has been reverted.

Please choose another assignee or increase your assignment limit.

(see documentation)

@Noratrieb
Copy link
Member

r? oli-obk

@Centri3
Copy link
Member

Centri3 commented Jan 24, 2025

Clippy also has a lint with unsafe_ptr in the name -- but that would better be changed in a clippy PR (if you want, otherwise filing an issue to record this mismatch is also fine).

I believe you are speaking on not_unsafe_ptr_arg_deref? The unsafe is mentioning a fn not being marked unsafe - not the raw pointer. I don't believe this is necessary (but I concur it is still a weird lint name!)

@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Jan 24, 2025

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung
Copy link
Member

I believe you are speaking on not_unsafe_ptr_arg_deref? The unsafe is mentioning a fn not being marked unsafe - not the raw pointer. I don't believe this is necessary (but I concur it is still a weird lint name!)

Ah I see, makes sense.

@1c3t3a 1c3t3a mentioned this pull request Jan 30, 2025
Merged
@bors
Copy link
Contributor

bors commented Jan 31, 2025

📌 Commit 6afd0c9 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 Jan 31, 2025
@workingjubilee workingjubilee mentioned this pull request Jan 31, 2025
Closed
@workingjubilee
Copy link
Member

apparent merge conflicts?

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2025
@workingjubilee
Copy link
Member

even with the master branch I mean.

also
@bors rollup=iffy

@1c3t3a 1c3t3a force-pushed the rename-unsafe-ptr branch from 6afd0c9 to b4abf2f Compare January 31, 2025 20:30
@1c3t3a
Copy link
Contributor Author

1c3t3a commented Jan 31, 2025

apparent merge conflicts?

@bors r-

Yes! Just rebased after #134424 has landed :)

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
@bors
Auto merge of rust-lang#136367 - workingjubilee:rollup-pud5uj0, r=wor…
1bc23f1 
…kingjubilee

Rollup of 16 pull requests

Successful merges:

 - rust-lang#133266 (ci: fix explanation why LLVM download is disabled for windows-gnu)
 - rust-lang#135768 (tests: Port `symbol-mangling-hashed` to rmake.rs)
 - rust-lang#135836 (bootstrap: only build `crt{begin,end}.o` when compiling to MUSL)
 - rust-lang#135840 (omit unused args warnings for intrinsics without body)
 - rust-lang#135900 (Manually walk into WF obligations in `BestObligation` proof tree visitor)
 - rust-lang#136146 (Explicitly choose x86 softfloat/hardfloat ABI)
 - rust-lang#136154 (Use +secure-plt for powerpc-unknown-linux-gnu{,spe})
 - rust-lang#136163 (Fix off-by-one error causing slice::sort to abort the program)
 - rust-lang#136266 (fix broken release notes id)
 - rust-lang#136283 (Update encode_utf16 to mention it is native endian)
 - rust-lang#136309 (set rustc dylib on manually constructed rustc command)
 - rust-lang#136314 (Use proper type when applying deref adjustment in const)
 - rust-lang#136339 (CompileTest: Add Directives to Ignore `arm-unknown-*` Targets)
 - rust-lang#136348 (miri: make float min/max non-deterministic)
 - rust-lang#136351 (Add documentation for derive(CoercePointee))
 - rust-lang#136358 (`#[optimize(none)]` implies `#[inline(never)]`)

Failed merges:

 - rust-lang#135994 (Rename rustc_middle::Ty::is_unsafe_ptr to is_raw_ptr)

r? `@ghost`
`@rustbot` modify labels: rollup
@1c3t3a
Copy link
Contributor Author

1c3t3a commented Feb 3, 2025

This should work now :) Sorry for the confusion, the other patch landed first and broke this one.

@bors
Copy link
Contributor

bors commented Feb 9, 2025

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

@1c3t3a
Rename rustc_middle::Ty::is_unsafe_ptr to is_raw_ptr
f842ee8 
The wording unsafe pointer is less common and not mentioned in a lot of
places, instead this is usually called a "raw pointer". For the sake of
uniformity, we rename this method.
This came up during the review of
rust-lang#134424.
@1c3t3a
Extend the renaming to coerce_unsafe_ptr
432ff5e
@1c3t3a
Convert two missed places
854948a
@1c3t3a
Convert more missed places
0c7d5e2
@1c3t3a
Copy link
Contributor Author

1c3t3a commented Feb 10, 2025

Friendly ping on this change :) Should be ready to merge once again!

@1c3t3a
Copy link
Contributor Author

1c3t3a commented Feb 12, 2025

@workingjubilee @oli-obk Is there anything that blocks this from landing? I saw that Jubilee was r-'ing this patch but I assume that is because of the merge conflict that's now resolved, or do I miss something? :)

@oli-obk
Copy link
Contributor

oli-obk commented Feb 12, 2025

@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2025

📌 Commit 0c7d5e2 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 12, 2025
@bors
Copy link
Contributor

bors commented Feb 12, 2025

⌛ Testing commit 0c7d5e2 with merge 6dce9f8...

@bors
Copy link
Contributor

bors commented Feb 13, 2025

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 13, 2025
@bors bors merged commit 6dce9f8 into rust-lang:master Feb 13, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 13, 2025
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6dce9f8): 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)

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

Cycles

Results (secondary 2.4%)

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)		 2.4% [2.1%, 2.8%] 2
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 790.496s -> 788.472s (-0.26%)
Artifact size: 347.84 MiB -> 347.85 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

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.86.0
Development

Successfully merging this pull request may close these issues.
10 participants
@1c3t3a @rustbot @rust-log-analyzer @Noratrieb @RalfJung @Centri3 @oli-obk @bors @workingjubilee @rust-timer