Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't special case the Self parameter by name #63463

Merged
merged 4 commits into from
Aug 19, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 1 addition & 10 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2886,15 +2886,6 @@ impl<'a> LoweringContext<'a> {
(param_name, kind)
}
GenericParamKind::Type { ref default, .. } => {
// Don't expose `Self` (recovered "keyword used as ident" parse error).
// `rustc::ty` expects `Self` to be only used for a trait's `Self`.
// Instead, use `gensym("Self")` to create a distinct name that looks the same.
let ident = if param.ident.name == kw::SelfUpper {
param.ident.gensym()
} else {
param.ident
};

let add_bounds = add_bounds.get(&param.id).map_or(&[][..], |x| &x);
if !add_bounds.is_empty() {
let params = self.lower_param_bounds(add_bounds, itctx.reborrow()).into_iter();
Expand All @@ -2913,7 +2904,7 @@ impl<'a> LoweringContext<'a> {
.next(),
};

(hir::ParamName::Plain(ident), kind)
(hir::ParamName::Plain(param.ident), kind)
}
GenericParamKind::Const { ref ty } => {
(hir::ParamName::Plain(param.ident), hir::GenericParamKind::Const {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1329,15 +1329,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let generics = self.tcx.generics_of(did);
// Account for the case where `did` corresponds to `Self`, which doesn't have
// the expected type argument.
if !param.is_self() {
if !(generics.has_self && param.index == 0) {
let type_param = generics.type_param(param, self.tcx);
let hir = &self.tcx.hir();
hir.as_local_hir_id(type_param.def_id).map(|id| {
// Get the `hir::Param` to verify whether it already has any bounds.
// We do this to avoid suggesting code that ends up as `T: 'a'b`,
// instead we suggest `T: 'a + 'b` in that case.
let mut has_bounds = false;
if let Node::GenericParam(ref param) = hir.get(id) {
if let Node::GenericParam(param) = hir.get(id) {
has_bounds = !param.bounds.is_empty();
}
let sp = hir.span(id);
Expand Down
144 changes: 81 additions & 63 deletions src/librustc/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,10 @@ impl<'tcx> TyCtxt<'tcx> {
pub fn astconv_object_safety_violations(self, trait_def_id: DefId)
-> Vec<ObjectSafetyViolation>
{
debug_assert!(self.generics_of(trait_def_id).has_self);
let self_ty = self.mk_self_type();
let violations = traits::supertrait_def_ids(self, trait_def_id)
.filter(|&def_id| self.predicates_reference_self(def_id, true))
.filter(|&def_id| self.predicates_reference_self(def_id, self_ty, true))
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
.map(|_| ObjectSafetyViolation::SupertraitSelf)
.collect();

Expand All @@ -106,21 +108,44 @@ impl<'tcx> TyCtxt<'tcx> {
pub fn object_safety_violations(self, trait_def_id: DefId)
-> Vec<ObjectSafetyViolation>
{
debug_assert!(self.generics_of(trait_def_id).has_self);
let self_ty = self.mk_self_type();
debug!("object_safety_violations: {:?}", trait_def_id);

traits::supertrait_def_ids(self, trait_def_id)
.flat_map(|def_id| self.object_safety_violations_for_trait(def_id))
.flat_map(|def_id| self.object_safety_violations_for_trait(def_id, self_ty))
.collect()
}

fn object_safety_violations_for_trait(self, trait_def_id: DefId)
-> Vec<ObjectSafetyViolation>
{
/// We say a method is *vtable safe* if it can be invoked on a trait
/// object. Note that object-safe traits can have some
/// non-vtable-safe methods, so long as they require `Self:Sized` or
/// otherwise ensure that they cannot be used when `Self=Trait`.
pub fn is_vtable_safe_method(self, trait_def_id: DefId, method: &ty::AssocItem) -> bool {
debug_assert!(self.generics_of(trait_def_id).has_self);
let self_ty = self.mk_self_type();
debug!("is_vtable_safe_method({:?}, {:?})", trait_def_id, method);
// Any method that has a `Self : Sized` requisite can't be called.
if self.generics_require_sized_self(method.def_id, self_ty) {
return false;
}

match self.virtual_call_violation_for_method(trait_def_id, self_ty, method) {
None | Some(MethodViolationCode::WhereClauseReferencesSelf(_)) => true,
Some(_) => false,
}
}

fn object_safety_violations_for_trait(
self,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
) -> Vec<ObjectSafetyViolation> {
// Check methods for violations.
let mut violations: Vec<_> = self.associated_items(trait_def_id)
.filter(|item| item.kind == ty::AssocKind::Method)
.filter_map(|item|
self.object_safety_violation_for_method(trait_def_id, &item)
self.object_safety_violation_for_method(trait_def_id, self_ty, &item)
.map(|code| ObjectSafetyViolation::Method(item.ident.name, code))
).filter(|violation| {
if let ObjectSafetyViolation::Method(_,
Expand All @@ -142,10 +167,10 @@ impl<'tcx> TyCtxt<'tcx> {
}).collect();

// Check the trait itself.
if self.trait_has_sized_self(trait_def_id) {
if self.trait_has_sized_self(trait_def_id, self_ty) {
violations.push(ObjectSafetyViolation::SizedSelf);
}
if self.predicates_reference_self(trait_def_id, false) {
if self.predicates_reference_self(trait_def_id, self_ty, false) {
violations.push(ObjectSafetyViolation::SupertraitSelf);
}

Expand All @@ -163,14 +188,16 @@ impl<'tcx> TyCtxt<'tcx> {
fn predicates_reference_self(
self,
trait_def_id: DefId,
supertraits_only: bool) -> bool
{
self_ty: Ty<'tcx>,
supertraits_only: bool,
) -> bool {
let trait_ref = ty::Binder::dummy(ty::TraitRef::identity(self, trait_def_id));
let predicates = if supertraits_only {
self.super_predicates_of(trait_def_id)
} else {
self.predicates_of(trait_def_id)
};
let has_self_ty = |t: Ty<'tcx>| t.walk().any(|t| t == self_ty);
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
predicates
.predicates
.iter()
Expand All @@ -179,7 +206,7 @@ impl<'tcx> TyCtxt<'tcx> {
match predicate {
ty::Predicate::Trait(ref data) => {
// In the case of a trait predicate, we can skip the "self" type.
data.skip_binder().input_types().skip(1).any(|t| t.has_self_ty())
data.skip_binder().input_types().skip(1).any(has_self_ty)
}
ty::Predicate::Projection(ref data) => {
// And similarly for projections. This should be redundant with
Expand All @@ -199,7 +226,7 @@ impl<'tcx> TyCtxt<'tcx> {
.trait_ref(self)
.input_types()
.skip(1)
.any(|t| t.has_self_ty())
.any(has_self_ty)
}
ty::Predicate::WellFormed(..) |
ty::Predicate::ObjectSafe(..) |
Expand All @@ -214,11 +241,11 @@ impl<'tcx> TyCtxt<'tcx> {
})
}

fn trait_has_sized_self(self, trait_def_id: DefId) -> bool {
self.generics_require_sized_self(trait_def_id)
fn trait_has_sized_self(self, trait_def_id: DefId, self_ty: Ty<'tcx>) -> bool {
self.generics_require_sized_self(trait_def_id, self_ty)
}

fn generics_require_sized_self(self, def_id: DefId) -> bool {
fn generics_require_sized_self(self, def_id: DefId, self_ty: Ty<'tcx>) -> bool {
let sized_def_id = match self.lang_items().sized_trait() {
Some(def_id) => def_id,
None => { return false; /* No Sized trait, can't require it! */ }
Expand All @@ -229,11 +256,11 @@ impl<'tcx> TyCtxt<'tcx> {
let predicates = predicates.instantiate_identity(self).predicates;
elaborate_predicates(self, predicates)
.any(|predicate| match predicate {
ty::Predicate::Trait(ref trait_pred) if trait_pred.def_id() == sized_def_id => {
trait_pred.skip_binder().self_ty().is_self()
ty::Predicate::Trait(ref trait_pred) => {
trait_pred.def_id() == sized_def_id
&& trait_pred.skip_binder().self_ty() == self_ty
}
ty::Predicate::Projection(..) |
ty::Predicate::Trait(..) |
ty::Predicate::Subtype(..) |
ty::Predicate::RegionOutlives(..) |
ty::Predicate::WellFormed(..) |
Expand All @@ -248,51 +275,32 @@ impl<'tcx> TyCtxt<'tcx> {
}

/// Returns `Some(_)` if this method makes the containing trait not object safe.
fn object_safety_violation_for_method(self,
trait_def_id: DefId,
method: &ty::AssocItem)
-> Option<MethodViolationCode>
{
fn object_safety_violation_for_method(
self,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
method: &ty::AssocItem,
) -> Option<MethodViolationCode> {
debug!("object_safety_violation_for_method({:?}, {:?})", trait_def_id, method);
// Any method that has a `Self : Sized` requisite is otherwise
// exempt from the regulations.
if self.generics_require_sized_self(method.def_id) {
if self.generics_require_sized_self(method.def_id, self_ty) {
return None;
}

self.virtual_call_violation_for_method(trait_def_id, method)
}

/// We say a method is *vtable safe* if it can be invoked on a trait
/// object. Note that object-safe traits can have some
/// non-vtable-safe methods, so long as they require `Self:Sized` or
/// otherwise ensure that they cannot be used when `Self=Trait`.
pub fn is_vtable_safe_method(self,
trait_def_id: DefId,
method: &ty::AssocItem)
-> bool
{
debug!("is_vtable_safe_method({:?}, {:?})", trait_def_id, method);
// Any method that has a `Self : Sized` requisite can't be called.
if self.generics_require_sized_self(method.def_id) {
return false;
}

match self.virtual_call_violation_for_method(trait_def_id, method) {
None | Some(MethodViolationCode::WhereClauseReferencesSelf(_)) => true,
Some(_) => false,
}
self.virtual_call_violation_for_method(trait_def_id, self_ty, method)
}

/// Returns `Some(_)` if this method cannot be called on a trait
/// object; this does not necessarily imply that the enclosing trait
/// is not object safe, because the method might have a where clause
/// `Self:Sized`.
fn virtual_call_violation_for_method(self,
trait_def_id: DefId,
method: &ty::AssocItem)
-> Option<MethodViolationCode>
{
fn virtual_call_violation_for_method(
self,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
method: &ty::AssocItem,
) -> Option<MethodViolationCode> {
// The method's first parameter must be named `self`
if !method.method_has_self_argument {
return Some(MethodViolationCode::StaticMethod);
Expand All @@ -301,11 +309,15 @@ impl<'tcx> TyCtxt<'tcx> {
let sig = self.fn_sig(method.def_id);

for input_ty in &sig.skip_binder().inputs()[1..] {
if self.contains_illegal_self_type_reference(trait_def_id, input_ty) {
if self.contains_illegal_self_type_reference(trait_def_id, self_ty, input_ty) {
return Some(MethodViolationCode::ReferencesSelf);
}
}
if self.contains_illegal_self_type_reference(trait_def_id, sig.output().skip_binder()) {
if self.contains_illegal_self_type_reference(
trait_def_id,
self_ty,
sig.output().skip_binder(),
) {
return Some(MethodViolationCode::ReferencesSelf);
}

Expand All @@ -323,7 +335,9 @@ impl<'tcx> TyCtxt<'tcx> {
.collect::<Vec<_>>()
// Do a shallow visit so that `contains_illegal_self_type_reference`
// may apply it's custom visiting.
.visit_tys_shallow(|t| self.contains_illegal_self_type_reference(trait_def_id, t)) {
.visit_tys_shallow(|t| {
self.contains_illegal_self_type_reference(trait_def_id, self_ty, t)
}) {
let span = self.def_span(method.def_id);
return Some(MethodViolationCode::WhereClauseReferencesSelf(span));
}
Expand All @@ -337,7 +351,7 @@ impl<'tcx> TyCtxt<'tcx> {
// However, this is already considered object-safe. We allow it as a special case here.
// FIXME(mikeyhew) get rid of this `if` statement once `receiver_is_dispatchable` allows
// `Receiver: Unsize<Receiver[Self => dyn Trait]>`
if receiver_ty != self.mk_self_type() {
if receiver_ty != self_ty {
if !self.receiver_is_dispatchable(method, receiver_ty) {
return Some(MethodViolationCode::UndispatchableReceiver);
} else {
Expand Down Expand Up @@ -404,7 +418,10 @@ impl<'tcx> TyCtxt<'tcx> {
/// Performs a type substitution to produce the version of receiver_ty when `Self = self_ty`
/// e.g., for receiver_ty = `Rc<Self>` and self_ty = `Foo`, returns `Rc<Foo>`.
fn receiver_for_self_ty(
self, receiver_ty: Ty<'tcx>, self_ty: Ty<'tcx>, method_def_id: DefId
self,
receiver_ty: Ty<'tcx>,
self_ty: Ty<'tcx>,
method_def_id: DefId,
) -> Ty<'tcx> {
debug!("receiver_for_self_ty({:?}, {:?}, {:?})", receiver_ty, self_ty, method_def_id);
let substs = InternalSubsts::for_item(self, method_def_id, |param, _| {
Expand Down Expand Up @@ -608,11 +625,12 @@ impl<'tcx> TyCtxt<'tcx> {
})
}

fn contains_illegal_self_type_reference(self,
trait_def_id: DefId,
ty: Ty<'tcx>)
-> bool
{
fn contains_illegal_self_type_reference(
self,
trait_def_id: DefId,
self_ty: Ty<'tcx>,
ty: Ty<'tcx>,
) -> bool {
// This is somewhat subtle. In general, we want to forbid
// references to `Self` in the argument and return types,
// since the value of `Self` is erased. However, there is one
Expand Down Expand Up @@ -656,8 +674,8 @@ impl<'tcx> TyCtxt<'tcx> {
let mut error = false;
ty.maybe_walk(|ty| {
match ty.sty {
ty::Param(ref param_ty) => {
if param_ty.is_self() {
ty::Param(_) => {
if ty == self_ty {
error = true;
}

Expand Down
8 changes: 1 addition & 7 deletions src/librustc/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,7 @@ impl<'tcx> ty::TyS<'tcx> {
ty::Infer(ty::FreshFloatTy(_)) => "fresh floating-point type".into(),
ty::Projection(_) => "associated type".into(),
ty::UnnormalizedProjection(_) => "non-normalized associated type".into(),
ty::Param(ref p) => {
if p.is_self() {
"Self".into()
} else {
"type parameter".into()
}
}
ty::Param(_) => "type parameter".into(),
ty::Opaque(..) => "opaque type".into(),
ty::Error => "type error".into(),
}
Expand Down
17 changes: 4 additions & 13 deletions src/librustc/ty/flags.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::ty::subst::{SubstsRef, UnpackedKind};
use crate::ty::{self, Ty, TypeFlags, TypeFoldable, InferConst};
use crate::ty::{self, Ty, TypeFlags, InferConst};
use crate::mir::interpret::ConstValue;

#[derive(Debug)]
Expand Down Expand Up @@ -86,13 +86,9 @@ impl FlagComputation {
self.add_flags(TypeFlags::HAS_TY_ERR)
}

&ty::Param(ref p) => {
&ty::Param(_) => {
self.add_flags(TypeFlags::HAS_FREE_LOCAL_NAMES);
if p.is_self() {
self.add_flags(TypeFlags::HAS_SELF);
} else {
self.add_flags(TypeFlags::HAS_PARAMS);
}
self.add_flags(TypeFlags::HAS_PARAMS);
}

&ty::Generator(_, ref substs, _) => {
Expand Down Expand Up @@ -143,11 +139,6 @@ impl FlagComputation {
}

&ty::Projection(ref data) => {
// currently we can't normalize projections that
// include bound regions, so track those separately.
if !data.has_escaping_bound_vars() {
self.add_flags(TypeFlags::HAS_NORMALIZABLE_PROJECTION);
}
self.add_flags(TypeFlags::HAS_PROJECTION);
self.add_projection_ty(data);
}
Expand Down Expand Up @@ -243,7 +234,7 @@ impl FlagComputation {
match c.val {
ConstValue::Unevaluated(_, substs) => {
self.add_substs(substs);
self.add_flags(TypeFlags::HAS_NORMALIZABLE_PROJECTION | TypeFlags::HAS_PROJECTION);
self.add_flags(TypeFlags::HAS_PROJECTION);
},
ConstValue::Infer(infer) => {
self.add_flags(TypeFlags::HAS_FREE_LOCAL_NAMES | TypeFlags::HAS_CT_INFER);
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
fn has_param_types(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_PARAMS)
}
fn has_self_ty(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_SELF)
}
fn has_infer_types(&self) -> bool {
self.has_type_flags(TypeFlags::HAS_TY_INFER)
}
Expand Down
Loading