stabilize smart pointer map functions

[Qelxiros]

View all comments

Tracking issue: #144419
@rustbot modify labels: +T-libs-api

[rustbot]

r? @Darksonn

rustbot has assigned @Darksonn.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: libs
• libs expanded to 12 candidates
• Random selection from 6 candidates

[Darksonn]

Sorry I'm at capacity for the next two weeks.

@rustbot reroll

[dtolnay]

This stabilizes a subset of #144419 — just the map functions and not try_map.

• https://doc.rust-lang.org/nightly/std/boxed/struct.Box.html#method.map
• https://doc.rust-lang.org/nightly/std/rc/struct.UniqueRc.html#method.map
• https://doc.rust-lang.org/nightly/std/sync/struct.UniqueArc.html#method.map
• https://doc.rust-lang.org/nightly/std/rc/struct.Rc.html#method.map
• https://doc.rust-lang.org/nightly/std/sync/struct.Arc.html#method.map

@rfcbot fcp merge libs-api

[rust-rfcbot]

@dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @the8472

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[maxdexh]

Is there a reason custom allocators were not included in the implementation?

[nia-e]

...huh. It might honestly just be an oversight? I think it should be sound.

cc @Amanieu @BurntSushi @dtolnay @joshtriplett @the8472 (can't ping the old libs-api anymore) as ppl with checkboxes here - should we block this on adding an allocator param?

[maxdexh]

Allocators are not stable API anyway. The implementation looks simple enough that it should be possible to just jam it in, except that the transmute in Box::map needs to be into_raw_with... + from_raw_in (why is it even using transmute for this?)

If you want I can just write a PR right now

[nia-e]

Wouldn't it be breaking to move this later from an impl for Box<T> into impl for Box<T, A>? if not then ignore me ^^

[nia-e]

@bors r+

[rust-bors]

📌 Commit 82ee18a has been approved by nia-e

It is now in the queue for this repository.

[rust-timer]

Finished benchmarking commit (83074fe): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (secondary -2.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-3.6%, -1.2%]	10
All ❌✅ (primary)	-	-	0

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: missing data
Artifact size: 400.59 MiB -> 400.54 MiB (-0.01%)