From 01d0ae96188b99c367178a38906b7269e1ada244 Mon Sep 17 00:00:00 2001
From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
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 <https://github.com/rust-lang/rust/issues/57571>",
edition: None,
},
+ FutureIncompatibleInfo {
+ id: LintId::of(AMBIGUOUS_ASSOCIATED_ITEMS),
+ reference: "issue #57644 <https://github.com/rust-lang/rust/issues/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() {
+ <E>::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 | <E>::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() -> <E>::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() -> <E>::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: `<E as Trait>::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 <https://github.com/rust-lang/rust/issues/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
+