Suggest parentheses for possible range method calling by chenyukang · Pull Request #102454 · rust-lang/rust

chenyukang · 2022-09-29T05:18:26Z

Fixes #102396

rust-highfive · 2022-09-29T05:18:29Z

r? @TaKO8Ki

(rust-highfive has picked a reviewer for you, use r? to override)

compiler/rustc_hir_analysis/src/check/method/suggest.rs

src/test/ui/methods/issues/issue-90315.rs

compiler/rustc_hir_analysis/src/check/method/suggest.rs

src/test/ui/methods/issues/issue-90315.rs

compiler/rustc_hir_analysis/src/check/method/suggest.rs

rustbot · 2022-10-01T08:26:26Z

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

compiler/rustc_hir_analysis/src/check/method/suggest.rs

bors · 2022-10-04T07:36:50Z

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

compiler/rustc_error_messages/locales/en-US/hir_analysis.ftl

chenyukang · 2022-10-13T04:17:32Z

r? @lcnr
Help me on merging it, afraid need to rebase with master again, 😁

lcnr

some small nits, then r=me

compiler/rustc_hir_analysis/src/check/method/suggest.rs

lcnr · 2022-10-17T08:32:28Z

compiler/rustc_hir_analysis/src/check/method/suggest.rs

+                    let pick =
+                        self.lookup_probe(span, item_name, range_ty, expr, ProbeScope::AllTraits);


can you emit a comment for why we're using ProbeScope::AllTraits here, I remember that you mentioned the reason for that before but I forgot that again 😁

also, i think ideally we would use probe_for_name directly here with IsSuggestion(true). Not sure what IsSuggestion does but we are emitting a suggestion here, so setting that to true seems like it should be done 😅

Seems IsSuggestion only used at here:

rust/compiler/rustc_hir_analysis/src/check/method/probe.rs

Line 1230 in 1536ab1

if !self.is_suggestion.0 && !unstable_candidates.is_empty() {

Use ProbeScope::TraitsInScope can not find the pick, but ProbeScope::AllTraits will find it.

it seems helpful to not emit unstable_name_collision_hints while looking for diagnostics, so that should still be done.

ProbeScope::TraitsInScope that feels weird and shouldn't be the case as Iterator is in scope. Is this still the case with the current code?

updated the code to use probe_for_name.

ProbeScope::TraitsInScope means find with the scope of expr,

rust/compiler/rustc_hir_analysis/src/check/method/probe.rs

Line 456 in 1536ab1

probe_cx.assemble_extension_candidates_for_traits_in_scope(scope_expr_id)

othewise try to find methods in all traits.

At this function, we are at a point of error handling of range type checking, since error happened so the Iterator is not in the scope? it's only my guess😁

compiler/rustc_error_messages/locales/en-US/hir_analysis.ftl

lcnr · 2022-10-17T12:03:13Z

@bors r+ rollup

bors · 2022-10-17T12:03:15Z

📌 Commit 151001c has been approved by lcnr

It is now in the queue for this repository.

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#102454 (Suggest parentheses for possible range method calling) - rust-lang#102466 (only allow `ConstEquate` with `feature(gce)`) - rust-lang#102945 (Do not register placeholder `RegionOutlives` obligations when `considering_regions` is false) - rust-lang#103091 (rustdoc: remove unused HTML class `sidebar-title`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 29, 2022

rust-highfive assigned TaKO8Ki Sep 29, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2022

chenyukang commented Sep 29, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

chenyukang force-pushed the fix-102396-missing-parentheses branch from 5039d7e to 04e6dc4 Compare September 29, 2022 06:01

Rageking8 reviewed Sep 29, 2022

View reviewed changes

chenyukang marked this pull request as draft September 30, 2022 07:28

chenyukang commented Sep 30, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

lcnr reviewed Sep 30, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

chenyukang force-pushed the fix-102396-missing-parentheses branch from 6c0766b to 2e717c6 Compare September 30, 2022 15:35

chenyukang marked this pull request as ready for review September 30, 2022 15:36

Rageking8 reviewed Sep 30, 2022

View reviewed changes

chenyukang force-pushed the fix-102396-missing-parentheses branch 2 times, most recently from 5ee7bdb to 348d082 Compare October 1, 2022 00:58

compiler-errors reviewed Oct 1, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Oct 1, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

chenyukang commented Oct 1, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

cjgillot reviewed Oct 1, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

chenyukang force-pushed the fix-102396-missing-parentheses branch from 348d082 to 86947ab Compare October 1, 2022 08:26

rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Oct 1, 2022

lcnr reviewed Oct 3, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

lcnr approved these changes Oct 3, 2022

View reviewed changes

compiler/rustc_hir_analysis/src/check/method/suggest.rs Outdated Show resolved Hide resolved

fix rust-lang#102396, suggest parentheses for possible range methods

5dd44d4

chenyukang force-pushed the fix-102396-missing-parentheses branch from 17f2311 to c141b82 Compare October 4, 2022 09:32

lcnr reviewed Oct 4, 2022

View reviewed changes

compiler/rustc_error_messages/locales/en-US/hir_analysis.ftl Outdated Show resolved Hide resolved

compiler/rustc_error_messages/locales/en-US/hir_analysis.ftl Outdated Show resolved Hide resolved

chenyukang force-pushed the fix-102396-missing-parentheses branch from c141b82 to 854844e Compare October 4, 2022 09:40

find the correct lang item for ranges

e747201

chenyukang force-pushed the fix-102396-missing-parentheses branch from 854844e to e747201 Compare October 4, 2022 15:38

rust-highfive assigned lcnr and unassigned TaKO8Ki Oct 13, 2022

trivial fix for comments feedback

151001c

lcnr reviewed Oct 17, 2022

View reviewed changes

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 17, 2022

matthiaskrgr mentioned this pull request Oct 17, 2022

Rollup of 4 pull requests #103151

Merged

bors merged commit 66de34b into rust-lang:master Oct 17, 2022

rustbot added this to the 1.66.0 milestone Oct 17, 2022

		let pick =
		self.lookup_probe(span, item_name, range_ty, expr, ProbeScope::AllTraits);

Uh oh!

Conversation

chenyukang commented Sep 29, 2022

Uh oh!

rust-highfive commented Sep 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustbot commented Oct 1, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Oct 4, 2022

Uh oh!

Uh oh!

Uh oh!

chenyukang commented Oct 13, 2022

Uh oh!

lcnr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lcnr Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

chenyukang Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

lcnr Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

chenyukang Oct 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lcnr commented Oct 17, 2022

Uh oh!

bors commented Oct 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants