smart pointer (try_)map by Qelxiros · Pull Request #144420 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation55 Commits1 Checks11 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

rustbot added S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.

labels

Jul 24, 2025

This comment has been minimized.

Qelxiros marked this pull request as ready for review

July 25, 2025 00:12

r? @thomcc

rustbot has assigned @thomcc.
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 added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

labels

Jul 25, 2025

As far as I can tell, the ICE here is because of the function signature of Box::try_map. The return type is equivalent to ChangeOutputType<...> from the ACP, but that definition is in core, so I implemented it manually here. However, when I add the same type alias to alloc, the ICE persists. Is there a workaround here that remains in the spirit of the ACP? Is this a known compiler bug?

@joboet joboet left a comment • Loading

Choose a reason for hiding this comment

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

This comment has been minimized.

I'm not sure how to move forward given the ICE (issue #146174).

@rustbot label +S-blocked

rustbot added the S-blocked

Status: Blocked on something else such as an RFC or other implementation work.

label

Sep 4, 2025

tgross35 added a commit to tgross35/rust that referenced this pull request

Sep 5, 2025

…iler-errors

fix ICE when suggesting ::new

fixes rust-lang#146174

This code suggests to write Foo::new(...) when the user writes Foo(...) or Foo { ... } and the constructor is private, where new is some associated function that returns Self.

When checking that the return type of new is Self, we need to instantiate the parameters of new with infer vars, so we don't end up with a type like Box<$param(0)> in a context that doesn't have any parameters. But then we can't use normalize_erasing_late_bound_regions anymore because that goes though a query that can't deal with infer vars.

Since this is diagnostic-only code that is supposed to check for exactly -> Self, I think it's fine to just skip normalizing here, especially since The Correct Way^TM would involve a probe and make this code even more complicated.

Also, the code here does almost the same thing, and these suggestions can probably be unified in the future: https://github.com/rust-lang/rust/blob/4ca8078d37c53ee4ff8fb32b4453b915116f25b8/compiler/rustc_hir_typeck/src/method/suggest.rs#L2123-L2129

r? @compiler-errors cc @Qelxiros -- this should unblock rust-lang#144420

tgross35 added a commit to tgross35/rust that referenced this pull request

Sep 5, 2025

…iler-errors

fix ICE when suggesting ::new

fixes rust-lang#146174

This code suggests to write Foo::new(...) when the user writes Foo(...) or Foo { ... } and the constructor is private, where new is some associated function that returns Self.

r? @compiler-errors cc @Qelxiros -- this should unblock rust-lang#144420

tgross35 added a commit to tgross35/rust that referenced this pull request

Sep 5, 2025

…iler-errors

fix ICE when suggesting ::new

fixes rust-lang#146174

This code suggests to write Foo::new(...) when the user writes Foo(...) or Foo { ... } and the constructor is private, where new is some associated function that returns Self.

r? @compiler-errors cc @Qelxiros -- this should unblock rust-lang#144420

rust-timer added a commit that referenced this pull request

Sep 5, 2025

Rollup merge of #146217 - lukas-code:suggest-new-ice, r=compiler-errors

fix ICE when suggesting ::new

fixes #146174

This code suggests to write Foo::new(...) when the user writes Foo(...) or Foo { ... } and the constructor is private, where new is some associated function that returns Self.

r? @compiler-errors cc @Qelxiros -- this should unblock #144420

This comment has been minimized.

I'm not sure if the transmute is worth the readability from using Box::take, or if transmute is the only way to go from Box<MaybeUninit<T>> to Box<MaybeUninit<U>>. Let me know if I'm missing something obvious.

transmute is fine. You could also use Box::from_raw(Box::into_raw(..) as ..), if you prefer.

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rustbot ready
edit: ignore me, I got my labels confused and thought I'd forgotten to do this.

@joboet joboet left a comment • Loading

Choose a reason for hiding this comment

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

&& Arc::is_unique(&this)
{
unsafe {
use core::mem::MaybeUninit;

Choose a reason for hiding this comment

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

Please move this import to the top of the file.

&& Arc::is_unique(&this)
{
unsafe {
use core::mem::MaybeUninit;

Choose a reason for hiding this comment

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

Ditto.

✌️ @Qelxiros, you can now approve this pull request!

If @joboet told you to "r=me" after making some further change, please make that change, then do @bors r=@joboet

📌 Commit 9c5be67 has been approved by joboet

It is now in the queue for this repository.

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

Oct 31, 2025

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Oct 31, 2025

bors added a commit that referenced this pull request

Oct 31, 2025

Rollup of 7 pull requests

Successful merges:

#139310 (add first HelenOS compilation targets)
#144420 (smart pointer (try_)map)
#145974 (Stabilize -Zno-jump-tables into -Cjump-tables=bool)
#147161 (implement VecDeque extend_from_within and prepend_from_within)
#147780 (Implement VecDeque::extract_if)
#148319 (docs: Fix argument names for carrying_mul_add)
#148322 (Enable file locking support in illumos)

r? @ghost @rustbot modify labels: rollup

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 51f5892 (parent) -> 82ae0ee (this PR)

Test differences

Show 994 test diffs

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

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml --
test-dashboard 82ae0ee6487e93bd6c05167ccb2ef3485fdbc890 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

dist-x86_64-apple: 7286.5s -> 8916.6s (+22.4%)
aarch64-apple: 6823.9s -> 8325.5s (+22.0%)
i686-gnu-nopt-1: 7847.9s -> 6802.5s (-13.3%)
test-various: 7105.6s -> 6160.7s (-13.3%)
x86_64-gnu-tools: 3474.8s -> 3068.9s (-11.7%)
x86_64-gnu-aux: 7047.9s -> 6274.4s (-11.0%)
x86_64-mingw-1: 9720.2s -> 10475.4s (+7.8%)
x86_64-gnu-nopt: 7244.4s -> 6719.1s (-7.3%)
dist-aarch64-linux: 6635.8s -> 6166.4s (-7.1%)
dist-ohos-x86_64: 4258.9s -> 3959.2s (-7.0%) 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.

This was referenced

Oct 31, 2025

Finished benchmarking commit (82ae0ee): 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 0.7%, secondary -1.9%)

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

mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.9%	[-1.9%, -1.9%]	1
All ❌✅ (primary)	0.7%	[0.7%, 0.7%]	1

Cycles

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

Binary size

Results (secondary 0.0%)

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.0%	[0.0%, 0.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Bootstrap: 474.501s -> 475.716s (0.26%)
Artifact size: 390.85 MiB -> 390.89 MiB (0.01%)

makai410 pushed a commit to makai410/rust that referenced this pull request

Nov 8, 2025

makai410 pushed a commit to makai410/rust that referenced this pull request

Nov 10, 2025

github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request

Nov 30, 2025

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.