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

Safe Transmute: Revise safety analysis #121681

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

jswrenn
Copy link
Member

@jswrenn jswrenn commented Feb 27, 2024

This PR migrates BikeshedIntrinsicFrom to a simplified safety analysis (described here) that does not rely on analyzing the visibility of types and fields.

The revised analysis treats primitive types as safe, and user-defined types as potentially carrying safety invariants. If Rust gains explicit (un)safe fields, this PR is structured so that it will be fairly easy to thread support for those annotations into the analysis.

Notably, this PR removes the Context type parameter from BikeshedIntrinsicFrom. Most of the files changed by this PR are just UI tests tweaked to accommodate the removed parameter.

r? @compiler-errors

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Feb 27, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2024

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@rust-log-analyzer

This comment has been minimized.

Migrate to a simplified safety analysis that does not use visibility.

Closes rust-lang/project-safe-transmute#15
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 27, 2024
/// type and field privacy of the destination type (and sometimes of the source type, too).
/// When `true`, the compiler assumes that *you* have ensured that no
/// unsoundness will arise from violating the safety invariants of the
/// destination type (and sometimes of the source type, too).
pub safety: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a bikeshed, but I'd prefer if we made this name more precise. Specifically, these are user safety invariants, i.e. the set of invariants not reflected above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some ongoing discussion on naming among project safe transmute. Virtually every aspect of the naming is up for debate right now. In the interest of keeping names stable across discussions, I'd like to postpone bikeshedding names until we're closer to stabilization. When that conversation does occur, it will probably be with heavy deference to whatever terminology is preferred by t-opsem.

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 29, 2024

📌 Commit 23ab1bd has been approved by compiler-errors

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 Feb 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121326 (Detect empty leading where clauses on type aliases)
 - rust-lang#121464 (rustc: Fix wasm64 metadata object files)
 - rust-lang#121681 (Safe Transmute: Revise safety analysis)
 - rust-lang#121753 (Add proper cfg to keep only one AlignmentEnum definition for different target_pointer_widths)
 - rust-lang#121782 (allow statics pointing to mutable statics)
 - rust-lang#121798 (Fix links in rustc doc)
 - rust-lang#121806 (add const test for ptr::metadata)
 - rust-lang#121809 (Remove doc aliases to PATH)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 419f7ae into rust-lang:master Mar 1, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 1, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2024
Rollup merge of rust-lang#121681 - jswrenn:nix-visibility-analysis, r=compiler-errors

Safe Transmute: Revise safety analysis

This PR migrates `BikeshedIntrinsicFrom` to a simplified safety analysis (described [here](rust-lang/project-safe-transmute#15)) that does not rely on analyzing the visibility of types and fields.

The revised analysis treats primitive types as safe, and user-defined types as potentially carrying safety invariants. If Rust gains explicit (un)safe fields, this PR is structured so that it will be fairly easy to thread support for those annotations into the analysis.

Notably, this PR removes the `Context` type parameter from `BikeshedIntrinsicFrom`. Most of the files changed by this PR are just UI tests tweaked to accommodate the removed parameter.

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants