Provide suggestion to dereference closure tail if appropriate #122213

estebank · 2024-03-08T23:00:07Z

When encoutnering a case like

use std::collections::HashMap;

fn main() {
    let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3];

    let mut counts = HashMap::new();
    for num in vs {
        let count = counts.entry(num).or_insert(0);
        *count += 1;
    }

    let _ = counts.iter().max_by_key(|(_, v)| v);

produce the following suggestion

error: lifetime may not live long enough
  --> $DIR/return-value-lifetime-error.rs:13:47
   |
LL |     let _ = counts.iter().max_by_key(|(_, v)| v);
   |                                       ------- ^ returning this value requires that `'1` must outlive `'2`
   |                                       |     |
   |                                       |     return type of closure is &'2 &i32
   |                                       has type `&'1 (&i32, &i32)`
   |
help: dereference the return value
   |
LL |     let _ = counts.iter().max_by_key(|(_, v)| **v);
   |                                               ++

Fix #50195.

rustbot · 2024-03-08T23:00:16Z

r? @oli-obk

rustbot has assigned @oli-obk.
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 · 2024-03-08T23:00:17Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

estebank · 2024-03-08T23:02:04Z

The checks to verify that the value can indeed be dereferenced and it will continue to satisfy the trait bounds from the method are a bit much, and we could remove them in favor of a more approximate check ("when closure return value, always suggest dereference"?).

The logic as written will not work for a case where a single level of dereferencing would work with the existing lifetimes.

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

When encoutnering a case like ```rust //@ run-rustfix use std::collections::HashMap; fn main() { let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3]; let mut counts = HashMap::new(); for num in vs { let count = counts.entry(num).or_insert(0); *count += 1; } let _ = counts.iter().max_by_key(|(_, v)| v); ``` produce the following suggestion ``` error: lifetime may not live long enough --> $DIR/return-value-lifetime-error.rs:13:47 | LL | let _ = counts.iter().max_by_key(|(_, v)| v); | ------- ^ returning this value requires that `'1` must outlive `'2` | | | | | return type of closure is &'2 &i32 | has type `&'1 (&i32, &i32)` | help: dereference the return value | LL | let _ = counts.iter().max_by_key(|(_, v)| **v); | ++ ``` Fix rust-lang#50195.

Stop exporting `TypeckRootCtxt` and `FnCtxt`. While they have many convenient APIs, it is better to expose dedicated functions for them noticed in rust-lang#122213

Rollup merge of rust-lang#123625 - oli-obk:private_fnctxt, r=fee1-dead Stop exporting `TypeckRootCtxt` and `FnCtxt`. While they have many convenient APIs, it is better to expose dedicated functions for them noticed in rust-lang#122213

…ait solver

oli-obk · 2024-04-10T05:24:46Z

Sorry it took so long to review, but the hack just felt like something the compiler should be able to answer for us. It now does so

estebank · 2024-04-10T23:36:20Z

@bors r=oli-obk,estebank

bors · 2024-04-10T23:36:23Z

📌 Commit f2465f8 has been approved by oli-obk,estebank

It is now in the queue for this repository.

bors · 2024-04-10T23:50:52Z

⌛ Testing commit f2465f8 with merge 6766970...

…bank Provide suggestion to dereference closure tail if appropriate When encoutnering a case like ```rust use std::collections::HashMap; fn main() { let vs = vec![0, 0, 1, 1, 3, 4, 5, 6, 3, 3, 3]; let mut counts = HashMap::new(); for num in vs { let count = counts.entry(num).or_insert(0); *count += 1; } let _ = counts.iter().max_by_key(|(_, v)| v); ``` produce the following suggestion ``` error: lifetime may not live long enough --> $DIR/return-value-lifetime-error.rs:13:47 | LL | let _ = counts.iter().max_by_key(|(_, v)| v); | ------- ^ returning this value requires that `'1` must outlive `'2` | | | | | return type of closure is &'2 &i32 | has type `&'1 (&i32, &i32)` | help: dereference the return value | LL | let _ = counts.iter().max_by_key(|(_, v)| **v); | ++ ``` Fix rust-lang#50195.

rust-log-analyzer · 2024-04-11T00:13:45Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

bors · 2024-04-11T00:14:16Z

💔 Test failed - checks-actions

estebank · 2024-04-11T01:37:54Z

@bors retry

bors · 2024-04-11T02:37:38Z

⌛ Testing commit f2465f8 with merge 0827378...

bors · 2024-04-11T04:40:33Z

