Skip to content

Commit a3cfb45

Browse files
borsbors
committed
Auto merge of rust-lang#15584 - Veykril:diag-private-field-constructor, r=Veykril
Diagnose private fields in record constructor
2 parents c405509 + 8f5fee4 commit a3cfb45

File tree

11 files changed

+258
-90
lines changed

11 files changed

+258
-90
lines changed

‎crates/hir-def/src/body.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ pub type LabelSource = InFile<LabelPtr>;
6565

6666
pub type FieldPtr = AstPtr<ast::RecordExprField>;
6767
pub type FieldSource = InFile<FieldPtr>;
68+
pub type PatFieldPtr = AstPtr<ast::RecordPatField>;
69+
pub type PatFieldSource = InFile<PatFieldPtr>;
6870

6971
/// An item body together with the mapping from syntax nodes to HIR expression
7072
/// IDs. This is needed to go from e.g. a position in a file to the HIR
@@ -90,8 +92,8 @@ pub struct BodySourceMap {
9092

9193
/// We don't create explicit nodes for record fields (`S { record_field: 92 }`).
9294
/// Instead, we use id of expression (`92`) to identify the field.
93-
field_map: FxHashMap<FieldSource, ExprId>,
9495
field_map_back: FxHashMap<ExprId, FieldSource>,
96+
pat_field_map_back: FxHashMap<PatId, PatFieldSource>,
9597

9698
expansions: FxHashMap<InFile<AstPtr<ast::MacroCall>>, HirFileId>,
9799

@@ -375,9 +377,8 @@ impl BodySourceMap {
375377
self.field_map_back[&expr].clone()
376378
}
377379

378-
pub fn node_field(&self, node: InFile<&ast::RecordExprField>) -> Option<ExprId> {
379-
let src = node.map(AstPtr::new);
380-
self.field_map.get(&src).cloned()
380+
pub fn pat_field_syntax(&self, pat: PatId) -> PatFieldSource {
381+
self.pat_field_map_back[&pat].clone()
381382
}
382383

383384
pub fn macro_expansion_expr(&self, node: InFile<&ast::MacroExpr>) -> Option<ExprId> {

‎crates/hir-def/src/body/lower.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,6 @@ impl ExprCollector<'_> {
446446
None => self.missing_expr(),
447447
};
448448
let src = self.expander.to_source(AstPtr::new(&field));
449-
self.source_map.field_map.insert(src.clone(), expr);
450449
self.source_map.field_map_back.insert(expr, src);
451450
Some(RecordLitField { name, expr })
452451
})
@@ -1330,23 +1329,21 @@ impl ExprCollector<'_> {
13301329
ast::Pat::RecordPat(p) => {
13311330
let path =
13321331
p.path().and_then(|path| self.expander.parse_path(self.db, path)).map(Box::new);
1333-
let args = p
1334-
.record_pat_field_list()
1335-
.expect("every struct should have a field list")
1332+
let record_pat_field_list =
1333+
&p.record_pat_field_list().expect("every struct should have a field list");
1334+
let args = record_pat_field_list
13361335
.fields()
13371336
.filter_map(|f| {
13381337
let ast_pat = f.pat()?;
13391338
let pat = self.collect_pat(ast_pat, binding_list);
13401339
let name = f.field_name()?.as_name();
1340+
let src = self.expander.to_source(AstPtr::new(&f));
1341+
self.source_map.pat_field_map_back.insert(pat, src);
13411342
Some(RecordFieldPat { name, pat })
13421343
})
13431344
.collect();
13441345

1345-
let ellipsis = p
1346-
.record_pat_field_list()
1347-
.expect("every struct should have a field list")
1348-
.rest_pat()
1349-
.is_some();
1346+
let ellipsis = record_pat_field_list.rest_pat().is_some();
13501347

13511348
Pat::Record { path, args, ellipsis }
13521349
}

‎crates/hir-def/src/data/adt.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ impl VariantData {
447447
}
448448
}
449449

450+
// FIXME: Linear lookup
450451
pub fn field(&self, name: &Name) -> Option<LocalFieldId> {
451452
self.fields().iter().find_map(|(id, data)| if &data.name == name { Some(id) } else { None })
452453
}

