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 ptr::from_exposed_addr -> ptr::with_exposed_provenance #122935

Merged
merged 3 commits into from
Apr 3, 2024
Merged

rename ptr::from_exposed_addr -> ptr::with_exposed_provenance #122935

merged 3 commits into from
Apr 3, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 23, 2024 •
edited
Loading

As discussed on Zulip.

The old name, from_exposed_addr, makes little sense as it's not the address that is exposed, it's the provenance. (ptr.expose_addr() stays unchanged as we haven't found a better option yet. The intended interpretation is "expose the provenance and return the address".)

The new name nicely matches ptr::without_provenance.

@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

r? @Amanieu

rustbot has assigned @Amanieu.
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 O-hermit Operating System: Hermit O-itron Operating System: ITRON 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

The Miri subtree was changed

cc @rust-lang/miri

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

cc @calebzulawski, @programmerjake

@RalfJung RalfJung added I-libs-api-nominated Nominated for discussion during a libs-api team meeting. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. O-hermit Operating System: Hermit O-itron Operating System: ITRON labels Mar 23, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot rustbot added O-hermit Operating System: Hermit O-itron Operating System: ITRON T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

This PR changes MIR

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

@RalfJung RalfJung added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed O-hermit Operating System: Hermit O-itron Operating System: ITRON labels Mar 23, 2024
@rustbot rustbot added O-hermit Operating System: Hermit O-itron Operating System: ITRON labels Mar 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

This PR changes Stable MIR

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

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@RalfJung RalfJung added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Mar 23, 2024
@RalfJung
Copy link
Member Author

Cc @scottmcm @Gankra

@joboet joboet mentioned this pull request Mar 23, 2024
Merged
@RalfJung RalfJung added the A-strict-provenance Area: Strict provenance for raw pointers label Mar 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 23, 2024

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake

@Amanieu
Copy link
Member

Amanieu commented Apr 2, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Apr 2, 2024

📌 Commit f2cff5e has been approved by Amanieu

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. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Apr 2, 2024
@jhpratt jhpratt mentioned this pull request Apr 3, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
@bors
Auto merge of rust-lang#123396 - jhpratt:rollup-oa54mh1, r=jhpratt
b688d53 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#122865 (Split hir ty lowerer's error reporting code in check functions to mod errors.)
 - rust-lang#122935 (rename ptr::from_exposed_addr -> ptr::with_exposed_provenance)
 - rust-lang#123182 (Avoid expanding to unstable internal method)
 - rust-lang#123203 (Add `Context::ext`)
 - rust-lang#123380 (Improve bootstrap comments)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e9ef8e1 into rust-lang:master Apr 3, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
@rust-timer
Unrolled build for rust-lang#122935
4cdf24b 
Rollup merge of rust-lang#122935 - RalfJung:with-exposed-provenance, r=Amanieu

rename ptr::from_exposed_addr -> ptr::with_exposed_provenance

As discussed on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/To.20expose.20or.20not.20to.20expose/near/427757066).