☀️ Test successful - checks-actions
Approved by: oli-obk,estebank
Pushing 0827378 to master...

rust-timer · 2024-04-11T06:15:20Z

Finished benchmarking commit (0827378): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

Max RSS (memory usage)

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

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: 675.519s -> 676.971s (0.21%)
Artifact size: 315.99 MiB -> 316.01 MiB (0.01%)

…t-deref, r=oli-obk Make `suggest_deref_closure_return` more idiomatic/easier to understand The only functional change here really is just making it not use a fresh type variable for upvars. I'll point that out in the code. The rest of the changes are just stylistic, because reading this code was really confusing me (variable names were vague, ways of accessing types were unidiomatic, order of operations was kind of strange, etc). This is stacked on rust-lang#123989. r? oli-obk since you approved rust-lang#122213

Rollup merge of rust-lang#123990 - compiler-errors:suggest-closure-ret-deref, r=oli-obk Make `suggest_deref_closure_return` more idiomatic/easier to understand The only functional change here really is just making it not use a fresh type variable for upvars. I'll point that out in the code. The rest of the changes are just stylistic, because reading this code was really confusing me (variable names were vague, ways of accessing types were unidiomatic, order of operations was kind of strange, etc). This is stacked on rust-lang#123989. r? oli-obk since you approved rust-lang#122213

Stop exporting `TypeckRootCtxt` and `FnCtxt`. While they have many convenient APIs, it is better to expose dedicated functions for them noticed in rust-lang#122213

rustbot assigned oli-obk Mar 8, 2024

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. labels Mar 8, 2024

oli-obk reviewed Mar 11, 2024

View reviewed changes

compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved

oli-obk reviewed Mar 11, 2024

View reviewed changes

compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved

oli-obk reviewed Mar 11, 2024

View reviewed changes

compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved

surechen reviewed Apr 3, 2024

View reviewed changes

compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved

estebank force-pushed the issue-50195 branch from f06c0a8 to 0d9be34 Compare April 4, 2024 13:14

This comment has been minimized.

Sign in to view

estebank force-pushed the issue-50195 branch from 0d9be34 to d37100e Compare April 5, 2024 15:35

estebank force-pushed the issue-50195 branch from d37100e to 9de6b70 Compare April 5, 2024 19:43

oli-obk mentioned this pull request Apr 8, 2024

Stop exporting TypeckRootCtxt and FnCtxt. #123625

Merged

Use trait solver to answer questions instead of manually writing a tr…

f2465f8

…ait solver

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 Apr 10, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 11, 2024

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 Apr 11, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 11, 2024

bors merged commit 0827378 into rust-lang:master Apr 11, 2024
12 checks passed

rustbot added this to the 1.79.0 milestone Apr 11, 2024

compiler-errors mentioned this pull request Apr 15, 2024

Make suggest_deref_closure_return more idiomatic/easier to understand #123990

Merged

lqd mentioned this pull request Apr 30, 2024

ICE when using associated types with lifetimes #124563

Closed

lqd mentioned this pull request May 28, 2024

ICE: expected region for .. #125634

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide suggestion to dereference closure tail if appropriate #122213

Provide suggestion to dereference closure tail if appropriate #122213

estebank commented Mar 8, 2024

rustbot commented Mar 8, 2024

rustbot commented Mar 8, 2024

estebank commented Mar 8, 2024 •

edited

Loading

This comment has been minimized.

oli-obk commented Apr 10, 2024

estebank commented Apr 10, 2024

bors commented Apr 10, 2024

bors commented Apr 10, 2024

rust-log-analyzer commented Apr 11, 2024

bors commented Apr 11, 2024

estebank commented Apr 11, 2024

bors commented Apr 11, 2024

bors commented Apr 11, 2024

rust-timer commented Apr 11, 2024

Provide suggestion to dereference closure tail if appropriate #122213

Provide suggestion to dereference closure tail if appropriate #122213

Conversation

estebank commented Mar 8, 2024

rustbot commented Mar 8, 2024

rustbot commented Mar 8, 2024

estebank commented Mar 8, 2024 • edited Loading

This comment has been minimized.

oli-obk commented Apr 10, 2024

estebank commented Apr 10, 2024

bors commented Apr 10, 2024

bors commented Apr 10, 2024

rust-log-analyzer commented Apr 11, 2024

bors commented Apr 11, 2024

estebank commented Apr 11, 2024

bors commented Apr 11, 2024

bors commented Apr 11, 2024

rust-timer commented Apr 11, 2024

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

estebank commented Mar 8, 2024 •

edited

Loading