diagnostics: be more careful when suggesting struct fields #116429

fmease · 2023-10-04T19:08:11Z

Consolidate the various places which filter out struct fields that shouldn't be suggested into a single function.

Previously, each of those code paths had slightly different and incomplete metrics for no good reason. Now, there's only a single 'complete' metric (namely is_field_suggestable) which also filters out hygienic fields that come from different syntax contexts.

Fixes #116334.

rustbot · 2023-10-04T19:08:19Z

r? @oli-obk

(rustbot has picked a reviewer for you, use r? to override)

compiler-errors · 2023-10-04T19:11:48Z

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+            // If the field is from an external crate it must not be `doc(hidden)`.
+            && (field.did.is_local() || !self.tcx.is_doc_hidden(field.did))
+            // If the field is hygienic it must not originate from a macro expansion.
+            && !self.tcx.def_ident_span(field.did).unwrap().normalize_to_macros_2_0().from_expansion()


should we check that the span ctxts are the same? Does this affect our ability to suggest hygenic fields from within a macro?

Sorry, spans are the field ident and the span of the (expr) hir id passed in.

Have to check, good points (I had taken inspiration from 1536d72).

Maybe test something like:

#![feature(decl_macro)] pub macro foo() { fn test() { struct Hello { world: () } let x: Hello = Hello { worl: () }; } } foo!();

Yep, you're right. It's obvious in hindsight. I need to tweak the logic.

No worries. We have lots of diagnostics that are subtly wrong due to being overly defensive about macros, resulting them just totally not applying within macros even when they should. Thx for investigating.

fmease · 2023-10-04T19:38:48Z

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