The old name, `from_exposed_addr`, makes little sense as it's not the address that is exposed, it's the provenance. (`ptr.expose_addr()` stays unchanged as we haven't found a better option yet. The intended interpretation is "expose the provenance and return the address".)

The new name nicely matches `ptr::without_provenance`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 3, 2024
@matthiaskrgr
Rollup merge of rust-lang#122964 - joboet:pointer_expose, r=Amanieu
80d592c 
Rename `expose_addr` to `expose_provenance`

`expose_addr` is a bad name, an address is just a number and cannot be exposed. The operation is actually about the provenance of the pointer.

This PR thus changes the name of the method to `expose_provenance` without changing its return type. There is sufficient precedence for returning a useful value from an operation that does something else without the name indicating such, e.g. [`Option::insert`](https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.insert) and [`MaybeUninit::write`](https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.write).

Returning the address is merely convenient, not a fundamental part of the operation. This is implied by the fact that integers do not have provenance since
```rust
let addr = ptr.addr();
ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
must behave exactly like
```rust
let addr = ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
as the result of `ptr.expose_provenance()` and `ptr.addr()` is the same integer. Therefore, this PR removes the `#[must_use]` annotation on the function and updates the documentation to reflect the important part.

~~An alternative name would be `expose_provenance`. I'm not at all opposed to that, but it makes a stronger implication than we might want that the provenance of the pointer returned by `ptr::with_exposed_provenance`[^1] is the same as that what was exposed, which is not yet specified as such IIUC. IMHO `expose` does not make that connection.~~

A previous version of this PR suggested `expose` as name, libs-api [decided on](rust-lang#122964 (comment)) `expose_provenance` to keep the symmetry with `with_exposed_provenance`.

CC `@RalfJung`
r? libs-api

[^1]: I'm using the new name for `from_exposed_addr` suggested by rust-lang#122935 here.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
@rust-timer
Unrolled build for rust-lang#122964
7645645 
Rollup merge of rust-lang#122964 - joboet:pointer_expose, r=Amanieu

Rename `expose_addr` to `expose_provenance`

`expose_addr` is a bad name, an address is just a number and cannot be exposed. The operation is actually about the provenance of the pointer.

This PR thus changes the name of the method to `expose_provenance` without changing its return type. There is sufficient precedence for returning a useful value from an operation that does something else without the name indicating such, e.g. [`Option::insert`](https://doc.rust-lang.org/nightly/std/option/enum.Option.html#method.insert) and [`MaybeUninit::write`](https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.write).

Returning the address is merely convenient, not a fundamental part of the operation. This is implied by the fact that integers do not have provenance since
```rust
let addr = ptr.addr();
ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
must behave exactly like
```rust
let addr = ptr.expose_provenance();
let new = ptr::with_exposed_provenance(addr);
```
as the result of `ptr.expose_provenance()` and `ptr.addr()` is the same integer. Therefore, this PR removes the `#[must_use]` annotation on the function and updates the documentation to reflect the important part.

~~An alternative name would be `expose_provenance`. I'm not at all opposed to that, but it makes a stronger implication than we might want that the provenance of the pointer returned by `ptr::with_exposed_provenance`[^1] is the same as that what was exposed, which is not yet specified as such IIUC. IMHO `expose` does not make that connection.~~

A previous version of this PR suggested `expose` as name, libs-api [decided on](rust-lang#122964 (comment)) `expose_provenance` to keep the symmetry with `with_exposed_provenance`.

CC `@RalfJung`
r? libs-api

[^1]: I'm using the new name for `from_exposed_addr` suggested by rust-lang#122935 here.
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 4, 2024
@jhpratt
Rollup merge of rust-lang#122935 - RalfJung:with-exposed-provenance, …
e530b3d 
…r=Amanieu

rename ptr::from_exposed_addr -> ptr::with_exposed_provenance

As discussed on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/To.20expose.20or.20not.20to.20expose/near/427757066).

The old name, `from_exposed_addr`, makes little sense as it's not the address that is exposed, it's the provenance. (`ptr.expose_addr()` stays unchanged as we haven't found a better option yet. The intended interpretation is "expose the provenance and return the address".)

The new name nicely matches `ptr::without_provenance`.
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Apr 5, 2024
@jhpratt
Rollup merge of rust-lang#122935 - RalfJung:with-exposed-provenance, …
7a1a456 
…r=Amanieu

rename ptr::from_exposed_addr -> ptr::with_exposed_provenance

As discussed on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/To.20expose.20or.20not.20to.20expose/near/427757066).

The old name, `from_exposed_addr`, makes little sense as it's not the address that is exposed, it's the provenance. (`ptr.expose_addr()` stays unchanged as we haven't found a better option yet. The intended interpretation is "expose the provenance and return the address".)

The new name nicely matches `ptr::without_provenance`.
@RalfJung RalfJung deleted the with-exposed-provenance branch April 5, 2024 16:37
celinval added a commit to celinval/kani-dev that referenced this pull request Apr 16, 2024
@celinval
Upgrade toolchain to nightly-2024-04-15
6045e60 
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id
@celinval celinval mentioned this pull request Apr 16, 2024
Merged
celinval added a commit to model-checking/kani that referenced this pull request Apr 16, 2024
@celinval
Upgrade toolchain to nightly-2024-04-15 (#3144)
b9bd7a8 
Related changes:
  - rust-lang/rust#118310:
    Add `Ord::cmp` for primitives as a `BinOp` in MIR
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id

Resolves #3130, #3142
celinval added a commit to celinval/kani-dev that referenced this pull request Apr 17, 2024
@celinval
Upgrade toolchain to nightly-2024-04-15 (model-checking#3144)
5d962b5 
Related changes:
  - rust-lang/rust#118310:
    Add `Ord::cmp` for primitives as a `BinOp` in MIR
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id

Resolves model-checking#3130, model-checking#3142
stlankes added a commit to stlankes/kernel that referenced this pull request Apr 21, 2024
@stlankes
rename ptr::from_exposed_addr -> ptr::with_exposed_provenance
2884076 
rust-lang/rust#122935 changed the function name
@stlankes stlankes mentioned this pull request Apr 21, 2024
Merged
zpzigi754 pushed a commit to zpzigi754/kani that referenced this pull request May 8, 2024
@celinval @zpzigi754
Upgrade toolchain to nightly-2024-04-15 (model-checking#3144)
6bd7ef8 
Related changes:
  - rust-lang/rust#118310:
    Add `Ord::cmp` for primitives as a `BinOp` in MIR
  - rust-lang/rust#120131:
    Add support to `Pat` pattern type
  - rust-lang/rust#122935:
    Rename CastKind::PointerWithExposedProvenance
  - rust-lang/rust#123097:
    Adapt to changes to local_def_path_hash_to_def_id

Resolves model-checking#3130, model-checking#3142
@dtolnay dtolnay removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Sep 9, 2024
@dtolnay dtolnay mentioned this pull request Sep 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
A-strict-provenance Area: Strict provenance for raw pointers O-hermit Operating System: Hermit O-itron Operating System: ITRON 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.
6 participants
@RalfJung @rustbot @rust-log-analyzer @Amanieu @bors @dtolnay