Skip to content

Commit

Permalink
Unrolled build for rust-lang#127431
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#127431 - oli-obk:feed_item_attrs, r=compiler-errors

Use field ident spans directly instead of the full field span in diagnostics on local fields

This improves diagnostics and avoids having to store the `DefId`s of fields
  • Loading branch information
rust-timer committed Jul 8, 2024
2 parents 9af6fee + 8c2ea71 commit d0cab42
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 45 deletions.
10 changes: 8 additions & 2 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,14 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
// The fields are not expanded yet.
return;
}
let def_ids = fields.iter().map(|field| self.r.local_def_id(field.id).to_def_id());
self.r.field_def_ids.insert(def_id, self.r.tcx.arena.alloc_from_iter(def_ids));
let fields = fields
.iter()
.enumerate()
.map(|(i, field)| {
field.ident.unwrap_or_else(|| Ident::from_str_and_span(&format!("{i}"), field.span))
})
.collect();
self.r.field_names.insert(def_id, fields);
}

fn insert_field_visibilities_local(&mut self, def_id: DefId, fields: &[ast::FieldDef]) {
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1726,11 +1726,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
)) = binding.kind
{
let def_id = self.tcx.parent(ctor_def_id);
return self
.field_def_ids(def_id)?
.iter()
.map(|&field_id| self.def_span(field_id))
.reduce(Span::to); // None for `struct Foo()`
return self.field_idents(def_id)?.iter().map(|&f| f.span).reduce(Span::to); // None for `struct Foo()`
}
None
}
Expand Down
44 changes: 19 additions & 25 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1532,17 +1532,17 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
if !this.has_private_fields(def_id) {
// If the fields of the type are private, we shouldn't be suggesting using
// the struct literal syntax at all, as that will cause a subsequent error.
let field_ids = this.r.field_def_ids(def_id);
let (fields, applicability) = match field_ids {
Some(field_ids) => {
let fields = field_ids.iter().map(|&id| this.r.tcx.item_name(id));

let fields = this.r.field_idents(def_id);
let has_fields = fields.as_ref().is_some_and(|f| !f.is_empty());
let (fields, applicability) = match fields {
Some(fields) => {
let fields = if let Some(old_fields) = old_fields {
fields
.iter()
.enumerate()
.map(|(idx, new)| (new, old_fields.get(idx)))
.map(|(new, old)| {
let new = new.to_ident_string();
let new = new.name.to_ident_string();
if let Some(Some(old)) = old
&& new != *old
{
Expand All @@ -1553,17 +1553,17 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
})
.collect::<Vec<String>>()
} else {
fields.map(|f| format!("{f}{tail}")).collect::<Vec<String>>()
fields
.iter()
.map(|f| format!("{f}{tail}"))
.collect::<Vec<String>>()
};

(fields.join(", "), applicability)
}
None => ("/* fields */".to_string(), Applicability::HasPlaceholders),
};
let pad = match field_ids {
Some([]) => "",
_ => " ",
};
let pad = if has_fields { " " } else { "" };
err.span_suggestion(
span,
format!("use struct {descr} syntax instead"),
Expand Down Expand Up @@ -1723,12 +1723,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
&args[..],
);
// Use spans of the tuple struct definition.
self.r.field_def_ids(def_id).map(|field_ids| {
field_ids
.iter()
.map(|&field_id| self.r.def_span(field_id))
.collect::<Vec<_>>()
})
self.r
.field_idents(def_id)
.map(|fields| fields.iter().map(|f| f.span).collect::<Vec<_>>())
}
_ => None,
};
Expand Down Expand Up @@ -1791,7 +1788,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
(Res::Def(DefKind::Ctor(_, CtorKind::Fn), ctor_def_id), _) if ns == ValueNS => {
let def_id = self.r.tcx.parent(ctor_def_id);
err.span_label(self.r.def_span(def_id), format!("`{path_str}` defined here"));
let fields = self.r.field_def_ids(def_id).map_or_else(
let fields = self.r.field_idents(def_id).map_or_else(
|| "/* fields */".to_string(),
|field_ids| vec!["_"; field_ids.len()].join(", "),
);
Expand Down Expand Up @@ -2017,12 +2014,9 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
if let Some(Res::Def(DefKind::Struct | DefKind::Union, did)) =
resolution.full_res()
{
if let Some(field_ids) = self.r.field_def_ids(did) {
if let Some(field_id) = field_ids
.iter()
.find(|&&field_id| ident.name == self.r.tcx.item_name(field_id))
{
return Some(AssocSuggestion::Field(self.r.def_span(*field_id)));
if let Some(fields) = self.r.field_idents(did) {
if let Some(field) = fields.iter().find(|id| ident.name == id.name) {
return Some(AssocSuggestion::Field(field.span));
}
}
}
Expand Down Expand Up @@ -2418,7 +2412,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
match kind {
CtorKind::Const => false,
CtorKind::Fn => {
!self.r.field_def_ids(def_id).is_some_and(|field_ids| field_ids.is_empty())
!self.r.field_idents(def_id).is_some_and(|field_ids| field_ids.is_empty())
}
}
};
Expand Down
18 changes: 13 additions & 5 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ pub struct Resolver<'a, 'tcx> {
extern_prelude: FxHashMap<Ident, ExternPreludeEntry<'a>>,

/// N.B., this is used only for better diagnostics, not name resolution itself.
field_def_ids: LocalDefIdMap<&'tcx [DefId]>,
field_names: LocalDefIdMap<Vec<Ident>>,

/// Span of the privacy modifier in fields of an item `DefId` accessible with dot syntax.
/// Used for hints during error reporting.
Expand Down Expand Up @@ -1407,7 +1407,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
prelude: None,
extern_prelude,

field_def_ids: Default::default(),
field_names: Default::default(),
field_visibility_spans: FxHashMap::default(),

determined_imports: Vec::new(),
Expand Down Expand Up @@ -2128,10 +2128,18 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

fn field_def_ids(&self, def_id: DefId) -> Option<&'tcx [DefId]> {
fn field_idents(&self, def_id: DefId) -> Option<Vec<Ident>> {
match def_id.as_local() {
Some(def_id) => self.field_def_ids.get(&def_id).copied(),
None => Some(self.tcx.associated_item_def_ids(def_id)),
Some(def_id) => self.field_names.get(&def_id).cloned(),
None => Some(
self.tcx
.associated_item_def_ids(def_id)
.iter()
.map(|&def_id| {
Ident::new(self.tcx.item_name(def_id), self.tcx.def_span(def_id))
})
.collect(),
),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0425]: cannot find value `field` in this scope
--> $DIR/field-and-method-in-self-not-available-in-assoc-fn.rs:11:9
|
LL | field: u32,
| ---------- a field by that name exists in `Self`
| ----- a field by that name exists in `Self`
...
LL | fn field(&self) -> u32 {
| ----- a method by that name is available on `Self` here
Expand All @@ -14,7 +14,7 @@ error[E0425]: cannot find value `field` in this scope
--> $DIR/field-and-method-in-self-not-available-in-assoc-fn.rs:12:15
|
LL | field: u32,
| ---------- a field by that name exists in `Self`
| ----- a field by that name exists in `Self`
...
LL | fn field(&self) -> u32 {
| ----- a method by that name is available on `Self` here
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/resolve/issue-2356.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0425]: cannot find value `whiskers` in this scope
--> $DIR/issue-2356.rs:39:5
|
LL | whiskers: isize,
| --------------- a field by that name exists in `Self`
| -------- a field by that name exists in `Self`
...
LL | whiskers -= other;
| ^^^^^^^^
Expand Down Expand Up @@ -35,7 +35,7 @@ error[E0425]: cannot find value `whiskers` in this scope
--> $DIR/issue-2356.rs:84:5
|
LL | whiskers: isize,
| --------------- a field by that name exists in `Self`
| -------- a field by that name exists in `Self`
...
LL | whiskers = 4;
| ^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/resolve/issue-60057.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0425]: cannot find value `banana` in this scope
--> $DIR/issue-60057.rs:8:21
|
LL | banana: u8,
| ---------- a field by that name exists in `Self`
| ------ a field by that name exists in `Self`
...
LL | banana: banana
| ^^^^^^
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0425]: cannot find value `config` in this scope
--> $DIR/typo-suggestion-for-variable-with-name-similar-to-struct-field.rs:7:16
|
LL | config: String,
| -------------- a field by that name exists in `Self`
| ------ a field by that name exists in `Self`
...
LL | Self { config }
| ^^^^^^ help: a local variable with a similar name exists: `cofig`
Expand All @@ -11,7 +11,7 @@ error[E0425]: cannot find value `config` in this scope
--> $DIR/typo-suggestion-for-variable-with-name-similar-to-struct-field.rs:11:20
|
LL | config: String,
| -------------- a field by that name exists in `Self`
| ------ a field by that name exists in `Self`
...
LL | println!("{config}");
| ^^^^^^ help: a local variable with a similar name exists: `cofig`
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/resolve/unresolved_static_type_field.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0425]: cannot find value `cx` in this scope
--> $DIR/unresolved_static_type_field.rs:9:11
|
LL | cx: bool,
| -------- a field by that name exists in `Self`
| -- a field by that name exists in `Self`
...
LL | f(cx);
| ^^
Expand Down

0 comments on commit d0cab42

Please sign in to comment.