+        span: Span,
+    ) -> bool {
+        // The field must be visible in the containing module.
+        field.vis.is_accessible_from(self.tcx.parent_module(hir_id), self.tcx)


I've taken is_accessible_from(parent_module()) from one of the places I've removed but another one tried to obtain the DefScope instead of the parent module (via adjust_ident_and_get_scope). Which one is more correct / general?

I think the latter, but I doubt it matters. If something is reachable from an item then it's reachable from its parent module, no?

I wonder if there are some edge cases like structss defined inside of a fn instead of a module. Not sure.

compiler-errors

r=me

fmease · 2023-10-04T20:09:48Z

@bors r=compiler-errors

bors · 2023-10-04T20:09:50Z

📌 Commit 867cc41 has been approved by compiler-errors

It is now in the queue for this repository.

…, r=compiler-errors Diagnostics: Be more careful when suggesting struct fields Consolidate the various places which filter out struct fields that shouldn't be suggested into a single function. Previously, each of those code paths had slightly different and incomplete metrics for no good reason. Now, there's only a single 'complete' metric (namely `is_field_suggestable`) which also filters out hygienic fields that come from different syntax contexts. Fixes rust-lang#116334.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#116223 (Fix misuses of a vs an) - rust-lang#116296 (More accurately point to where default return type should go) - rust-lang#116402 (Panic when the global allocator tries to register a TLS destructor) - rust-lang#116429 (Diagnostics: Be more careful when suggesting struct fields) - rust-lang#116431 (Tweak wording of E0562) - rust-lang#116432 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 2)) r? `@ghost` `@rustbot` modify labels: rollup

…, r=compiler-errors Diagnostics: Be more careful when suggesting struct fields Consolidate the various places which filter out struct fields that shouldn't be suggested into a single function. Previously, each of those code paths had slightly different and incomplete metrics for no good reason. Now, there's only a single 'complete' metric (namely `is_field_suggestable`) which also filters out hygienic fields that come from different syntax contexts. Fixes rust-lang#116334.

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#115304 (Allow file names to end with '>') - rust-lang#116223 (Fix misuses of a vs an) - rust-lang#116296 (More accurately point to where default return type should go) - rust-lang#116429 (Diagnostics: Be more careful when suggesting struct fields) - rust-lang#116431 (Tweak wording of E0562) - rust-lang#116432 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 2)) r? `@ghost` `@rustbot` modify labels: rollup

…kingjubilee Rollup of 5 pull requests Successful merges: - rust-lang#116223 (Fix misuses of a vs an) - rust-lang#116296 (More accurately point to where default return type should go) - rust-lang#116429 (Diagnostics: Be more careful when suggesting struct fields) - rust-lang#116431 (Tweak wording of E0562) - rust-lang#116432 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 2)) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#116429 - fmease:clean-up-struct-field-suggs, r=compiler-errors Diagnostics: Be more careful when suggesting struct fields Consolidate the various places which filter out struct fields that shouldn't be suggested into a single function. Previously, each of those code paths had slightly different and incomplete metrics for no good reason. Now, there's only a single 'complete' metric (namely `is_field_suggestable`) which also filters out hygienic fields that come from different syntax contexts. Fixes rust-lang#116334.

rustbot assigned oli-obk Oct 4, 2023

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 Oct 4, 2023

fmease added the A-diagnostics Area: Messages for errors, warnings, and lints label Oct 4, 2023

compiler-errors reviewed Oct 4, 2023

View reviewed changes

compiler-errors assigned compiler-errors and unassigned oli-obk Oct 4, 2023

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 4, 2023

clean up struct field suggestions

867cc41

fmease force-pushed the clean-up-struct-field-suggs branch from 6b508b8 to 867cc41 Compare October 4, 2023 19:36

fmease commented Oct 4, 2023

View reviewed changes

fmease 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 Oct 4, 2023

compiler-errors approved these changes Oct 4, 2023

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 4, 2023

matthiaskrgr mentioned this pull request Oct 5, 2023

Rollup of 6 pull requests #116441

Closed

matthiaskrgr mentioned this pull request Oct 5, 2023

Rollup of 6 pull requests #116442

Closed

workingjubilee mentioned this pull request Oct 5, 2023

Rollup of 5 pull requests #116443

Merged

bors merged commit a9a389c into rust-lang:master Oct 5, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 5, 2023

fmease deleted the clean-up-struct-field-suggs branch October 5, 2023 09:59

pvdrz mentioned this pull request Oct 6, 2023

Fix divergence from upstream master ferrocene/ferrocene#23

Merged

fmease changed the title ~~Diagnostics: Be more careful when suggesting struct fields~~ diagnostics: be more careful when suggesting struct fields Oct 9, 2023

fmease mentioned this pull request Oct 9, 2023

diagnostic: properly deal with hygienic names on unresolved fields and imports #116575

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

diagnostics: be more careful when suggesting struct fields #116429

diagnostics: be more careful when suggesting struct fields #116429

fmease commented Oct 4, 2023 •

edited

rustbot commented Oct 4, 2023

compiler-errors Oct 4, 2023

compiler-errors Oct 4, 2023

fmease Oct 4, 2023 •

edited

compiler-errors Oct 4, 2023

fmease Oct 4, 2023 •

edited

compiler-errors Oct 4, 2023 •

edited

fmease Oct 4, 2023 •

edited

compiler-errors Oct 4, 2023

fmease Oct 4, 2023

compiler-errors left a comment

fmease commented Oct 4, 2023

bors commented Oct 4, 2023

diagnostics: be more careful when suggesting struct fields #116429

diagnostics: be more careful when suggesting struct fields #116429

Conversation

fmease commented Oct 4, 2023 • edited

rustbot commented Oct 4, 2023

compiler-errors Oct 4, 2023

Choose a reason for hiding this comment

compiler-errors Oct 4, 2023

Choose a reason for hiding this comment

fmease Oct 4, 2023 • edited

Choose a reason for hiding this comment

compiler-errors Oct 4, 2023

Choose a reason for hiding this comment

fmease Oct 4, 2023 • edited

Choose a reason for hiding this comment

compiler-errors Oct 4, 2023 • edited

Choose a reason for hiding this comment

fmease Oct 4, 2023 • edited

Choose a reason for hiding this comment

compiler-errors Oct 4, 2023

Choose a reason for hiding this comment

fmease Oct 4, 2023

Choose a reason for hiding this comment

compiler-errors left a comment

Choose a reason for hiding this comment

fmease commented Oct 4, 2023

bors commented Oct 4, 2023

fmease commented Oct 4, 2023 •

edited

fmease Oct 4, 2023 •

edited

fmease Oct 4, 2023 •

edited

compiler-errors Oct 4, 2023 •

edited

fmease Oct 4, 2023 •

edited