From 01d0ae96188b99c367178a38906b7269e1ada244 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 10 Jan 2019 23:23:30 +0300 Subject: [PATCH] Prioritize variants as inherent associated items during name resolution --- src/librustc/lint/builtin.rs | 7 + src/librustc_lint/lib.rs | 5 + src/librustc_typeck/astconv.rs | 162 +++++++++++------- src/librustc_typeck/check/method/mod.rs | 61 +++---- src/librustc_typeck/check/mod.rs | 4 +- .../ui/type-alias-enum-variants-priority-2.rs | 13 ++ ...type-alias-enum-variants-priority-2.stderr | 12 ++ .../ui/type-alias-enum-variants-priority-3.rs | 10 ++ ...type-alias-enum-variants-priority-3.stderr | 8 + .../ui/type-alias-enum-variants-priority.rs | 20 +++ .../type-alias-enum-variants-priority.stderr | 26 +++ 11 files changed, 225 insertions(+), 103 deletions(-) create mode 100644 src/test/ui/type-alias-enum-variants-priority-2.rs create mode 100644 src/test/ui/type-alias-enum-variants-priority-2.stderr create mode 100644 src/test/ui/type-alias-enum-variants-priority-3.rs create mode 100644 src/test/ui/type-alias-enum-variants-priority-3.stderr create mode 100644 src/test/ui/type-alias-enum-variants-priority.rs create mode 100644 src/test/ui/type-alias-enum-variants-priority.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index c8d137a42b2e8..a0bd4f01cd231 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -368,6 +368,12 @@ declare_lint! { report_in_external_macro: true } +declare_lint! { + pub AMBIGUOUS_ASSOCIATED_ITEMS, + Warn, + "ambiguous associated items" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -433,6 +439,7 @@ impl LintPass for HardwiredLints { parser::QUESTION_MARK_MACRO_SEP, parser::ILL_FORMED_ATTRIBUTE_INPUT, DEPRECATED_IN_FUTURE, + AMBIGUOUS_ASSOCIATED_ITEMS, ) } } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 2d764d11c66aa..71c859d8dbea7 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -341,6 +341,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue #57571 ", edition: None, }, + FutureIncompatibleInfo { + id: LintId::of(AMBIGUOUS_ASSOCIATED_ITEMS), + reference: "issue #57644 ", + edition: None, + }, ]); // Register renamed and removed lints. diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 8e5eaa18b9de0..534279e981d62 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -10,6 +10,7 @@ use hir::HirVec; use lint; use middle::resolve_lifetime as rl; use namespace::Namespace; +use rustc::lint::builtin::AMBIGUOUS_ASSOCIATED_ITEMS; use rustc::traits; use rustc::ty::{self, Ty, TyCtxt, ToPredicate, TypeFoldable}; use rustc::ty::{GenericParamDef, GenericParamDefKind}; @@ -1278,29 +1279,50 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { } // Create a type from a path to an associated type. - // For a path `A::B::C::D`, `ty` and `ty_path_def` are the type and def for `A::B::C` + // For a path `A::B::C::D`, `qself_ty` and `qself_def` are the type and def for `A::B::C` // and item_segment is the path segment for `D`. We return a type and a def for // the whole path. - // Will fail except for `T::A` and `Self::A`; i.e., if `ty`/`ty_path_def` are not a type + // Will fail except for `T::A` and `Self::A`; i.e., if `qself_ty`/`qself_def` are not a type // parameter or `Self`. - pub fn associated_path_def_to_ty(&self, - ref_id: ast::NodeId, - span: Span, - ty: Ty<'tcx>, - ty_path_def: Def, - item_segment: &hir::PathSegment) - -> (Ty<'tcx>, Def) - { + pub fn associated_path_to_ty( + &self, + ref_id: ast::NodeId, + span: Span, + qself_ty: Ty<'tcx>, + qself_def: Def, + assoc_segment: &hir::PathSegment, + permit_variants: bool, + ) -> (Ty<'tcx>, Def) { let tcx = self.tcx(); - let assoc_name = item_segment.ident; + let assoc_ident = assoc_segment.ident; - debug!("associated_path_def_to_ty: {:?}::{}", ty, assoc_name); + debug!("associated_path_to_ty: {:?}::{}", qself_ty, assoc_ident); - self.prohibit_generics(slice::from_ref(item_segment)); + self.prohibit_generics(slice::from_ref(assoc_segment)); + + // Check if we have an enum variant. + let mut variant_resolution = None; + if let ty::Adt(adt_def, _) = qself_ty.sty { + if adt_def.is_enum() { + let variant_def = adt_def.variants.iter().find(|vd| { + tcx.hygienic_eq(assoc_ident, vd.ident, adt_def.did) + }); + if let Some(variant_def) = variant_def { + let def = Def::Variant(variant_def.did); + if permit_variants { + check_type_alias_enum_variants_enabled(tcx, span); + tcx.check_stability(variant_def.did, Some(ref_id), span); + return (qself_ty, def); + } else { + variant_resolution = Some(def); + } + } + } + } // Find the type of the associated item, and the trait where the associated // item is declared. - let bound = match (&ty.sty, ty_path_def) { + let bound = match (&qself_ty.sty, qself_def) { (_, Def::SelfTy(Some(_), Some(impl_def_id))) => { // `Self` in an impl of a trait -- we have a concrete self type and a // trait reference. @@ -1313,77 +1335,61 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { }; let candidates = traits::supertraits(tcx, ty::Binder::bind(trait_ref)) - .filter(|r| self.trait_defines_associated_type_named(r.def_id(), assoc_name)); + .filter(|r| self.trait_defines_associated_type_named(r.def_id(), assoc_ident)); - match self.one_bound_for_assoc_type(candidates, "Self", assoc_name, span) { + match self.one_bound_for_assoc_type(candidates, "Self", assoc_ident, span) { Ok(bound) => bound, Err(ErrorReported) => return (tcx.types.err, Def::Err), } } (&ty::Param(_), Def::SelfTy(Some(param_did), None)) | (&ty::Param(_), Def::TyParam(param_did)) => { - match self.find_bound_for_assoc_item(param_did, assoc_name, span) { + match self.find_bound_for_assoc_item(param_did, assoc_ident, span) { Ok(bound) => bound, Err(ErrorReported) => return (tcx.types.err, Def::Err), } } - (&ty::Adt(adt_def, _substs), Def::Enum(_did)) => { - let ty_str = ty.to_string(); - // Incorrect enum variant. - let mut err = tcx.sess.struct_span_err( - span, - &format!("no variant `{}` on enum `{}`", &assoc_name.as_str(), ty_str), - ); - // Check if it was a typo. - let input = adt_def.variants.iter().map(|variant| &variant.ident.name); - if let Some(suggested_name) = find_best_match_for_name( - input, - &assoc_name.as_str(), - None, - ) { - err.span_suggestion_with_applicability( + _ => { + if variant_resolution.is_some() { + // Variant in type position + let msg = format!("expected type, found variant `{}`", assoc_ident); + tcx.sess.span_err(span, &msg); + } else if qself_ty.is_enum() { + // Report as incorrect enum variant rather than ambiguous type. + let mut err = tcx.sess.struct_span_err( span, - "did you mean", - format!("{}::{}", ty_str, suggested_name.to_string()), - Applicability::MaybeIncorrect, + &format!("no variant `{}` on enum `{}`", &assoc_ident.as_str(), qself_ty), ); - } else { - err.span_label(span, "unknown variant"); - } - err.emit(); - return (tcx.types.err, Def::Err); - } - _ => { - // Check if we have an enum variant. - match ty.sty { - ty::Adt(adt_def, _) if adt_def.is_enum() => { - let variant_def = adt_def.variants.iter().find(|vd| { - tcx.hygienic_eq(assoc_name, vd.ident, adt_def.did) - }); - if let Some(variant_def) = variant_def { - check_type_alias_enum_variants_enabled(tcx, span); - - let def = Def::Variant(variant_def.did); - tcx.check_stability(def.def_id(), Some(ref_id), span); - return (ty, def); - } - }, - _ => (), - } - - // Don't print `TyErr` to the user. - if !ty.references_error() { + // Check if it was a typo. + let adt_def = qself_ty.ty_adt_def().expect("enum is not an ADT"); + if let Some(suggested_name) = find_best_match_for_name( + adt_def.variants.iter().map(|variant| &variant.ident.name), + &assoc_ident.as_str(), + None, + ) { + err.span_suggestion_with_applicability( + span, + "did you mean", + format!("{}::{}", qself_ty, suggested_name), + Applicability::MaybeIncorrect, + ); + } else { + err.span_label(span, "unknown variant"); + } + err.emit(); + } else if !qself_ty.references_error() { + // Don't print `TyErr` to the user. self.report_ambiguous_associated_type(span, - &ty.to_string(), + &qself_ty.to_string(), "Trait", - &assoc_name.as_str()); + &assoc_ident.as_str()); } return (tcx.types.err, Def::Err); } }; let trait_did = bound.def_id(); - let (assoc_ident, def_scope) = tcx.adjust_ident(assoc_name, trait_did, ref_id); + let (assoc_ident, def_scope) = tcx.adjust_ident(assoc_ident, trait_did, ref_id); let item = tcx.associated_items(trait_did).find(|i| { Namespace::from(i.kind) == Namespace::Type && i.ident.modern() == assoc_ident @@ -1394,11 +1400,35 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { let def = Def::AssociatedTy(item.def_id); if !item.vis.is_accessible_from(def_scope, tcx) { - let msg = format!("{} `{}` is private", def.kind_name(), assoc_name); + let msg = format!("{} `{}` is private", def.kind_name(), assoc_ident); tcx.sess.span_err(span, &msg); } tcx.check_stability(item.def_id, Some(ref_id), span); + if let Some(variant_def) = variant_resolution { + let mut err = tcx.struct_span_lint_node( + AMBIGUOUS_ASSOCIATED_ITEMS, + ref_id, + span, + "ambiguous associated item", + ); + + let mut could_refer_to = |def: Def, also| { + let note_msg = format!("`{}` could{} refer to {} defined here", + assoc_ident, also, def.kind_name()); + err.span_note(tcx.def_span(def.def_id()), ¬e_msg); + }; + could_refer_to(variant_def, ""); + could_refer_to(def, " also"); + + err.span_suggestion_with_applicability( + span, + "use fully-qualified syntax", + format!("<{} as {}>::{}", qself_ty, "Trait", assoc_ident), + Applicability::HasPlaceholders, + ).emit(); + } + (ty, def) } @@ -1773,7 +1803,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { } else { Def::Err }; - self.associated_path_def_to_ty(ast_ty.id, ast_ty.span, ty, def, segment).0 + self.associated_path_to_ty(ast_ty.id, ast_ty.span, ty, def, segment, false).0 } hir::TyKind::Array(ref ty, ref length) => { let length_def_id = tcx.hir().local_def_id(length.id); diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 02687df6a94fc..e71dc019471bd 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -408,45 +408,36 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let tcx = self.tcx; - let mode = probe::Mode::Path; - match self.probe_for_name(span, mode, method_name, IsSuggestion(false), - self_ty, expr_id, ProbeScope::TraitsInScope) { - Ok(pick) => { - debug!("resolve_ufcs: pick={:?}", pick); - if let Some(import_id) = pick.import_id { - let import_def_id = tcx.hir().local_def_id(import_id); - debug!("resolve_ufcs: used_trait_import: {:?}", import_def_id); - Lrc::get_mut(&mut self.tables.borrow_mut().used_trait_imports) - .unwrap().insert(import_def_id); + // Check if we have an enum variant. + if let ty::Adt(adt_def, _) = self_ty.sty { + if adt_def.is_enum() { + let variant_def = adt_def.variants.iter().find(|vd| { + tcx.hygienic_eq(method_name, vd.ident, adt_def.did) + }); + if let Some(variant_def) = variant_def { + check_type_alias_enum_variants_enabled(tcx, span); + + let def = Def::VariantCtor(variant_def.did, variant_def.ctor_kind); + tcx.check_stability(def.def_id(), Some(expr_id), span); + return Ok(def); } - - let def = pick.item.def(); - debug!("resolve_ufcs: def={:?}", def); - tcx.check_stability(def.def_id(), Some(expr_id), span); - - Ok(def) } - Err(err) => { - // Check if we have an enum variant. - match self_ty.sty { - ty::Adt(adt_def, _) if adt_def.is_enum() => { - let variant_def = adt_def.variants.iter().find(|vd| { - tcx.hygienic_eq(method_name, vd.ident, adt_def.did) - }); - if let Some(variant_def) = variant_def { - check_type_alias_enum_variants_enabled(tcx, span); - - let def = Def::VariantCtor(variant_def.did, variant_def.ctor_kind); - tcx.check_stability(def.def_id(), Some(expr_id), span); - return Ok(def); - } - }, - _ => (), - } + } - Err(err) - } + let pick = self.probe_for_name(span, probe::Mode::Path, method_name, IsSuggestion(false), + self_ty, expr_id, ProbeScope::TraitsInScope)?; + debug!("resolve_ufcs: pick={:?}", pick); + if let Some(import_id) = pick.import_id { + let import_def_id = tcx.hir().local_def_id(import_id); + debug!("resolve_ufcs: used_trait_import: {:?}", import_def_id); + Lrc::get_mut(&mut self.tables.borrow_mut().used_trait_imports) + .unwrap().insert(import_def_id); } + + let def = pick.item.def(); + debug!("resolve_ufcs: def={:?}", def); + tcx.check_stability(def.def_id(), Some(expr_id), span); + Ok(def) } /// Find item with name `item_name` defined in impl/trait `def_id` diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 1b07385d4d1f4..22300f2ae11ed 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -4724,8 +4724,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } else { Def::Err }; - let (ty, def) = AstConv::associated_path_def_to_ty(self, node_id, path_span, - ty, def, segment); + let (ty, def) = AstConv::associated_path_to_ty(self, node_id, path_span, + ty, def, segment, true); // Write back the new resolution. let hir_id = self.tcx.hir().node_to_hir_id(node_id); diff --git a/src/test/ui/type-alias-enum-variants-priority-2.rs b/src/test/ui/type-alias-enum-variants-priority-2.rs new file mode 100644 index 0000000000000..295f8acf62f85 --- /dev/null +++ b/src/test/ui/type-alias-enum-variants-priority-2.rs @@ -0,0 +1,13 @@ +#![feature(type_alias_enum_variants)] + +enum E { + V(u8) +} + +impl E { + fn V() {} +} + +fn main() { + ::V(); //~ ERROR this function takes 1 parameter but 0 parameters were supplied +} diff --git a/src/test/ui/type-alias-enum-variants-priority-2.stderr b/src/test/ui/type-alias-enum-variants-priority-2.stderr new file mode 100644 index 0000000000000..c6ec96ebb7d3b --- /dev/null +++ b/src/test/ui/type-alias-enum-variants-priority-2.stderr @@ -0,0 +1,12 @@ +error[E0061]: this function takes 1 parameter but 0 parameters were supplied + --> $DIR/type-alias-enum-variants-priority-2.rs:12:5 + | +LL | V(u8) + | ----- defined here +... +LL | ::V(); //~ ERROR this function takes 1 parameter but 0 parameters were supplied + | ^^^^^^^^ expected 1 parameter + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0061`. diff --git a/src/test/ui/type-alias-enum-variants-priority-3.rs b/src/test/ui/type-alias-enum-variants-priority-3.rs new file mode 100644 index 0000000000000..33f96553b57f0 --- /dev/null +++ b/src/test/ui/type-alias-enum-variants-priority-3.rs @@ -0,0 +1,10 @@ +#![feature(type_alias_enum_variants)] + +enum E { + V +} + +fn check() -> ::V {} +//~^ ERROR expected type, found variant `V` + +fn main() {} diff --git a/src/test/ui/type-alias-enum-variants-priority-3.stderr b/src/test/ui/type-alias-enum-variants-priority-3.stderr new file mode 100644 index 0000000000000..b3451542a2570 --- /dev/null +++ b/src/test/ui/type-alias-enum-variants-priority-3.stderr @@ -0,0 +1,8 @@ +error: expected type, found variant `V` + --> $DIR/type-alias-enum-variants-priority-3.rs:7:15 + | +LL | fn check() -> ::V {} + | ^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/type-alias-enum-variants-priority.rs b/src/test/ui/type-alias-enum-variants-priority.rs new file mode 100644 index 0000000000000..db1da2b12e256 --- /dev/null +++ b/src/test/ui/type-alias-enum-variants-priority.rs @@ -0,0 +1,20 @@ +#![feature(type_alias_enum_variants)] +#![deny(ambiguous_associated_items)] + +enum E { + V +} + +trait Tr { + type V; + fn f() -> Self::V; +} + +impl Tr for E { + type V = u8; + fn f() -> Self::V { 0 } + //~^ ERROR ambiguous associated item + //~| WARN this was previously accepted +} + +fn main() {} diff --git a/src/test/ui/type-alias-enum-variants-priority.stderr b/src/test/ui/type-alias-enum-variants-priority.stderr new file mode 100644 index 0000000000000..dcf7dc77ed5ea --- /dev/null +++ b/src/test/ui/type-alias-enum-variants-priority.stderr @@ -0,0 +1,26 @@ +error: ambiguous associated item + --> $DIR/type-alias-enum-variants-priority.rs:15:15 + | +LL | fn f() -> Self::V { 0 } + | ^^^^^^^ help: use fully-qualified syntax: `::V` + | +note: lint level defined here + --> $DIR/type-alias-enum-variants-priority.rs:2:9 + | +LL | #![deny(ambiguous_associated_items)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #57644 +note: `V` could refer to variant defined here + --> $DIR/type-alias-enum-variants-priority.rs:5:5 + | +LL | V + | ^ +note: `V` could also refer to associated type defined here + --> $DIR/type-alias-enum-variants-priority.rs:9:5 + | +LL | type V; + | ^^^^^^^ + +error: aborting due to previous error +