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.

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

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

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 23, 2024

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.

@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.

@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

@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
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
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
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
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
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
…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
…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
  - 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 added a commit to model-checking/kani that referenced this pull request Apr 16, 2024
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
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
zpzigi754 pushed a commit to zpzigi754/kani that referenced this pull request May 8, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants