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

Implement alloc::sync::UniqueArc #133572

Merged
merged 5 commits into from
Mar 29, 2025
Merged

Conversation

frank-king
Copy link
Contributor

This implements the alloc::sync::UniqueArc part of #112566.

@rustbot
Copy link
Collaborator

rustbot commented Nov 28, 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 S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 28, 2024
@rust-log-analyzer

This comment has been minimized.

@steffahn
Copy link
Member

Hah! I’m just noticing that this, as well as UniqueRc, absolutely must be invariant (w.r.t. the parameter T).

Otherwise, you can

fn extend_lifetime<'a, 'b>(x: &'a str) -> &'b str {
    let r = UniqueRc::new(""); // UniqueRc<&'static str>
    let w = UniqueRc::downgrade(&r); // Weak<&'static str>
    let mut r = r; // COVARIANT: ==>> UniqueRc<&'a str>
    *r = x; // assign the &'a str
    let _r = UniqueRc::into_rc(r); // Rc<&'a str>, but we only care to activate the weak
    let r = w.upgrade().unwrap(); // upgrade succeeds: Rc<&'static str>
    *r // &'static str, coerces to &'b str
}

(playground)

@bors
Copy link
Collaborator

bors commented Jan 11, 2025

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

jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 12, 2025
…k-Simulacrum

Make (unstable API) `UniqueRc` invariant for soundness

Add test case from rust-lang#133572 (comment) (comment in review of `UniqueArc`), and fix the issue for `UniqueRc`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 12, 2025
…k-Simulacrum

Make (unstable API) `UniqueRc` invariant for soundness

Add test case from rust-lang#133572 (comment) (comment in review of `UniqueArc`), and fix the issue for `UniqueRc`.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 12, 2025
Rollup merge of rust-lang#135379 - steffahn:uniquerc-invariant, r=Mark-Simulacrum

Make (unstable API) `UniqueRc` invariant for soundness

Add test case from rust-lang#133572 (comment) (comment in review of `UniqueArc`), and fix the issue for `UniqueRc`.
@frank-king frank-king force-pushed the feature/unique_arc branch from 5142171 to 9310c96 Compare March 3, 2025 02:54
@rust-log-analyzer

This comment has been minimized.

@frank-king frank-king force-pushed the feature/unique_arc branch from 951fa08 to 0b2c03e Compare March 3, 2025 08:46
@frank-king frank-king force-pushed the feature/unique_arc branch 2 times, most recently from ff35547 to d4783e3 Compare March 6, 2025 14:16
@bors
Copy link
Collaborator

bors commented Mar 8, 2025

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

github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 11, 2025
…k-Simulacrum

Make (unstable API) `UniqueRc` invariant for soundness

Add test case from rust-lang#133572 (comment) (comment in review of `UniqueArc`), and fix the issue for `UniqueRc`.
@Amanieu
Copy link
Member

Amanieu commented Mar 26, 2025

Could you add a test that checks the behavior of weak pointers created from UniqueArc?

@Amanieu
Copy link
Member

Amanieu commented Mar 29, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 29, 2025

📌 Commit 5004e10 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. labels Mar 29, 2025
@bors
Copy link
Collaborator

bors commented Mar 29, 2025

⌛ Testing commit 5004e10 with merge 1799887...

@bors
Copy link
Collaborator

bors commented Mar 29, 2025

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 1799887 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 29, 2025
@bors bors merged commit 1799887 into rust-lang:master Mar 29, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Mar 29, 2025
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing d4812c8 (parent) -> 1799887 (this PR)

Test differences

Show 20 test diffs

Stage 0

  • arc::test_unique_arc_weak: [missing] -> pass (J3)

Stage 1

  • arc::test_unique_arc_weak: [missing] -> pass (J0)
  • [ui] tests/ui/variance/variance-uniquearc.rs: [missing] -> pass (J2)

Stage 2

  • [ui] tests/ui/variance/variance-uniquearc.rs: [missing] -> pass (J1)

Additionally, 16 doctest diffs were found. These are ignored, as they are noisy.

Job group index

  • J0: aarch64-apple, aarch64-gnu, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-apple-1, x86_64-gnu, x86_64-gnu-aux, x86_64-gnu-llvm-18-1, x86_64-gnu-llvm-18-2, x86_64-gnu-llvm-18-3, x86_64-gnu-llvm-19-1, x86_64-gnu-llvm-19-2, x86_64-gnu-llvm-19-3, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
  • J1: aarch64-apple, aarch64-gnu, arm-android, armhf-gnu, dist-i586-gnu-i586-i686-musl, i686-gnu-1, i686-gnu-nopt-1, i686-msvc-1, test-various, x86_64-apple-2, x86_64-gnu, x86_64-gnu-llvm-18-1, x86_64-gnu-llvm-18-2, x86_64-gnu-llvm-19-1, x86_64-gnu-llvm-19-2, x86_64-gnu-nopt, x86_64-gnu-stable, x86_64-mingw-1, x86_64-msvc-1
  • J2: x86_64-gnu-llvm-18-3, x86_64-gnu-llvm-19-3
  • J3: mingw-check

Job duration changes

  1. x86_64-apple-1: 6838.0s -> 8036.7s (17.5%)
  2. x86_64-apple-2: 4457.6s -> 5236.7s (17.5%)
  3. dist-aarch64-apple: 4601.0s -> 5331.5s (15.9%)
  4. dist-various-2: 3234.8s -> 3384.0s (4.6%)
  5. x86_64-gnu-nopt: 5428.0s -> 5664.4s (4.4%)
  6. test-various: 4110.6s -> 4258.8s (3.6%)
  7. aarch64-apple: 3796.0s -> 3926.6s (3.4%)
  8. i686-mingw-2: 6494.3s -> 6715.7s (3.4%)
  9. dist-powerpc64le-linux: 9252.0s -> 9518.7s (2.9%)
  10. dist-x86_64-netbsd: 5043.5s -> 5182.7s (2.8%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

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

Results (primary 2.5%, secondary 2.5%)

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)
2.5% [2.3%, 2.7%] 2
Regressions ❌
(secondary)
2.5% [2.5%, 2.5%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.5% [2.3%, 2.7%] 2

Cycles

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

Binary size

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

Bootstrap: 777.207s -> 777.64s (0.06%)
Artifact size: 365.95 MiB -> 365.94 MiB (-0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants