Suggest calling `Self::associated_function()` #96507

TaKO8Ki · 2022-04-28T07:55:18Z

closes #96453

rust-highfive · 2022-04-28T07:55:22Z

r? @lcnr

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

lcnr

some small questions

compiler/rustc_resolve/src/late/diagnostics.rs

lcnr · 2022-04-28T09:34:01Z

compiler/rustc_resolve/src/late/diagnostics.rs

+                    && let FnCtxt::Assoc(_) = function_ctxt
+                    && let Some(item) = items.iter().find(|i| {
+                        if let AssocItemKind::Fn(fn_) = &i.kind
+                            && fn_.sig.decl.inputs.is_empty()


why do you only suggest that if there are no inputs?

I am not too familiar with this function

I fixed it to suggest that if inputs don't include self, &self, or &mut self.

lcnr · 2022-04-28T09:37:21Z

compiler/rustc_resolve/src/late/diagnostics.rs

@@ -147,10 +147,12 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
        let mut expected = source.descr_expected();
        let path_str = Segment::names_to_string(path);
        let item_str = path.last().unwrap().ident;
-        let (base_msg, fallback_label, base_span, could_be_expr) = if let Some(res) = res {
+        let (base_msg, fallback_label, base_span, could_be_expr, suggestion) = if let Some(res) =


i dislike large tuples like this. it's just personal preference, but what's your take on moving these 4 fields to a helper struct?

I agree with you. I'll fix it just like you said.

lcnr

after that r=me

lcnr · 2022-05-05T07:07:41Z

compiler/rustc_resolve/src/late/diagnostics.rs

+                debug!(
+                    "self.diagnostic_metadata.current_impl_items={:?}",
+                    self.diagnostic_metadata.current_impl_items
+                );
+                debug!(
+                    "self.diagnostic_metadata.current_function={:?}",
+                    self.diagnostic_metadata.current_function
+                );


Suggested change

debug!(

"self.diagnostic_metadata.current_impl_items={:?}",

self.diagnostic_metadata.current_impl_items

);

debug!(

"self.diagnostic_metadata.current_function={:?}",

self.diagnostic_metadata.current_function

);

debug!(?self.diagnostic_metadata.current_impl_items);

debug!(?self.diagnostic_metadata.current_function);

lcnr · 2022-05-05T07:15:17Z

compiler/rustc_resolve/src/late/diagnostics.rs

+                            && fn_.sig.decl.inputs.iter().all(|i| match &i.ty.kind {
+                                TyKind::Rptr(_, ty) => match ty.ty.kind {
+                                    TyKind::ImplicitSelf => false,
+                                    _ => true,
+                                }
+                                TyKind::ImplicitSelf => false,
+                                _ => true
+                            })


Suggested change

&& fn_.sig.decl.inputs.iter().all(|i| match &i.ty.kind {

TyKind::Rptr(_, ty) => match ty.ty.kind {

TyKind::ImplicitSelf => false,

_ => true,

}

TyKind::ImplicitSelf => false,

_ => true

})

&& !fn_.sig.decl.has_self()

lcnr · 2022-05-05T07:15:48Z

compiler/rustc_resolve/src/late/diagnostics.rs

+                            debug!("name={}", item_str.name);
+                            debug!("fn_.sig.decl.inputs={:?}", fn_.sig.decl.inputs);


Suggested change

debug!("name={}", item_str.name);

debug!("fn_.sig.decl.inputs={:?}", fn_.sig.decl.inputs);

debug!(?item_str.name);

debug!(?fn_.sig.decl.inputs);

or completely remove these debug statements '^^

do not suggest when trait_ref is some Update compiler/rustc_resolve/src/late/diagnostics.rs Co-authored-by: lcnr <[email protected]> use helper struct add a test for functions with some params refactor debug log

TaKO8Ki · 2022-05-05T08:43:21Z

@bors r=lcnr

bors · 2022-05-05T08:43:23Z

@TaKO8Ki: 🔑 Insufficient privileges: Not in reviewers

lcnr · 2022-05-05T08:56:08Z

Thanks 👍

@bors r+ rollup

lcnr · 2022-05-05T10:23:59Z

@bors r+ rollup

bors · 2022-05-05T10:24:00Z

📌 Commit df25189 has been approved by lcnr

…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#95359 (Update `int_roundings` methods from feedback) - rust-lang#95843 (Improve Rc::new_cyclic and Arc::new_cyclic documentation) - rust-lang#96507 (Suggest calling `Self::associated_function()`) - rust-lang#96635 (Use "strict" mode in JS scripts) - rust-lang#96673 (Report that opaque types are not allowed in impls even in the presence of other errors) - rust-lang#96682 (Show invisible delimeters (within comments) when pretty printing.) - rust-lang#96714 (interpret/validity: debug-check ScalarPair layout information) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned lcnr Apr 28, 2022

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

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

lcnr reviewed Apr 28, 2022

View reviewed changes

TaKO8Ki requested a review from lcnr May 4, 2022 14:54

TaKO8Ki force-pushed the suggest-calling-associated-function branch from 59b8a24 to bdd3cfb Compare May 4, 2022 14:54

lcnr reviewed May 5, 2022

View reviewed changes

suggest calling Self::associated_function()

df25189

do not suggest when trait_ref is some Update compiler/rustc_resolve/src/late/diagnostics.rs Co-authored-by: lcnr <[email protected]> use helper struct add a test for functions with some params refactor debug log

TaKO8Ki force-pushed the suggest-calling-associated-function branch from bdd3cfb to df25189 Compare May 5, 2022 07:44

lcnr closed this May 5, 2022

lcnr reopened this May 5, 2022

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 May 5, 2022

matthiaskrgr mentioned this pull request May 5, 2022

Rollup of 7 pull requests #96734

Merged

bors merged commit 34bf620 into rust-lang:master May 5, 2022

rustbot added this to the 1.62.0 milestone May 5, 2022

TaKO8Ki deleted the suggest-calling-associated-function branch May 6, 2022 00:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggest calling `Self::associated_function()` #96507

Suggest calling `Self::associated_function()` #96507

TaKO8Ki commented Apr 28, 2022

rust-highfive commented Apr 28, 2022

lcnr left a comment

lcnr Apr 28, 2022

TaKO8Ki May 5, 2022 •

edited

Loading

lcnr Apr 28, 2022

TaKO8Ki May 2, 2022

lcnr left a comment

lcnr May 5, 2022

lcnr May 5, 2022

lcnr May 5, 2022

TaKO8Ki commented May 5, 2022

bors commented May 5, 2022

lcnr commented May 5, 2022

lcnr commented May 5, 2022

bors commented May 5, 2022

		debug!("name={}", item_str.name);
		debug!("fn_.sig.decl.inputs={:?}", fn_.sig.decl.inputs);

Suggest calling Self::associated_function() #96507

Suggest calling Self::associated_function() #96507

Conversation

TaKO8Ki commented Apr 28, 2022

rust-highfive commented Apr 28, 2022

lcnr left a comment

Choose a reason for hiding this comment

lcnr Apr 28, 2022

Choose a reason for hiding this comment

TaKO8Ki May 5, 2022 • edited Loading

Choose a reason for hiding this comment

lcnr Apr 28, 2022

Choose a reason for hiding this comment

TaKO8Ki May 2, 2022

Choose a reason for hiding this comment

lcnr left a comment

Choose a reason for hiding this comment

lcnr May 5, 2022

Choose a reason for hiding this comment

lcnr May 5, 2022

Choose a reason for hiding this comment

lcnr May 5, 2022

Choose a reason for hiding this comment

TaKO8Ki commented May 5, 2022

bors commented May 5, 2022

lcnr commented May 5, 2022

lcnr commented May 5, 2022

bors commented May 5, 2022

Suggest calling `Self::associated_function()` #96507

Suggest calling `Self::associated_function()` #96507

TaKO8Ki May 5, 2022 •

edited

Loading