‎crates/hir-ty/src/infer.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ pub(crate) type InferResult<T> = Result<InferOk<T>, TypeError>;
194194
#[derive(Debug, PartialEq, Eq, Clone)]
195195
pub enum InferenceDiagnostic {
196196
NoSuchField {
197-
expr: ExprId,
197+
field: ExprOrPatId,
198+
private: bool,
198199
},
199200
PrivateField {
200201
expr: ExprId,

‎crates/hir-ty/src/infer/expr.rs

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -514,9 +514,6 @@ impl InferenceContext<'_> {
514514
}
515515
Expr::RecordLit { path, fields, spread, .. } => {
516516
let (ty, def_id) = self.resolve_variant(path.as_deref(), false);
517-
if let Some(variant) = def_id {
518-
self.write_variant_resolution(tgt_expr.into(), variant);
519-
}
520517

521518
if let Some(t) = expected.only_has_type(&mut self.table) {
522519
self.unify(&ty, &t);
@@ -526,26 +523,56 @@ impl InferenceContext<'_> {
526523
.as_adt()
527524
.map(|(_, s)| s.clone())
528525
.unwrap_or_else(|| Substitution::empty(Interner));
529-
let field_types = def_id.map(|it| self.db.field_types(it)).unwrap_or_default();
530-
let variant_data = def_id.map(|it| it.variant_data(self.db.upcast()));
531-
for field in fields.iter() {
532-
let field_def =
533-
variant_data.as_ref().and_then(|it| match it.field(&field.name) {
534-
Some(local_id) => Some(FieldId { parent: def_id.unwrap(), local_id }),
535-
None => {
536-
self.push_diagnostic(InferenceDiagnostic::NoSuchField {
537-
expr: field.expr,
538-
});
539-
None
540-
}
541-
});
542-
let field_ty = field_def.map_or(self.err_ty(), |it| {
543-
field_types[it.local_id].clone().substitute(Interner, &substs)
544-
});
545-
// Field type might have some unknown types
546-
// FIXME: we may want to emit a single type variable for all instance of type fields?
547-
let field_ty = self.insert_type_vars(field_ty);
548-
self.infer_expr_coerce(field.expr, &Expectation::has_type(field_ty));
526+
if let Some(variant) = def_id {
527+
self.write_variant_resolution(tgt_expr.into(), variant);
528+
}
529+
match def_id {
530+
_ if fields.is_empty() => {}
531+
Some(def) => {
532+
let field_types = self.db.field_types(def);
533+
let variant_data = def.variant_data(self.db.upcast());
534+
let visibilities = self.db.field_visibilities(def);
535+
for field in fields.iter() {
536+
let field_def = {
537+
match variant_data.field(&field.name) {
538+
Some(local_id) => {
539+
if !visibilities[local_id].is_visible_from(
540+
self.db.upcast(),
541+
self.resolver.module(),
542+
) {
543+
self.push_diagnostic(
544+
InferenceDiagnostic::NoSuchField {
545+
field: field.expr.into(),
546+
private: true,
547+
},
548+
);
549+
}
550+
Some(local_id)
551+
}
552+
None => {
553+
self.push_diagnostic(InferenceDiagnostic::NoSuchField {
554+
field: field.expr.into(),
555+
private: false,
556+
});
557+
None
558+
}
559+
}
560+
};
561+
let field_ty = field_def.map_or(self.err_ty(), |it| {
562+
field_types[it].clone().substitute(Interner, &substs)
563+
});
564+
565+
// Field type might have some unknown types
566+
// FIXME: we may want to emit a single type variable for all instance of type fields?
567+
let field_ty = self.insert_type_vars(field_ty);
568+
self.infer_expr_coerce(field.expr, &Expectation::has_type(field_ty));
569+
}
570+
}
571+
None => {
572+
for field in fields.iter() {
573+
self.infer_expr_coerce(field.expr, &Expectation::None);
574+
}
575+
}
549576
}
550577
if let Some(expr) = spread {
551578
self.infer_expr(*expr, &Expectation::has_type(ty.clone()));
@@ -1127,7 +1154,7 @@ impl InferenceContext<'_> {
11271154
Expr::Underscore => rhs_ty.clone(),
11281155
_ => {
11291156
// `lhs` is a place expression, a unit struct, or an enum variant.
1130-
let lhs_ty = self.infer_expr(lhs, &Expectation::none());
1157+
let lhs_ty = self.infer_expr_inner(lhs, &Expectation::none());
11311158

11321159
// This is the only branch where this function may coerce any type.
11331160
// We are returning early to avoid the unifiability check below.

‎crates/hir-ty/src/infer/pat.rs

Lines changed: 89 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -92,24 +92,51 @@ impl InferenceContext<'_> {
9292
let substs =
9393
ty.as_adt().map(|(_, s)| s.clone()).unwrap_or_else(|| Substitution::empty(Interner));
9494

95-
let field_tys = def.map(|it| self.db.field_types(it)).unwrap_or_default();
96-
let (pre, post) = match ellipsis {
97-
Some(idx) => subs.split_at(idx),
98-
None => (subs, &[][..]),
99-
};
100-
let post_idx_offset = field_tys.iter().count().saturating_sub(post.len());
101-
102-
let pre_iter = pre.iter().enumerate();
103-
let post_iter = (post_idx_offset..).zip(post.iter());
104-
for (i, &subpat) in pre_iter.chain(post_iter) {
105-
let expected_ty = var_data
106-
.as_ref()
107-
.and_then(|d| d.field(&Name::new_tuple_field(i)))
108-
.map_or(self.err_ty(), |field| {
109-
field_tys[field].clone().substitute(Interner, &substs)
110-
});
111-
let expected_ty = self.normalize_associated_types_in(expected_ty);
112-
T::infer(self, subpat, &expected_ty, default_bm);
95+
match def {
96+
_ if subs.len() == 0 => {}
97+
Some(def) => {
98+
let field_types = self.db.field_types(def);
99+
let variant_data = def.variant_data(self.db.upcast());
100+
let visibilities = self.db.field_visibilities(def);
101+
102+
let (pre, post) = match ellipsis {
103+
Some(idx) => subs.split_at(idx),
104+
None => (subs, &[][..]),
105+
};
106+
let post_idx_offset = field_types.iter().count().saturating_sub(post.len());
107+
108+
let pre_iter = pre.iter().enumerate();
109+
let post_iter = (post_idx_offset..).zip(post.iter());
110+
111+
for (i, &subpat) in pre_iter.chain(post_iter) {
112+
let field_def = {
113+
match variant_data.field(&Name::new_tuple_field(i)) {
114+
Some(local_id) => {
115+
if !visibilities[local_id]
116+
.is_visible_from(self.db.upcast(), self.resolver.module())
117+
{
118+
// FIXME(DIAGNOSE): private tuple field
119+
}
120+
Some(local_id)
121+
}
122+
None => None,
123+
}
124+
};
125+
126+
let expected_ty = field_def.map_or(self.err_ty(), |f| {
127+
field_types[f].clone().substitute(Interner, &substs)
128+
});
129+
let expected_ty = self.normalize_associated_types_in(expected_ty);
130+
131+
T::infer(self, subpat, &expected_ty, default_bm);
132+
}
133+
}
134+
None => {
135+
let err_ty = self.err_ty();
136+
for &inner in subs {
137+
T::infer(self, inner, &err_ty, default_bm);
138+
}
139+
}
113140
}
114141

115142
ty
@@ -122,7 +149,7 @@ impl InferenceContext<'_> {
122149
expected: &Ty,
123150
default_bm: T::BindingMode,
124151
id: T,
125-
subs: impl Iterator<Item = (Name, T)>,
152+
subs: impl Iterator<Item = (Name, T)> + ExactSizeIterator,
126153
) -> Ty {
127154
let (ty, def) = self.resolve_variant(path, false);
128155
if let Some(variant) = def {
@@ -134,17 +161,51 @@ impl InferenceContext<'_> {
134161
let substs =
135162
ty.as_adt().map(|(_, s)| s.clone()).unwrap_or_else(|| Substitution::empty(Interner));
136163

137-
let field_tys = def.map(|it| self.db.field_types(it)).unwrap_or_default();
138-
let var_data = def.map(|it| it.variant_data(self.db.upcast()));
164+
match def {
165+
_ if subs.len() == 0 => {}
166+
Some(def) => {
167+
let field_types = self.db.field_types(def);
168+
let variant_data = def.variant_data(self.db.upcast());
169+
let visibilities = self.db.field_visibilities(def);
170+
171+
for (name, inner) in subs {
172+
let field_def = {
173+
match variant_data.field(&name) {
174+
Some(local_id) => {
175+
if !visibilities[local_id]
176+
.is_visible_from(self.db.upcast(), self.resolver.module())
177+
{
178+
self.push_diagnostic(InferenceDiagnostic::NoSuchField {
179+
field: inner.into(),
180+
private: true,
181+
});
182+
}
183+
Some(local_id)
184+
}
185+
None => {
186+
self.push_diagnostic(InferenceDiagnostic::NoSuchField {
187+
field: inner.into(),
188+
private: false,
189+
});
190+
None
191+
}
192+
}
193+
};
139194

140-
for (name, inner) in subs {
141-
let expected_ty = var_data
142-
.as_ref()
143-
.and_then(|it| it.field(&name))
144-
.map_or(self.err_ty(), |f| field_tys[f].clone().substitute(Interner, &substs));
145-
let expected_ty = self.normalize_associated_types_in(expected_ty);
195+
let expected_ty = field_def.map_or(self.err_ty(), |f| {
196+
field_types[f].clone().substitute(Interner, &substs)
197+
});
198+
let expected_ty = self.normalize_associated_types_in(expected_ty);
146199

147-
T::infer(self, inner, &expected_ty, default_bm);
200+
T::infer(self, inner, &expected_ty, default_bm);
201+
}
202+
}
203+
None => {
204+
let err_ty = self.err_ty();
205+
for (_, inner) in subs {
206+
T::infer(self, inner, &err_ty, default_bm);
207+
}
208+
}
148209
}
149210

150211
ty

‎crates/hir-ty/src/mir.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ impl<V, T> ProjectionElem<V, T> {
166166
TyKind::Adt(_, subst) => {
167167
db.field_types(f.parent)[f.local_id].clone().substitute(Interner, subst)
168168
}
169-
_ => {
170-
never!("Only adt has field");
169+
ty => {
170+
never!("Only adt has field, found {:?}", ty);
171171
return TyKind::Error.intern(Interner);
172172
}
173173
},

‎crates/hir/src/diagnostics.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,8 @@ pub struct MalformedDerive {
173173

174174
#[derive(Debug)]
175175
pub struct NoSuchField {
176-
pub field: InFile<AstPtr<ast::RecordExprField>>,
176+
pub field: InFile<Either<AstPtr<ast::RecordExprField>, AstPtr<ast::RecordPatField>>>,
177+
pub private: bool,
177178
}
178179

179180
#[derive(Debug)]

‎crates/hir/src/lib.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,11 +1503,19 @@ impl DefWithBody {
15031503
let infer = db.infer(self.into());
15041504
let source_map = Lazy::new(|| db.body_with_source_map(self.into()).1);
15051505
let expr_syntax = |expr| source_map.expr_syntax(expr).expect("unexpected synthetic");
1506+
let pat_syntax = |pat| source_map.pat_syntax(pat).expect("unexpected synthetic");
15061507
for d in &infer.diagnostics {
15071508
match d {
1508-
&hir_ty::InferenceDiagnostic::NoSuchField { expr } => {
1509-
let field = source_map.field_syntax(expr);
1510-
acc.push(NoSuchField { field }.into())
1509+
&hir_ty::InferenceDiagnostic::NoSuchField { field: expr, private } => {
1510+
let expr_or_pat = match expr {
1511+
ExprOrPatId::ExprId(expr) => {
1512+
source_map.field_syntax(expr).map(Either::Left)
1513+
}
1514+
ExprOrPatId::PatId(pat) => {
1515+
source_map.pat_field_syntax(pat).map(Either::Right)
1516+
}
1517+
};
1518+
acc.push(NoSuchField { field: expr_or_pat, private }.into())
15111519
}
15121520
&hir_ty::InferenceDiagnostic::MismatchedArgCount { call_expr, expected, found } => {
15131521
acc.push(
@@ -1523,10 +1531,7 @@ impl DefWithBody {
15231531
&hir_ty::InferenceDiagnostic::PrivateAssocItem { id, item } => {
15241532
let expr_or_pat = match id {
15251533
ExprOrPatId::ExprId(expr) => expr_syntax(expr).map(Either::Left),
1526-
ExprOrPatId::PatId(pat) => source_map
1527-
.pat_syntax(pat)
1528-
.expect("unexpected synthetic")
1529-
.map(Either::Right),
1534+
ExprOrPatId::PatId(pat) => pat_syntax(pat).map(Either::Right),
15301535
};
15311536
let item = item.into();
15321537
acc.push(PrivateAssocItem { expr_or_pat, item }.into())

‎crates/ide-diagnostics/src/handlers/missing_match_arms.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,7 @@ fn main() {
849849
struct Foo { }
850850
fn main(f: Foo) {
851851
match f { Foo { bar } => () }
852+
// ^^^ error: no such field
852853
}
853854
"#,
854855
);

0 commit comments

Comments
 (0)