From b87c2926271dcf3d0556b16ae3ea8fec75058d4a Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Fri, 14 Aug 2015 16:14:09 +0300 Subject: [PATCH 1/4] be more robust to bogus items in struct patterns/constructors Fixes #27815 --- src/librustc_typeck/check/_match.rs | 50 +++++------ src/librustc_typeck/check/mod.rs | 83 +++++++++---------- src/librustc_typeck/diagnostics.rs | 57 ++++--------- src/test/compile-fail-fulldeps/issue-18986.rs | 2 +- src/test/compile-fail/issue-27815.rs | 20 +++++ .../trait-as-struct-constructor.rs | 2 +- 6 files changed, 99 insertions(+), 115 deletions(-) create mode 100644 src/test/compile-fail/issue-27815.rs diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 883e3659720a6..9eb372a6c590b 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -526,41 +526,41 @@ pub fn check_pat_struct<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, pat: &'tcx ast::Pat, etc: bool, expected: Ty<'tcx>) { let fcx = pcx.fcx; let tcx = pcx.fcx.ccx.tcx; + let report_nonstruct = || { + let name = pprust::path_to_string(path); + span_err!(tcx.sess, pat.span, E0163, + "`{}` does not name a struct or a struct variant", name); + fcx.write_error(pat.id); + + for field in fields { + check_pat(pcx, &field.node.pat, tcx.types.err); + } + }; let def = tcx.def_map.borrow().get(&pat.id).unwrap().full_def(); let (adt_def, variant) = match def { - def::DefTrait(_) => { - let name = pprust::path_to_string(path); - span_err!(tcx.sess, pat.span, E0168, - "use of trait `{}` in a struct pattern", name); - fcx.write_error(pat.id); - - for field in fields { - check_pat(pcx, &*field.node.pat, tcx.types.err); - } - return; - }, - _ => { - let def_type = tcx.lookup_item_type(def.def_id()); - match def_type.ty.sty { + def::DefTy(did, _) | def::DefStruct(did) => { + match tcx.lookup_item_type(did).ty.sty { ty::TyStruct(struct_def, _) => (struct_def, struct_def.struct_variant()), - ty::TyEnum(enum_def, _) - if def == def::DefVariant(enum_def.did, def.def_id(), true) => - (enum_def, enum_def.variant_of_def(def)), _ => { - let name = pprust::path_to_string(path); - span_err!(tcx.sess, pat.span, E0163, - "`{}` does not name a struct or a struct variant", name); - fcx.write_error(pat.id); - - for field in fields { - check_pat(pcx, &*field.node.pat, tcx.types.err); - } + report_nonstruct(); return; } } } + def::DefVariant(eid, vid, true) => { + match tcx.lookup_item_type(vid).ty.sty { + ty::TyEnum(enum_def, _) if enum_def.did == eid => { + (enum_def, enum_def.variant_with_id(vid)) + } + _ => tcx.sess.span_bug(pat.span, "variant's type is not its enum") + } + } + _ => { + report_nonstruct(); + return; + } }; instantiate_path(pcx.fcx, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 6221134afd38a..92f39d972ee19 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1421,14 +1421,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// /// This is used when checking the constructor in struct literals. fn instantiate_struct_literal_ty(&self, - did: ast::DefId, + struct_ty: ty::TypeScheme<'tcx>, path: &ast::Path) -> TypeAndSubsts<'tcx> { - let tcx = self.tcx(); - - let ty::TypeScheme { generics, ty: decl_ty } = - tcx.lookup_item_type(did); + let ty::TypeScheme { generics, ty: decl_ty } = struct_ty; let substs = astconv::ast_path_substs_for_ty(self, self, path.span, @@ -3168,6 +3165,18 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, } } + fn report_exprstruct_on_nondict<'a, 'tcx>(fcx: &FnCtxt<'a,'tcx>, + id: ast::NodeId, + fields: &'tcx [ast::Field], + base_expr: &'tcx Option>, + path: &ast::Path) + { + span_err!(fcx.tcx().sess, path.span, E0071, + "`{}` does not name a structure", + pprust::path_to_string(path)); + check_struct_fields_on_error(fcx, id, fields, base_expr) + } + type ExprCheckerWithTy = fn(&FnCtxt, &ast::Expr, Ty); let tcx = fcx.ccx.tcx; @@ -3618,7 +3627,8 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, ast::ExprStruct(ref path, ref fields, ref base_expr) => { // Resolve the path. let def = lookup_full_def(tcx, path.span, id); - let struct_id = match def { + + let struct_ty = match def { def::DefVariant(enum_id, variant_id, true) => { if let &Some(ref base_expr) = base_expr { span_err!(tcx.sess, base_expr.span, E0436, @@ -3627,54 +3637,38 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, } check_struct_enum_variant(fcx, id, expr.span, enum_id, variant_id, &fields[..]); - enum_id + Some(tcx.lookup_item_type(enum_id)) } - def::DefTrait(def_id) => { - span_err!(tcx.sess, path.span, E0159, - "use of trait `{}` as a struct constructor", - pprust::path_to_string(path)); - check_struct_fields_on_error(fcx, - id, - &fields[..], - base_expr); - def_id - }, - def => { + def::DefTy(did, _) | def::DefStruct(did) => { // Verify that this was actually a struct. - let typ = fcx.ccx.tcx.lookup_item_type(def.def_id()); - match typ.ty.sty { - ty::TyStruct(struct_def, _) => { - check_struct_constructor(fcx, - id, - expr.span, - struct_def, - &fields[..], - base_expr.as_ref().map(|e| &**e)); - } - _ => { - span_err!(tcx.sess, path.span, E0071, - "`{}` does not name a structure", - pprust::path_to_string(path)); - check_struct_fields_on_error(fcx, - id, - &fields[..], - base_expr); - } + let typ = tcx.lookup_item_type(did); + if let ty::TyStruct(struct_def, _) = typ.ty.sty { + check_struct_constructor(fcx, + id, + expr.span, + struct_def, + &fields, + base_expr.as_ref().map(|e| &**e)); + } else { + report_exprstruct_on_nondict(fcx, id, &fields, base_expr, path); } - - def.def_id() + Some(typ) + } + _ => { + report_exprstruct_on_nondict(fcx, id, &fields, base_expr, path); + None } }; // Turn the path into a type and verify that that type unifies with // the resulting structure type. This is needed to handle type // parameters correctly. - let actual_structure_type = fcx.expr_ty(&*expr); - if !actual_structure_type.references_error() { - let type_and_substs = fcx.instantiate_struct_literal_ty(struct_id, path); + if let Some(struct_ty) = struct_ty { + let expr_ty = fcx.expr_ty(&expr); + let type_and_substs = fcx.instantiate_struct_literal_ty(struct_ty, path); match fcx.mk_subty(false, infer::Misc(path.span), - actual_structure_type, + expr_ty, type_and_substs.ty) { Ok(()) => {} Err(type_error) => { @@ -3685,8 +3679,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, fcx.infcx() .ty_to_string(type_and_substs.ty), fcx.infcx() - .ty_to_string( - actual_structure_type), + .ty_to_string(expr_ty), type_error); tcx.note_and_explain_type_err(&type_error, path.span); } diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index fd1a5595e8f1f..c49f05fa7a2e1 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -735,39 +735,33 @@ fn some_func(x: &mut i32) { "##, E0071: r##" -You tried to use a structure initialization with a non-structure type. +You tried to use structure-literal syntax to create an item that is +not a struct-style structure or enum variant. + Example of erroneous code: ``` -enum Foo { FirstValue }; +enum Foo { FirstValue(i32) }; let u = Foo::FirstValue { value: 0i32 }; // error: Foo::FirstValue // isn't a structure! -// or even simpler, if the structure wasn't defined at all: -let u = RandomName { random_field: 0i32 }; // error: RandomName - // isn't a structure! -``` +// or even simpler, if the name doesn't refer to a structure at all. +let t = u32 { value: 4 }; // error: `u32` does not name a structure.``` -To fix this, please check: - * Did you spell it right? - * Did you accidentaly used an enum as a struct? - * Did you accidentaly make an enum when you intended to use a struct? +To fix this, ensure that the name was correctly spelled, and that +the correct form of initializer was used. -Here is the previous code with all missing information: +For example, the code above can be fixed to: ``` -struct Inner { - value: i32 -} - enum Foo { - FirstValue(Inner) + FirstValue(i32) } fn main() { - let u = Foo::FirstValue(Inner { value: 0i32 }); + let u = Foo::FirstValue(0i32); - let t = Inner { value: 0i32 }; + let t = 4; } ``` "##, @@ -1636,30 +1630,6 @@ fn(isize, *const *const u8) -> isize ``` "##, -E0159: r##" -You tried to use a trait as a struct constructor. Erroneous code example: - -``` -trait TraitNotAStruct {} - -TraitNotAStruct{ value: 0 }; // error: use of trait `TraitNotAStruct` as a - // struct constructor -``` - -Please verify you used the correct type name or please implement the trait -on a struct and use this struct constructor. Example: - -``` -trait TraitNotAStruct {} - -struct Foo { - value: i32 -} - -Foo{ value: 0 }; // ok! -``` -"##, - E0166: r##" This error means that the compiler found a return expression in a function marked as diverging. A function diverges if it has `!` in the place of the @@ -2673,10 +2643,11 @@ register_diagnostics! { E0127, E0129, E0141, +// E0159, // use of trait `{}` as struct constructor E0163, E0164, E0167, - E0168, +// E0168, E0173, // manual implementations of unboxed closure traits are experimental E0174, // explicit use of unboxed closure methods are experimental E0182, diff --git a/src/test/compile-fail-fulldeps/issue-18986.rs b/src/test/compile-fail-fulldeps/issue-18986.rs index 06fc3db58c159..5f7752bb203c2 100644 --- a/src/test/compile-fail-fulldeps/issue-18986.rs +++ b/src/test/compile-fail-fulldeps/issue-18986.rs @@ -15,6 +15,6 @@ pub use use_from_trait_xc::Trait; fn main() { match () { - Trait { x: 42 } => () //~ ERROR use of trait `Trait` in a struct pattern + Trait { x: 42 } => () //~ ERROR `Trait` does not name a struct } } diff --git a/src/test/compile-fail/issue-27815.rs b/src/test/compile-fail/issue-27815.rs new file mode 100644 index 0000000000000..b1ac2dfd1c414 --- /dev/null +++ b/src/test/compile-fail/issue-27815.rs @@ -0,0 +1,20 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +mod A {} + +fn main() { + let u = A { x: 1 }; //~ ERROR `A` does not name a structure + let v = u32 { x: 1 }; //~ ERROR `u32` does not name a structure + match () { + A { x: 1 } => {} //~ ERROR `A` does not name a struct + u32 { x: 1 } => {} //~ ERROR `u32` does not name a struct + } +} diff --git a/src/test/compile-fail/trait-as-struct-constructor.rs b/src/test/compile-fail/trait-as-struct-constructor.rs index b864e6ca9578b..67ccd6b7cd058 100644 --- a/src/test/compile-fail/trait-as-struct-constructor.rs +++ b/src/test/compile-fail/trait-as-struct-constructor.rs @@ -12,5 +12,5 @@ trait TraitNotAStruct {} fn main() { TraitNotAStruct{ value: 0 }; - //~^ ERROR: use of trait `TraitNotAStruct` as a struct constructor [E0159] + //~^ ERROR: `TraitNotAStruct` does not name a structure [E0071] } From ba98228b4d4c2676edd074e39029952d8e841502 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 15 Aug 2015 20:39:28 +0300 Subject: [PATCH 2/4] clean-up ExprStruct and PatStruct type-checking This fixes the crazy "transparent aliases" bug, which I hope nobody relied on. --- src/librustc_typeck/check/_match.rs | 64 ++--- src/librustc_typeck/check/mod.rs | 268 ++++++------------ src/librustc_typeck/diagnostics.rs | 5 +- src/test/compile-fail/issue-15034.rs | 2 +- .../structure-constructor-type-mismatch.rs | 51 +++- 5 files changed, 145 insertions(+), 245 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 9eb372a6c590b..3757c55a162f3 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -19,6 +19,7 @@ use check::{check_expr, check_expr_has_type, check_expr_with_expectation}; use check::{check_expr_coercable_to_type, demand, FnCtxt, Expectation}; use check::{check_expr_with_lvalue_pref, LvaluePreference}; use check::{instantiate_path, resolve_ty_and_def_ufcs, structurally_resolved_type}; +use TypeAndSubsts; use require_same_types; use util::nodemap::FnvHashMap; @@ -526,62 +527,31 @@ pub fn check_pat_struct<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, pat: &'tcx ast::Pat, etc: bool, expected: Ty<'tcx>) { let fcx = pcx.fcx; let tcx = pcx.fcx.ccx.tcx; - let report_nonstruct = || { - let name = pprust::path_to_string(path); - span_err!(tcx.sess, pat.span, E0163, - "`{}` does not name a struct or a struct variant", name); - fcx.write_error(pat.id); - - for field in fields { - check_pat(pcx, &field.node.pat, tcx.types.err); - } - }; let def = tcx.def_map.borrow().get(&pat.id).unwrap().full_def(); - let (adt_def, variant) = match def { - def::DefTy(did, _) | def::DefStruct(did) => { - match tcx.lookup_item_type(did).ty.sty { - ty::TyStruct(struct_def, _) => - (struct_def, struct_def.struct_variant()), - _ => { - report_nonstruct(); - return; - } - } - } - def::DefVariant(eid, vid, true) => { - match tcx.lookup_item_type(vid).ty.sty { - ty::TyEnum(enum_def, _) if enum_def.did == eid => { - (enum_def, enum_def.variant_with_id(vid)) - } - _ => tcx.sess.span_bug(pat.span, "variant's type is not its enum") + let variant = match fcx.def_struct_variant(def) { + Some((_, variant)) => variant, + None => { + let name = pprust::path_to_string(path); + span_err!(tcx.sess, pat.span, E0163, + "`{}` does not name a struct or a struct variant", name); + fcx.write_error(pat.id); + + for field in fields { + check_pat(pcx, &field.node.pat, tcx.types.err); } - } - _ => { - report_nonstruct(); return; } }; - instantiate_path(pcx.fcx, - &path.segments, - adt_def.type_scheme(tcx), - &adt_def.predicates(tcx), - None, - def, - pat.span, - pat.id); - - let pat_ty = fcx.node_ty(pat.id); + let TypeAndSubsts { + ty: pat_ty, substs: item_substs + } = pcx.fcx.instantiate_type(def.def_id(), path); demand::eqtype(fcx, pat.span, expected, pat_ty); - - let item_substs = fcx - .item_substs() - .get(&pat.id) - .map(|substs| substs.substs.clone()) - .unwrap_or_else(|| Substs::empty()); - check_struct_pat_fields(pcx, pat.span, fields, variant, &item_substs, etc); + + fcx.write_ty(pat.id, pat_ty); + fcx.write_substs(pat.id, ty::ItemSubsts { substs: item_substs }); } pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 92f39d972ee19..facd60de6c283 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1379,65 +1379,64 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { cause) } - /// Returns the type of `def_id` with all generics replaced by by fresh type/region variables. - /// Also returns the substitution from the type parameters on `def_id` to the fresh variables. - /// Registers any trait obligations specified on `def_id` at the same time. + /// Instantiates the type in `did` with the generics in `path` and returns + /// it (registering the necessary trait obligations along the way). /// - /// Note that function is only intended to be used with types (notably, not fns). This is - /// because it doesn't do any instantiation of late-bound regions. + /// Note that this function is only intended to be used with type-paths, + /// not with value-paths. pub fn instantiate_type(&self, - span: Span, - def_id: ast::DefId) + did: ast::DefId, + path: &ast::Path) -> TypeAndSubsts<'tcx> { + debug!("instantiate_type(did={:?}, path={:?})", did, path); let type_scheme = - self.tcx().lookup_item_type(def_id); + self.tcx().lookup_item_type(did); let type_predicates = - self.tcx().lookup_predicates(def_id); - let substs = - self.infcx().fresh_substs_for_generics( - span, - &type_scheme.generics); + self.tcx().lookup_predicates(did); + let substs = astconv::ast_path_substs_for_ty(self, self, + path.span, + PathParamMode::Optional, + &type_scheme.generics, + path.segments.last().unwrap()); + debug!("instantiate_type: ty={:?} substs={:?}", &type_scheme.ty, &substs); let bounds = - self.instantiate_bounds(span, &substs, &type_predicates); + self.instantiate_bounds(path.span, &substs, &type_predicates); self.add_obligations_for_parameters( traits::ObligationCause::new( - span, + path.span, self.body_id, - traits::ItemObligation(def_id)), + traits::ItemObligation(did)), &bounds); - let monotype = - self.instantiate_type_scheme(span, &substs, &type_scheme.ty); TypeAndSubsts { - ty: monotype, + ty: self.instantiate_type_scheme(path.span, &substs, &type_scheme.ty), substs: substs } } - /// Returns the type that this AST path refers to. If the path has no type - /// parameters and the corresponding type has type parameters, fresh type - /// and/or region variables are substituted. - /// - /// This is used when checking the constructor in struct literals. - fn instantiate_struct_literal_ty(&self, - struct_ty: ty::TypeScheme<'tcx>, - path: &ast::Path) - -> TypeAndSubsts<'tcx> + pub fn def_struct_variant(&self, + def: def::Def) + -> Option<(ty::AdtDef<'tcx>, ty::VariantDef<'tcx>)> { - let ty::TypeScheme { generics, ty: decl_ty } = struct_ty; - - let substs = astconv::ast_path_substs_for_ty(self, self, - path.span, - PathParamMode::Optional, - &generics, - path.segments.last().unwrap()); - - let ty = self.instantiate_type_scheme(path.span, &substs, &decl_ty); - - TypeAndSubsts { substs: substs, ty: ty } + match def { + def::DefVariant(enum_id, variant_id, true) => { + let adt = self.tcx().lookup_adt_def(enum_id); + Some((adt, adt.variant_with_id(variant_id))) + } + def::DefTy(did, _) | def::DefStruct(did) => { + let typ = self.tcx().lookup_item_type(did); + if let ty::TyStruct(adt, _) = typ.ty.sty { + Some((adt, adt.struct_variant())) + } else { + None + } + } + _ => None + } } + pub fn write_nil(&self, node_id: ast::NodeId) { self.write_ty(node_id, self.tcx().mk_nil()); } @@ -3028,18 +3027,17 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, } - fn check_struct_or_variant_fields<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, - adt_ty: Ty<'tcx>, - span: Span, - variant_id: ast::DefId, - ast_fields: &'tcx [ast::Field], - check_completeness: bool) -> Result<(),()> { + fn check_expr_struct_fields<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, + adt_ty: Ty<'tcx>, + span: Span, + variant: ty::VariantDef<'tcx>, + ast_fields: &'tcx [ast::Field], + check_completeness: bool) { let tcx = fcx.ccx.tcx; - let (adt_def, substs) = match adt_ty.sty { - ty::TyStruct(def, substs) | ty::TyEnum(def, substs) => (def, substs), - _ => tcx.sess.span_bug(span, "non-ADT passed to check_struct_or_variant_fields") + let substs = match adt_ty.sty { + ty::TyStruct(_, substs) | ty::TyEnum(_, substs) => substs, + _ => tcx.sess.span_bug(span, "non-ADT passed to check_expr_struct_fields") }; - let variant = adt_def.variant_with_id(variant_id); let mut remaining_fields = FnvHashMap(); for field in &variant.fields { @@ -3076,7 +3074,6 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, !error_happened && !remaining_fields.is_empty() { - error_happened = true; span_err!(tcx.sess, span, E0063, "missing field{}: {}", if remaining_fields.len() == 1 {""} else {"s"}, @@ -3085,68 +3082,6 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, .collect::>() .join(", ")); } - - if error_happened { Err(()) } else { Ok(()) } - } - - fn check_struct_constructor<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, - id: ast::NodeId, - span: codemap::Span, - struct_def: ty::AdtDef<'tcx>, - fields: &'tcx [ast::Field], - base_expr: Option<&'tcx ast::Expr>) { - let tcx = fcx.ccx.tcx; - - // Generate the struct type. - let TypeAndSubsts { - ty: mut struct_type, - substs: _ - } = fcx.instantiate_type(span, struct_def.did); - - // Look up and check the fields. - let res = check_struct_or_variant_fields(fcx, - struct_type, - span, - struct_def.did, - fields, - base_expr.is_none()); - if res.is_err() { - struct_type = tcx.types.err; - } - - // Check the base expression if necessary. - match base_expr { - None => {} - Some(base_expr) => { - check_expr_has_type(fcx, &*base_expr, struct_type); - } - } - - // Write in the resulting type. - fcx.write_ty(id, struct_type); - } - - fn check_struct_enum_variant<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, - id: ast::NodeId, - span: codemap::Span, - enum_id: ast::DefId, - variant_id: ast::DefId, - fields: &'tcx [ast::Field]) { - // Look up the number of type parameters and the raw type, and - // determine whether the enum is region-parameterized. - let TypeAndSubsts { - ty: enum_type, - substs: _ - } = fcx.instantiate_type(span, enum_id); - - // Look up and check the enum variant fields. - let _ = check_struct_or_variant_fields(fcx, - enum_type, - span, - variant_id, - fields, - true); - fcx.write_ty(id, enum_type); } fn check_struct_fields_on_error<'a,'tcx>(fcx: &FnCtxt<'a,'tcx>, @@ -3165,16 +3100,42 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, } } - fn report_exprstruct_on_nondict<'a, 'tcx>(fcx: &FnCtxt<'a,'tcx>, - id: ast::NodeId, - fields: &'tcx [ast::Field], - base_expr: &'tcx Option>, - path: &ast::Path) + fn check_expr_struct<'a, 'tcx>(fcx: &FnCtxt<'a,'tcx>, + expr: &ast::Expr, + path: &ast::Path, + fields: &'tcx [ast::Field], + base_expr: &'tcx Option>) { - span_err!(fcx.tcx().sess, path.span, E0071, - "`{}` does not name a structure", - pprust::path_to_string(path)); - check_struct_fields_on_error(fcx, id, fields, base_expr) + let tcx = fcx.tcx(); + + // Find the relevant variant + let def = lookup_full_def(tcx, path.span, expr.id); + let (adt, variant) = match fcx.def_struct_variant(def) { + Some((adt, variant)) => (adt, variant), + None => { + span_err!(fcx.tcx().sess, path.span, E0071, + "`{}` does not name a structure", + pprust::path_to_string(path)); + check_struct_fields_on_error(fcx, expr.id, fields, base_expr); + return; + } + }; + + let TypeAndSubsts { + ty: expr_ty, .. + } = fcx.instantiate_type(def.def_id(), path); + fcx.write_ty(expr.id, expr_ty); + + check_expr_struct_fields(fcx, expr_ty, expr.span, variant, fields, + base_expr.is_none()); + + if let &Some(ref base_expr) = base_expr { + check_expr_has_type(fcx, base_expr, expr_ty); + if adt.adt_kind() == ty::AdtKind::Enum { + span_err!(tcx.sess, base_expr.span, E0436, + "functional record update syntax requires a struct"); + } + } } type ExprCheckerWithTy = fn(&FnCtxt, &ast::Expr, Ty); @@ -3625,67 +3586,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, } } ast::ExprStruct(ref path, ref fields, ref base_expr) => { - // Resolve the path. - let def = lookup_full_def(tcx, path.span, id); - - let struct_ty = match def { - def::DefVariant(enum_id, variant_id, true) => { - if let &Some(ref base_expr) = base_expr { - span_err!(tcx.sess, base_expr.span, E0436, - "functional record update syntax requires a struct"); - fcx.write_error(base_expr.id); - } - check_struct_enum_variant(fcx, id, expr.span, enum_id, - variant_id, &fields[..]); - Some(tcx.lookup_item_type(enum_id)) - } - def::DefTy(did, _) | def::DefStruct(did) => { - // Verify that this was actually a struct. - let typ = tcx.lookup_item_type(did); - if let ty::TyStruct(struct_def, _) = typ.ty.sty { - check_struct_constructor(fcx, - id, - expr.span, - struct_def, - &fields, - base_expr.as_ref().map(|e| &**e)); - } else { - report_exprstruct_on_nondict(fcx, id, &fields, base_expr, path); - } - Some(typ) - } - _ => { - report_exprstruct_on_nondict(fcx, id, &fields, base_expr, path); - None - } - }; - - // Turn the path into a type and verify that that type unifies with - // the resulting structure type. This is needed to handle type - // parameters correctly. - if let Some(struct_ty) = struct_ty { - let expr_ty = fcx.expr_ty(&expr); - let type_and_substs = fcx.instantiate_struct_literal_ty(struct_ty, path); - match fcx.mk_subty(false, - infer::Misc(path.span), - expr_ty, - type_and_substs.ty) { - Ok(()) => {} - Err(type_error) => { - span_err!(fcx.tcx().sess, path.span, E0235, - "structure constructor specifies a \ - structure of type `{}`, but this \ - structure has type `{}`: {}", - fcx.infcx() - .ty_to_string(type_and_substs.ty), - fcx.infcx() - .ty_to_string(expr_ty), - type_error); - tcx.note_and_explain_type_err(&type_error, path.span); - } - } - } - + check_expr_struct(fcx, expr, path, fields, base_expr); fcx.require_expr_have_sized_type(expr, traits::StructInitializerSized); } ast::ExprField(ref base, ref field) => { @@ -4673,6 +4574,9 @@ pub fn instantiate_path<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, } } + debug!("instantiate_path: type of {:?} is {:?}", + node_id, + ty_substituted); fcx.write_ty(node_id, ty_substituted); fcx.write_substs(node_id, ty::ItemSubsts { substs: substs }); return; diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index c49f05fa7a2e1..e10b53f7eaf22 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -746,7 +746,8 @@ enum Foo { FirstValue(i32) }; let u = Foo::FirstValue { value: 0i32 }; // error: Foo::FirstValue // isn't a structure! // or even simpler, if the name doesn't refer to a structure at all. -let t = u32 { value: 4 }; // error: `u32` does not name a structure.``` +let t = u32 { value: 4 }; // error: `u32` does not name a structure. +``` To fix this, ensure that the name was correctly spelled, and that the correct form of initializer was used. @@ -2681,7 +2682,7 @@ register_diagnostics! { E0231, // only named substitution parameters are allowed E0233, E0234, - E0235, // structure constructor specifies a structure of type but +// E0235, // structure constructor specifies a structure of type but E0236, // no lang item for range syntax E0237, // no lang item for range syntax E0238, // parenthesized parameters may only be used with a trait diff --git a/src/test/compile-fail/issue-15034.rs b/src/test/compile-fail/issue-15034.rs index 13d27e7152b03..69e10b90bfeba 100644 --- a/src/test/compile-fail/issue-15034.rs +++ b/src/test/compile-fail/issue-15034.rs @@ -25,7 +25,7 @@ struct Parser<'a> { impl<'a> Parser<'a> { pub fn new(lexer: &'a mut Lexer) -> Parser<'a> { Parser { lexer: lexer } - //~^ ERROR cannot infer an appropriate lifetime for lifetime parameter + //~^ ERROR cannot infer an appropriate lifetime } } diff --git a/src/test/compile-fail/structure-constructor-type-mismatch.rs b/src/test/compile-fail/structure-constructor-type-mismatch.rs index 02c4f3d5d5268..7a6b8ff662240 100644 --- a/src/test/compile-fail/structure-constructor-type-mismatch.rs +++ b/src/test/compile-fail/structure-constructor-type-mismatch.rs @@ -24,41 +24,66 @@ type PairF = Pair; fn main() { let pt = PointF { - //~^ ERROR structure constructor specifies a structure of type + x: 1, + //~^ ERROR mismatched types //~| expected f32 //~| found integral variable - x: 1, y: 2, + //~^ ERROR mismatched types + //~| expected f32 + //~| found integral variable }; let pt2 = Point:: { - //~^ ERROR structure constructor specifies a structure of type + x: 3, + //~^ ERROR mismatched types //~| expected f32 //~| found integral variable - x: 3, y: 4, + //~^ ERROR mismatched types + //~| expected f32 + //~| found integral variable }; let pair = PairF { - //~^ ERROR structure constructor specifies a structure of type + x: 5, + //~^ ERROR mismatched types //~| expected f32 //~| found integral variable - x: 5, y: 6, }; let pair2 = PairF:: { - //~^ ERROR structure constructor specifies a structure of type + x: 7, + //~^ ERROR mismatched types //~| expected f32 //~| found integral variable - x: 7, y: 8, }; - let pt3 = PointF:: { - //~^ ERROR wrong number of type arguments - //~| ERROR structure constructor specifies a structure of type - x: 9, - y: 10, + let pt3 = PointF:: { //~ ERROR wrong number of type arguments + x: 9, //~ ERROR mismatched types + y: 10, //~ ERROR mismatched types }; + + match (Point { x: 1, y: 2 }) { + PointF:: { .. } => {} //~ ERROR wrong number of type arguments + //~^ ERROR mismatched types + } + + match (Point { x: 1, y: 2 }) { + PointF { .. } => {} //~ ERROR mismatched types + } + + match (Point { x: 1.0, y: 2.0 }) { + PointF { .. } => {} // ok + } + + match (Pair { x: 1, y: 2 }) { + PairF:: { .. } => {} //~ ERROR mismatched types + } + + match (Pair { x: 1.0, y: 2 }) { + PairF:: { .. } => {} // ok + } } From 91be1258993b592531e8e66181f26ad23ab5b2f4 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 15 Aug 2015 21:03:10 +0300 Subject: [PATCH 3/4] prevent non-dict-structs from being intialized via FRU Fixes #27831 --- src/librustc_typeck/check/mod.rs | 17 ++++++++++---- src/test/compile-fail/issue-27831.rs | 34 ++++++++++++++++++++++++++++ src/test/compile-fail/issue-4736.rs | 2 +- 3 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 src/test/compile-fail/issue-27831.rs diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index facd60de6c283..74df7d8e58a11 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1415,24 +1415,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Return the dict-like variant corresponding to a given `Def`. pub fn def_struct_variant(&self, def: def::Def) -> Option<(ty::AdtDef<'tcx>, ty::VariantDef<'tcx>)> { - match def { + let (adt, variant) = match def { def::DefVariant(enum_id, variant_id, true) => { let adt = self.tcx().lookup_adt_def(enum_id); - Some((adt, adt.variant_with_id(variant_id))) + (adt, adt.variant_with_id(variant_id)) } def::DefTy(did, _) | def::DefStruct(did) => { let typ = self.tcx().lookup_item_type(did); if let ty::TyStruct(adt, _) = typ.ty.sty { - Some((adt, adt.struct_variant())) + (adt, adt.struct_variant()) } else { - None + return None; } } - _ => None + _ => return None + }; + + if let ty::VariantKind::Dict = variant.kind() { + Some((adt, variant)) + } else { + None } } diff --git a/src/test/compile-fail/issue-27831.rs b/src/test/compile-fail/issue-27831.rs new file mode 100644 index 0000000000000..336368cf8a49d --- /dev/null +++ b/src/test/compile-fail/issue-27831.rs @@ -0,0 +1,34 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct Foo(u32); +struct Bar; + +enum Enum { + Foo(u32), + Bar +} + +fn main() { + let x = Foo(1); + Foo { ..x }; //~ ERROR `Foo` does not name a structure + let Foo { .. } = x; //~ ERROR `Foo` does not name a struct + + let x = Bar; + Bar { ..x }; //~ ERROR `Bar` does not name a structure + let Bar { .. } = x; //~ ERROR `Bar` does not name a struct + + match Enum::Bar { + Enum::Bar { .. } //~ ERROR `Enum::Bar` does not name a struct + => {} + Enum::Foo { .. } //~ ERROR `Enum::Foo` does not name a struct + => {} + } +} diff --git a/src/test/compile-fail/issue-4736.rs b/src/test/compile-fail/issue-4736.rs index b63db7c5a30e5..843ff38df49cb 100644 --- a/src/test/compile-fail/issue-4736.rs +++ b/src/test/compile-fail/issue-4736.rs @@ -11,5 +11,5 @@ struct NonCopyable(()); fn main() { - let z = NonCopyable{ p: () }; //~ ERROR structure `NonCopyable` has no field named `p` + let z = NonCopyable{ p: () }; //~ ERROR `NonCopyable` does not name a structure } From 44f41063dd241320cbe19b078862fc29eb66c097 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 16 Aug 2015 01:20:36 +0300 Subject: [PATCH 4/4] use the correct substs when checking struct patterns also, make sure this is tested. --- src/librustc_typeck/check/_match.rs | 11 +++++---- src/librustc_typeck/check/mod.rs | 11 +++------ src/test/run-pass/struct-aliases.rs | 38 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 3757c55a162f3..0ac9e0a9c59a0 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -19,7 +19,6 @@ use check::{check_expr, check_expr_has_type, check_expr_with_expectation}; use check::{check_expr_coercable_to_type, demand, FnCtxt, Expectation}; use check::{check_expr_with_lvalue_pref, LvaluePreference}; use check::{instantiate_path, resolve_ty_and_def_ufcs, structurally_resolved_type}; -use TypeAndSubsts; use require_same_types; use util::nodemap::FnvHashMap; @@ -544,14 +543,16 @@ pub fn check_pat_struct<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, pat: &'tcx ast::Pat, } }; - let TypeAndSubsts { - ty: pat_ty, substs: item_substs - } = pcx.fcx.instantiate_type(def.def_id(), path); + let pat_ty = pcx.fcx.instantiate_type(def.def_id(), path); + let item_substs = match pat_ty.sty { + ty::TyStruct(_, substs) | ty::TyEnum(_, substs) => substs, + _ => tcx.sess.span_bug(pat.span, "struct variant is not an ADT") + }; demand::eqtype(fcx, pat.span, expected, pat_ty); check_struct_pat_fields(pcx, pat.span, fields, variant, &item_substs, etc); fcx.write_ty(pat.id, pat_ty); - fcx.write_substs(pat.id, ty::ItemSubsts { substs: item_substs }); + fcx.write_substs(pat.id, ty::ItemSubsts { substs: item_substs.clone() }); } pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 74df7d8e58a11..7c35aea685032 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1387,7 +1387,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pub fn instantiate_type(&self, did: ast::DefId, path: &ast::Path) - -> TypeAndSubsts<'tcx> + -> Ty<'tcx> { debug!("instantiate_type(did={:?}, path={:?})", did, path); let type_scheme = @@ -1409,10 +1409,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { traits::ItemObligation(did)), &bounds); - TypeAndSubsts { - ty: self.instantiate_type_scheme(path.span, &substs, &type_scheme.ty), - substs: substs - } + self.instantiate_type_scheme(path.span, &substs, &type_scheme.ty) } /// Return the dict-like variant corresponding to a given `Def`. @@ -3128,9 +3125,7 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, } }; - let TypeAndSubsts { - ty: expr_ty, .. - } = fcx.instantiate_type(def.def_id(), path); + let expr_ty = fcx.instantiate_type(def.def_id(), path); fcx.write_ty(expr.id, expr_ty); check_expr_struct_fields(fcx, expr_ty, expr.span, variant, fields, diff --git a/src/test/run-pass/struct-aliases.rs b/src/test/run-pass/struct-aliases.rs index 7107243d760a4..f1337a5b07974 100644 --- a/src/test/run-pass/struct-aliases.rs +++ b/src/test/run-pass/struct-aliases.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::mem; struct S { x: isize, @@ -16,6 +17,13 @@ struct S { type S2 = S; +struct S3 { + x: U, + y: V +} + +type S4 = S3; + fn main() { let s = S2 { x: 1, @@ -30,4 +38,34 @@ fn main() { assert_eq!(y, 2); } } + // check that generics can be specified from the pattern + let s = S4 { + x: 4, + y: 'a' + }; + match s { + S4:: { + x: x, + y: y + } => { + assert_eq!(x, 4); + assert_eq!(y, 'a'); + assert_eq!(mem::size_of_val(&x), 1); + } + }; + // check that generics can be specified from the constructor + let s = S4:: { + x: 5, + y: 'b' + }; + match s { + S4 { + x: x, + y: y + } => { + assert_eq!(x, 5); + assert_eq!(y, 'b'); + assert_eq!(mem::size_of_val(&x), 2); + } + }; }