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

#[lang_item] for core::ptr::Unique #112662

Merged
merged 1 commit into from
Jun 17, 2023
Merged

#[lang_item] for core::ptr::Unique #112662

merged 1 commit into from
Jun 17, 2023

Conversation

Vanille-N
Copy link
Contributor

@Vanille-N Vanille-N commented Jun 15, 2023 •
edited
Loading

Tree Borrows is about to introduce experimental special handling of core::ptr::Unique in Miri to give it a semantics.
As of now there does not seem to be a clean way (i.e. other than &format!("{adt:?}") == "std::ptr::Unique") to check if an AdtDef represents a Unique.

r? @RalfJung

Draft: making a lang item

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 15, 2023
@compiler-errors
Copy link
Member

Are you using this just for diagnostics, or is correctness in Miri dependent on always being able to know what a Unique is? If the latter, perhaps this should be a lang item instead.

@RalfJung
Copy link
Member

RalfJung commented Jun 15, 2023 via email •
edited
Loading

@compiler-errors
Copy link
Member

No, they're compiler-internal details so there's no stability guarantee or anything, not that that would matter for Miri or anything. I think my approval should be sufficient.

@RalfJung
Copy link
Member

Okay, in that case -- @Vanille-N looks like I guessed wrong and a lang item is preferable here. Sorry for that.

@compiler-errors
Copy link
Member

My understanding is that both the compiler and tools should work, perhaps with degraded diagnostics, if all diagnostic items are stripped from the standard library -- but the compiler (and by extension, compiler-adjacent tools) should be allowed to depend on the existence and well-formedness of a lang item. We don't expect the compiler to work the lang = "copy" is stripped, but we do if rustc_diagnostic_item = "RefCell" is.

@Noratrieb
Copy link
Member

yeah, branching on the happy path should be a lang item
(Ideally also with a comment inside core explaining that it's used experimentally inside Miri)

@compiler-errors compiler-errors self-assigned this Jun 15, 2023
@Vanille-N Vanille-N marked this pull request as draft June 15, 2023 18:34
@Vanille-N
Copy link
Contributor Author

This seems to be the minimum needed for use by Miri, but there's probably more that should be added.

  • should it get some GenericRequirement other than None ?
  • should mini_core.rs inside rustc_codegen_{cranelift,gcc} get it as well ?
  • test usage ? I'll save the full PR on semantics of Unique for the miri repository, but maybe I should add here a simple test that the lang item is recognized.

This compiles after some --stage 1 --keep-stage 0 manipulations, but I'm not even sure it'll pass the CI checks.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung
Copy link
Member

This seems to be the minimum needed for use by Miri, but there's probably more that should be added.

* should it get some `GenericRequirement` other than `None` ?

I wouldn't worry about it, None is fine.

* should `mini_core.rs` inside `rustc_codegen_{cranelift,gcc}` get it as well ?

No, the lang item doesn't have to exist.

* test usage ? I'll save the full PR on semantics of `Unique` for the miri repository, but maybe I should add here a simple test that the lang item is recognized.

How would you test this, other than implementing Unique semantics?

@Vanille-N Vanille-N changed the title rustc_diagnostic_item for core::ptr::Unique #[lang_item] for core::ptr::Unique Jun 16, 2023
@Noratrieb
Copy link
Member

it has one generic parameter, just use Exact(1)

@RalfJung
Copy link
Member

LGTM, please squash the commits then we can land this. :)

@Vanille-N Vanille-N marked this pull request as ready for review June 16, 2023 15:12
@compiler-errors
Copy link
Member

@bors r=RalfJung

@bors
Copy link
Contributor

bors commented Jun 16, 2023

📌 Commit dc3e91c has been approved by RalfJung

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 Jun 16, 2023
@compiler-errors compiler-errors mentioned this pull request Jun 16, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 16, 2023
@bors
Auto merge of rust-lang#112716 - compiler-errors:rollup-h77daia, r=co…
670a0ed 
…mpiler-errors

Rollup of 7 pull requests

Successful merges:

 - rust-lang#111074 (Relax implicit `T: Sized` bounds on `BufReader<T>`, `BufWriter<T>` and `LineWriter<T>`)
 - rust-lang#112226 (std: available_parallelism using native netbsd api first)
 - rust-lang#112474 (Support 128-bit enum variant in debuginfo codegen)
 - rust-lang#112662 (`#[lang_item]` for `core::ptr::Unique`)
 - rust-lang#112665 (Make assumption functions in new solver take `Binder<'tcx, Clause<'tcx>>`)
 - rust-lang#112684 (Disable alignment checks on i686-pc-windows-msvc)
 - rust-lang#112706 (Add `SyntaxContext::is_root`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 38fc6be into rust-lang:master Jun 17, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 17, 2023
@Vanille-N Vanille-N mentioned this pull request Jun 20, 2023
Merged
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 20, 2023
@compiler-errors
Rollup merge of rust-lang#112844 - Vanille-N:unique, r=RalfJung
3dd1406 
Add retag in MIR transform: `Adt` for `Unique` may contain a reference

Following rust-lang#112662 , `may_contain_reference` in `rustc_mir_transform::add_retag` underapproximates too much the types that require retagging.

r? `@RalfJung`
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Jun 21, 2023
@Noratrieb
Rollup merge of rust-lang#112844 - Vanille-N:unique, r=RalfJung
82e6a16 
Add retag in MIR transform: `Adt` for `Unique` may contain a reference

Following rust-lang#112662 , `may_contain_reference` in `rustc_mir_transform::add_retag` underapproximates too much the types that require retagging.

r? ``@RalfJung``
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

@RalfJung RalfJung

@compiler-errors compiler-errors

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. T-libs Relevant to the library 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.
7 participants
@Vanille-N @compiler-errors @RalfJung @Noratrieb @rust-log-analyzer @bors @rustbot