From 26b24cd7558c02c433ef9f621894cb6a2f31898d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 4 Dec 2022 17:59:21 +0000 Subject: [PATCH 1/3] drive-by: move field_index to typeck results --- compiler/rustc_hir_typeck/src/expr_use_visitor.rs | 6 +++--- compiler/rustc_lint/src/builtin.rs | 2 +- compiler/rustc_middle/src/ty/context.rs | 8 ++++++++ compiler/rustc_middle/src/ty/mod.rs | 4 ---- compiler/rustc_mir_build/src/thir/cx/expr.rs | 4 ++-- compiler/rustc_mir_build/src/thir/pattern/mod.rs | 2 +- compiler/rustc_passes/src/dead.rs | 6 +++--- compiler/rustc_privacy/src/lib.rs | 10 +++++----- 8 files changed, 23 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index e5e798f4b933f..4f06ad8a01821 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -540,9 +540,9 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { ty::Adt(adt, substs) if adt.is_struct() => { // Consume those fields of the with expression that are needed. for (f_index, with_field) in adt.non_enum_variant().fields.iter().enumerate() { - let is_mentioned = fields.iter().any(|f| { - self.tcx().field_index(f.hir_id, self.mc.typeck_results) == f_index - }); + let is_mentioned = fields + .iter() + .any(|f| self.mc.typeck_results.field_index(f.hir_id) == f_index); if !is_mentioned { let field_place = self.mc.cat_projection( &*with_expr, diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 825093384fba7..d0d34d8d2b40e 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -259,7 +259,7 @@ impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns { } if let PatKind::Binding(binding_annot, _, ident, None) = fieldpat.pat.kind { if cx.tcx.find_field_index(ident, &variant) - == Some(cx.tcx.field_index(fieldpat.hir_id, cx.typeck_results())) + == Some(cx.typeck_results().field_index(fieldpat.hir_id)) { cx.struct_span_lint( NON_SHORTHAND_FIELD_PATTERNS, diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index c5683a9db9473..11811de8e7afc 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -671,6 +671,14 @@ impl<'tcx> TypeckResults<'tcx> { LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.field_indices } } + pub fn field_index(&self, id: hir::HirId) -> usize { + self.field_indices().get(id).cloned().expect("no index for a field") + } + + pub fn opt_field_index(&self, id: hir::HirId) -> Option { + self.field_indices().get(id).cloned() + } + pub fn user_provided_types(&self) -> LocalTableInContext<'_, CanonicalUserType<'tcx>> { LocalTableInContext { hir_owner: self.hir_owner, data: &self.user_provided_types } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index dd4ab3e8d30bf..d869f17e3ac50 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -2228,10 +2228,6 @@ impl<'tcx> TyCtxt<'tcx> { } } - pub fn field_index(self, hir_id: hir::HirId, typeck_results: &TypeckResults<'_>) -> usize { - typeck_results.field_indices().get(hir_id).cloned().expect("no index for a field") - } - pub fn find_field_index(self, ident: Ident, variant: &VariantDef) -> Option { variant .fields diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 5fa41ebeb6e59..261b95ba95b0e 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -704,7 +704,7 @@ impl<'tcx> Cx<'tcx> { hir::ExprKind::Field(ref source, ..) => ExprKind::Field { lhs: self.mirror_expr(source), variant_index: VariantIdx::new(0), - name: Field::new(tcx.field_index(expr.hir_id, self.typeck_results)), + name: Field::new(self.typeck_results.field_index(expr.hir_id)), }, hir::ExprKind::Cast(ref source, ref cast_ty) => { // Check for a user-given type annotation on this `cast` @@ -1079,7 +1079,7 @@ impl<'tcx> Cx<'tcx> { fields .iter() .map(|field| FieldExpr { - name: Field::new(self.tcx.field_index(field.hir_id, self.typeck_results)), + name: Field::new(self.typeck_results.field_index(field.hir_id)), expr: self.mirror_expr(field.expr), }) .collect() diff --git a/compiler/rustc_mir_build/src/thir/pattern/mod.rs b/compiler/rustc_mir_build/src/thir/pattern/mod.rs index 4c2a80e523f35..48a231a6cd6b7 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/mod.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/mod.rs @@ -321,7 +321,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { let subpatterns = fields .iter() .map(|field| FieldPat { - field: Field::new(self.tcx.field_index(field.hir_id, self.typeck_results)), + field: Field::new(self.typeck_results.field_index(field.hir_id)), pattern: self.lower_pattern(&field.pat), }) .collect(); diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index da023fcf4c3b5..a71ae717a508d 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -124,7 +124,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { fn handle_field_access(&mut self, lhs: &hir::Expr<'_>, hir_id: hir::HirId) { match self.typeck_results().expr_ty_adjusted(lhs).kind() { ty::Adt(def, _) => { - let index = self.tcx.field_index(hir_id, self.typeck_results()); + let index = self.typeck_results().field_index(hir_id); self.insert_def_id(def.non_enum_variant().fields[index].did); } ty::Tuple(..) => {} @@ -208,7 +208,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { if let PatKind::Wild = pat.pat.kind { continue; } - let index = self.tcx.field_index(pat.hir_id, self.typeck_results()); + let index = self.typeck_results().field_index(pat.hir_id); self.insert_def_id(variant.fields[index].did); } } @@ -341,7 +341,7 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { fn mark_as_used_if_union(&mut self, adt: ty::AdtDef<'tcx>, fields: &[hir::ExprField<'_>]) { if adt.is_union() && adt.non_enum_variant().fields.len() > 1 && adt.did().is_local() { for field in fields { - let index = self.tcx.field_index(field.hir_id, self.typeck_results()); + let index = self.typeck_results().field_index(field.hir_id); self.insert_def_id(adt.non_enum_variant().fields[index].did); } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index f2177a7c28332..a254c892478cf 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -1065,9 +1065,9 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { // are checked for privacy (RFC 736). Rather than computing the set of // unmentioned fields, just check them all. for (vf_index, variant_field) in variant.fields.iter().enumerate() { - let field = fields.iter().find(|f| { - self.tcx.field_index(f.hir_id, self.typeck_results()) == vf_index - }); + let field = fields + .iter() + .find(|f| self.typeck_results().field_index(f.hir_id) == vf_index); let (use_ctxt, span) = match field { Some(field) => (field.ident.span, field.span), None => (base.span, base.span), @@ -1077,7 +1077,7 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { } else { for field in fields { let use_ctxt = field.ident.span; - let index = self.tcx.field_index(field.hir_id, self.typeck_results()); + let index = self.typeck_results().field_index(field.hir_id); self.check_field(use_ctxt, field.span, adt, &variant.fields[index], false); } } @@ -1093,7 +1093,7 @@ impl<'tcx> Visitor<'tcx> for NamePrivacyVisitor<'tcx> { let variant = adt.variant_of_res(res); for field in fields { let use_ctxt = field.ident.span; - let index = self.tcx.field_index(field.hir_id, self.typeck_results()); + let index = self.typeck_results().field_index(field.hir_id); self.check_field(use_ctxt, field.span, adt, &variant.fields[index], false); } } From d442c015d6d688460a182947e797f4bf69876044 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 4 Dec 2022 18:53:50 +0000 Subject: [PATCH 2/3] Don't ICE in ExprUseVisitor on FRU for non-existent struct --- compiler/rustc_hir_typeck/src/expr_use_visitor.rs | 7 ++++++- src/test/ui/structs/unresolved-struct-with-fru.rs | 12 ++++++++++++ .../ui/structs/unresolved-struct-with-fru.stderr | 9 +++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/structs/unresolved-struct-with-fru.rs create mode 100644 src/test/ui/structs/unresolved-struct-with-fru.stderr diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index 4f06ad8a01821..03b174c77955f 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -523,6 +523,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { // Consume the expressions supplying values for each field. for field in fields { self.consume_expr(field.expr); + + // The struct path probably didn't resolve + if self.mc.typeck_results.opt_field_index(field.hir_id).is_none() { + self.tcx().sess.delay_span_bug(field.span, "couldn't resolve index for field"); + } } let with_expr = match *opt_with { @@ -542,7 +547,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { for (f_index, with_field) in adt.non_enum_variant().fields.iter().enumerate() { let is_mentioned = fields .iter() - .any(|f| self.mc.typeck_results.field_index(f.hir_id) == f_index); + .any(|f| self.mc.typeck_results.opt_field_index(f.hir_id) == Some(f_index)); if !is_mentioned { let field_place = self.mc.cat_projection( &*with_expr, diff --git a/src/test/ui/structs/unresolved-struct-with-fru.rs b/src/test/ui/structs/unresolved-struct-with-fru.rs new file mode 100644 index 0000000000000..c9fdca4577279 --- /dev/null +++ b/src/test/ui/structs/unresolved-struct-with-fru.rs @@ -0,0 +1,12 @@ +struct S { + a: u32, +} + +fn main() { + let s1 = S { a: 1 }; + + let _ = || { + let s2 = Oops { a: 2, ..s1 }; + //~^ ERROR cannot find struct, variant or union type `Oops` in this scope + }; +} diff --git a/src/test/ui/structs/unresolved-struct-with-fru.stderr b/src/test/ui/structs/unresolved-struct-with-fru.stderr new file mode 100644 index 0000000000000..a5796a222256d --- /dev/null +++ b/src/test/ui/structs/unresolved-struct-with-fru.stderr @@ -0,0 +1,9 @@ +error[E0422]: cannot find struct, variant or union type `Oops` in this scope + --> $DIR/unresolved-struct-with-fru.rs:9:18 + | +LL | let s2 = Oops { a: 2, ..s1 }; + | ^^^^ not found in this scope + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0422`. From 1c81540206b10a2b5b709540ba5ac138eaaa5261 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 4 Dec 2022 21:02:51 +0000 Subject: [PATCH 3/3] Unconditional check FRU expression, even if there are errors present --- compiler/rustc_hir_typeck/src/expr.rs | 3 +++ .../drop-track-bad-field-in-fru.rs | 10 ++++++++ .../drop-track-bad-field-in-fru.stderr | 23 +++++++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 src/test/ui/async-await/drop-track-bad-field-in-fru.rs create mode 100644 src/test/ui/async-await/drop-track-bad-field-in-fru.stderr diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 0c5bbb3e20be3..2ca5b980fc836 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1646,6 +1646,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // the fields with the base_expr. This could cause us to hit errors later // when certain fields are assumed to exist that in fact do not. if error_happened { + if let Some(base_expr) = base_expr { + self.check_expr(base_expr); + } return; } diff --git a/src/test/ui/async-await/drop-track-bad-field-in-fru.rs b/src/test/ui/async-await/drop-track-bad-field-in-fru.rs new file mode 100644 index 0000000000000..28ad7767583cf --- /dev/null +++ b/src/test/ui/async-await/drop-track-bad-field-in-fru.rs @@ -0,0 +1,10 @@ +// compile-flags: -Zdrop-tracking +// edition: 2021 + +fn main() {} + +async fn foo() { + None { value: (), ..Default::default() }.await; + //~^ ERROR `Option<_>` is not a future + //~| ERROR variant `Option<_>::None` has no field named `value` +} diff --git a/src/test/ui/async-await/drop-track-bad-field-in-fru.stderr b/src/test/ui/async-await/drop-track-bad-field-in-fru.stderr new file mode 100644 index 0000000000000..819b64ad77f5d --- /dev/null +++ b/src/test/ui/async-await/drop-track-bad-field-in-fru.stderr @@ -0,0 +1,23 @@ +error[E0559]: variant `Option<_>::None` has no field named `value` + --> $DIR/drop-track-bad-field-in-fru.rs:7:12 + | +LL | None { value: (), ..Default::default() }.await; + | ^^^^^ `Option<_>::None` does not have this field + +error[E0277]: `Option<_>` is not a future + --> $DIR/drop-track-bad-field-in-fru.rs:7:45 + | +LL | None { value: (), ..Default::default() }.await; + | ^^^^^^ + | | + | `Option<_>` is not a future + | help: remove the `.await` + | + = help: the trait `Future` is not implemented for `Option<_>` + = note: Option<_> must be a future or must implement `IntoFuture` to be awaited + = note: required for `Option<_>` to implement `IntoFuture` + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0277, E0559. +For more information about an error, try `rustc --explain E0277`.