From 6007e6f649feac9a0cb39270444033f8dbfa9259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 17 Mar 2019 20:09:53 -0700 Subject: [PATCH 1/2] Do not complain about non-existing fields after parse recovery When failing to parse struct-like enum variants, the ADT gets recorded as having no fields. Record that we have actually recovered during parsing of this variant to avoid complaing about non-existing fields when actually using it. --- src/librustc/hir/lowering.rs | 3 +- src/librustc/hir/mod.rs | 4 +-- src/librustc/ty/mod.rs | 28 +++++++++++-------- src/librustc_metadata/decoder.rs | 3 +- src/librustc_save_analysis/dump_visitor.rs | 6 ++-- src/librustc_save_analysis/sig.rs | 18 +++++++----- src/librustc_typeck/check/_match.rs | 20 +++++++------ src/librustc_typeck/check/mod.rs | 17 +++++++---- src/librustc_typeck/collect.rs | 7 ++++- src/libsyntax/ast.rs | 6 ++-- src/libsyntax/config.rs | 2 +- src/libsyntax/mut_visit.rs | 2 +- src/libsyntax/parse/parser.rs | 28 ++++++++++++------- .../ui/parser/recovered-struct-variant.rs | 13 +++++++++ .../ui/parser/recovered-struct-variant.stderr | 8 ++++++ 15 files changed, 108 insertions(+), 57 deletions(-) create mode 100644 src/test/ui/parser/recovered-struct-variant.rs create mode 100644 src/test/ui/parser/recovered-struct-variant.stderr diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 949fdd2682b96..73c3b3026d98b 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -2672,7 +2672,7 @@ impl<'a> LoweringContext<'a> { fn lower_variant_data(&mut self, vdata: &VariantData) -> hir::VariantData { match *vdata { - VariantData::Struct(ref fields, id) => { + VariantData::Struct(ref fields, id, recovered) => { let LoweredNodeId { node_id: _, hir_id } = self.lower_node_id(id); hir::VariantData::Struct( @@ -2682,6 +2682,7 @@ impl<'a> LoweringContext<'a> { .map(|f| self.lower_struct_field(f)) .collect(), hir_id, + recovered, ) }, VariantData::Tuple(ref fields, id) => { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 88ab58d10fc34..785d7745b297b 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2173,7 +2173,7 @@ impl StructField { /// Id of the whole struct lives in `Item`. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum VariantData { - Struct(HirVec, HirId), + Struct(HirVec, HirId, bool), Tuple(HirVec, HirId), Unit(HirId), } @@ -2187,7 +2187,7 @@ impl VariantData { } pub fn hir_id(&self) -> HirId { match *self { - VariantData::Struct(_, hir_id) + VariantData::Struct(_, hir_id, _) | VariantData::Tuple(_, hir_id) | VariantData::Unit(hir_id) => hir_id, } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 882e2dc62b1c3..47dbed914ddc3 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1811,6 +1811,7 @@ pub struct VariantDef { pub fields: Vec, pub ctor_kind: CtorKind, flags: VariantFlags, + pub recovered: bool, } impl<'a, 'gcx, 'tcx> VariantDef { @@ -1829,16 +1830,17 @@ impl<'a, 'gcx, 'tcx> VariantDef { /// /// If someone speeds up attribute loading to not be a performance concern, they can /// remove this hack and use the constructor `DefId` everywhere. - pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>, - did: DefId, - ident: Ident, - discr: VariantDiscr, - fields: Vec, - adt_kind: AdtKind, - ctor_kind: CtorKind, - attribute_def_id: DefId) - -> Self - { + pub fn new( + tcx: TyCtxt<'a, 'gcx, 'tcx>, + did: DefId, + ident: Ident, + discr: VariantDiscr, + fields: Vec, + adt_kind: AdtKind, + ctor_kind: CtorKind, + attribute_def_id: DefId, + recovered: bool, + ) -> Self { debug!("VariantDef::new({:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?})", did, ident, discr, fields, adt_kind, ctor_kind, attribute_def_id); let mut flags = VariantFlags::NO_VARIANT_FLAGS; @@ -1852,7 +1854,8 @@ impl<'a, 'gcx, 'tcx> VariantDef { discr, fields, ctor_kind, - flags + flags, + recovered, } } @@ -1868,7 +1871,8 @@ impl_stable_hash_for!(struct VariantDef { discr, fields, ctor_kind, - flags + flags, + recovered }); #[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable, HashStable)] diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 6fe00a4ad2ff2..c608c03095aa3 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -576,7 +576,8 @@ impl<'a, 'tcx> CrateMetadata { }).collect(), adt_kind, data.ctor_kind, - attribute_def_id + attribute_def_id, + false, ) } diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 3fea515ae401e..01bb643c1d587 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -481,8 +481,8 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { }; let (value, fields) = match item.node { - ast::ItemKind::Struct(ast::VariantData::Struct(ref fields, _), _) | - ast::ItemKind::Union(ast::VariantData::Struct(ref fields, _), _) => { + ast::ItemKind::Struct(ast::VariantData::Struct(ref fields, ..), _) | + ast::ItemKind::Union(ast::VariantData::Struct(ref fields, ..), _) => { let include_priv_fields = !self.save_ctxt.config.pub_only; let fields_str = fields .iter() @@ -560,7 +560,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> { let name_span = variant.node.ident.span; match variant.node.data { - ast::VariantData::Struct(ref fields, _) => { + ast::VariantData::Struct(ref fields, ..) => { let fields_str = fields .iter() .enumerate() diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index 64a2c92d04dbd..6e47ae6b15984 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/src/librustc_save_analysis/sig.rs @@ -703,7 +703,7 @@ impl Sig for ast::Variant_ { fn make(&self, offset: usize, _parent_id: Option, scx: &SaveContext<'_, '_>) -> Result { let mut text = self.ident.to_string(); match self.data { - ast::VariantData::Struct(ref fields, id) => { + ast::VariantData::Struct(ref fields, id, r) => { let name_def = SigElement { id: id_from_node_id(id, scx), start: offset, @@ -712,12 +712,16 @@ impl Sig for ast::Variant_ { text.push_str(" { "); let mut defs = vec![name_def]; let mut refs = vec![]; - for f in fields { - let field_sig = f.make(offset + text.len(), Some(id), scx)?; - text.push_str(&field_sig.text); - text.push_str(", "); - defs.extend(field_sig.defs.into_iter()); - refs.extend(field_sig.refs.into_iter()); + if r { + text.push_str("/* parse error */ "); + } else { + for f in fields { + let field_sig = f.make(offset + text.len(), Some(id), scx)?; + text.push_str(&field_sig.text); + text.push_str(", "); + defs.extend(field_sig.defs.into_iter()); + refs.extend(field_sig.refs.into_iter()); + } } text.push('}'); Ok(Signature { text, defs, refs }) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index c6b66393dd2f1..c30b9d65fec83 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -918,14 +918,16 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); pat_ty } - fn check_struct_pat_fields(&self, - adt_ty: Ty<'tcx>, - pat_id: hir::HirId, - span: Span, - variant: &'tcx ty::VariantDef, - fields: &'gcx [Spanned], - etc: bool, - def_bm: ty::BindingMode) -> bool { + fn check_struct_pat_fields( + &self, + adt_ty: Ty<'tcx>, + pat_id: hir::HirId, + span: Span, + variant: &'tcx ty::VariantDef, + fields: &'gcx [Spanned], + etc: bool, + def_bm: ty::BindingMode, + ) -> bool { let tcx = self.tcx; let (substs, adt) = match adt_ty.sty { @@ -985,7 +987,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); .map(|field| field.ident.modern()) .filter(|ident| !used_fields.contains_key(&ident)) .collect::>(); - if inexistent_fields.len() > 0 { + if inexistent_fields.len() > 0 && !variant.recovered { let (field_names, t, plural) = if inexistent_fields.len() == 1 { (format!("a field named `{}`", inexistent_fields[0].1), "this", "") } else { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fa4bb02189f20..e38a575c560e3 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3689,12 +3689,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { field, expr_t) } - fn report_unknown_field(&self, - ty: Ty<'tcx>, - variant: &'tcx ty::VariantDef, - field: &hir::Field, - skip_fields: &[hir::Field], - kind_name: &str) { + fn report_unknown_field( + &self, + ty: Ty<'tcx>, + variant: &'tcx ty::VariantDef, + field: &hir::Field, + skip_fields: &[hir::Field], + kind_name: &str, + ) { + if variant.recovered { + return; + } let mut err = self.type_error_struct_with_diag( field.ident.span, |actual| match ty.sty { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 10e9613bf21a2..c0739db3df6a2 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -598,6 +598,10 @@ fn convert_variant<'a, 'tcx>( } }) .collect(); + let recovered = match def { + hir::VariantData::Struct(_, _, r) => *r, + _ => false, + }; ty::VariantDef::new(tcx, did, ident, @@ -605,7 +609,8 @@ fn convert_variant<'a, 'tcx>( fields, adt_kind, CtorKind::from_hir(def), - attribute_def_id + attribute_def_id, + recovered, ) } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 1a0da73880cfc..5da00ef8ebee2 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -2133,7 +2133,7 @@ pub enum VariantData { /// Struct variant. /// /// E.g., `Bar { .. }` as in `enum Foo { Bar { .. } }`. - Struct(Vec, NodeId), + Struct(Vec, NodeId, bool), /// Tuple variant. /// /// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`. @@ -2147,13 +2147,13 @@ pub enum VariantData { impl VariantData { pub fn fields(&self) -> &[StructField] { match *self { - VariantData::Struct(ref fields, _) | VariantData::Tuple(ref fields, _) => fields, + VariantData::Struct(ref fields, ..) | VariantData::Tuple(ref fields, _) => fields, _ => &[], } } pub fn id(&self) -> NodeId { match *self { - VariantData::Struct(_, id) | VariantData::Tuple(_, id) | VariantData::Unit(id) => id, + VariantData::Struct(_, id, _) | VariantData::Tuple(_, id) | VariantData::Unit(id) => id, } } pub fn is_struct(&self) -> bool { diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs index c300ffc2d61b9..7159c949513ac 100644 --- a/src/libsyntax/config.rs +++ b/src/libsyntax/config.rs @@ -225,7 +225,7 @@ impl<'a> StripUnconfigured<'a> { fn configure_variant_data(&mut self, vdata: &mut ast::VariantData) { match vdata { - ast::VariantData::Struct(fields, _id) | + ast::VariantData::Struct(fields, _id, _) | ast::VariantData::Tuple(fields, _id) => fields.flat_map_in_place(|field| self.configure(field)), ast::VariantData::Unit(_id) => {} diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index 462346df7d76d..5bb1d8a4b9476 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -765,7 +765,7 @@ pub fn noop_visit_where_predicate(pred: &mut WherePredicate, vis: pub fn noop_visit_variant_data(vdata: &mut VariantData, vis: &mut T) { match vdata { - VariantData::Struct(fields, id) | + VariantData::Struct(fields, id, _) | VariantData::Tuple(fields, id) => { visit_vec(fields, |field| vis.visit_struct_field(field)); vis.visit_id(id); diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index aa70c54a1ef8a..360a101a64faf 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -6829,14 +6829,16 @@ impl<'a> Parser<'a> { VariantData::Unit(ast::DUMMY_NODE_ID) } else { // If we see: `struct Foo where T: Copy { ... }` - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) } // No `where` so: `struct Foo;` } else if self.eat(&token::Semi) { VariantData::Unit(ast::DUMMY_NODE_ID) // Record-style struct definition } else if self.token == token::OpenDelim(token::Brace) { - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) // Tuple-style struct definition with optional where-clause. } else if self.token == token::OpenDelim(token::Paren) { let body = VariantData::Tuple(self.parse_tuple_struct_body()?, ast::DUMMY_NODE_ID); @@ -6864,9 +6866,11 @@ impl<'a> Parser<'a> { let vdata = if self.token.is_keyword(keywords::Where) { generics.where_clause = self.parse_where_clause()?; - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) } else if self.token == token::OpenDelim(token::Brace) { - VariantData::Struct(self.parse_record_struct_body()?, ast::DUMMY_NODE_ID) + let (fields, recovered) = self.parse_record_struct_body()?; + VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered) } else { let token_str = self.this_token_descr(); let mut err = self.fatal(&format!( @@ -6898,12 +6902,14 @@ impl<'a> Parser<'a> { } } - fn parse_record_struct_body(&mut self) -> PResult<'a, Vec> { + fn parse_record_struct_body(&mut self) -> PResult<'a, (Vec, bool)> { let mut fields = Vec::new(); + let mut recovered = false; if self.eat(&token::OpenDelim(token::Brace)) { while self.token != token::CloseDelim(token::Brace) { let field = self.parse_struct_decl_field().map_err(|e| { self.recover_stmt(); + recovered = true; e }); match field { @@ -6922,7 +6928,7 @@ impl<'a> Parser<'a> { return Err(err); } - Ok(fields) + Ok((fields, recovered)) } fn parse_tuple_struct_body(&mut self) -> PResult<'a, Vec> { @@ -7684,12 +7690,14 @@ impl<'a> Parser<'a> { if self.check(&token::OpenDelim(token::Brace)) { // Parse a struct variant. all_nullary = false; - struct_def = VariantData::Struct(self.parse_record_struct_body()?, - ast::DUMMY_NODE_ID); + let (fields, recovered) = self.parse_record_struct_body()?; + struct_def = VariantData::Struct(fields, ast::DUMMY_NODE_ID, recovered); } else if self.check(&token::OpenDelim(token::Paren)) { all_nullary = false; - struct_def = VariantData::Tuple(self.parse_tuple_struct_body()?, - ast::DUMMY_NODE_ID); + struct_def = VariantData::Tuple( + self.parse_tuple_struct_body()?, + ast::DUMMY_NODE_ID, + ); } else if self.eat(&token::Eq) { disr_expr = Some(AnonConst { id: ast::DUMMY_NODE_ID, diff --git a/src/test/ui/parser/recovered-struct-variant.rs b/src/test/ui/parser/recovered-struct-variant.rs new file mode 100644 index 0000000000000..5b195dcc37878 --- /dev/null +++ b/src/test/ui/parser/recovered-struct-variant.rs @@ -0,0 +1,13 @@ +enum Foo { + A { a, b: usize } + //~^ ERROR expected `:`, found `,` +} + +fn main() { + // no complaints about non-existing fields + let f = Foo::A { a:3, b: 4}; + match f { + // no complaints about non-existing fields + Foo::A {a, b} => {} + } +} diff --git a/src/test/ui/parser/recovered-struct-variant.stderr b/src/test/ui/parser/recovered-struct-variant.stderr new file mode 100644 index 0000000000000..51aaf8bb3cfbe --- /dev/null +++ b/src/test/ui/parser/recovered-struct-variant.stderr @@ -0,0 +1,8 @@ +error: expected `:`, found `,` + --> $DIR/recovered-struct-variant.rs:2:10 + | +LL | A { a, b: usize } + | ^ expected `:` + +error: aborting due to previous error + From 757eb679927e2c85457f567487e887eb080eb7cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 19 Mar 2019 13:17:25 -0700 Subject: [PATCH 2/2] review comments --- src/librustc/hir/mod.rs | 2 +- src/libsyntax/ast.rs | 2 +- src/libsyntax/parse/parser.rs | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 785d7745b297b..51d91cc562f68 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2173,7 +2173,7 @@ impl StructField { /// Id of the whole struct lives in `Item`. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum VariantData { - Struct(HirVec, HirId, bool), + Struct(HirVec, HirId, /* recovered */ bool), Tuple(HirVec, HirId), Unit(HirId), } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 5da00ef8ebee2..2cbd2dfeb25d6 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -620,7 +620,7 @@ pub enum PatKind { /// A struct or struct variant pattern (e.g., `Variant {x, y, ..}`). /// The `bool` is `true` in the presence of a `..`. - Struct(Path, Vec>, bool), + Struct(Path, Vec>, /* recovered */ bool), /// A tuple struct/variant pattern (`Variant(x, y, .., z)`). /// If the `..` pattern fragment is present, then `Option` denotes its position. diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 360a101a64faf..361ecce5bc438 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -6902,7 +6902,9 @@ impl<'a> Parser<'a> { } } - fn parse_record_struct_body(&mut self) -> PResult<'a, (Vec, bool)> { + fn parse_record_struct_body( + &mut self, + ) -> PResult<'a, (Vec, /* recovered */ bool)> { let mut fields = Vec::new(); let mut recovered = false; if self.eat(&token::OpenDelim(token::Brace)) {