Skip to content

Commit

Permalink
Auto merge of #63463 - matthewjasper:ty_param_cleanup, r=petrochenkov
Browse files Browse the repository at this point in the history
Don't special case the `Self` parameter by name

This results in a couple of small diagnostic regressions. They could be avoided by keeping the special case just for diagnostics, but that seems worse.

closes #50125
cc #60869
  • Loading branch information
bors committed Aug 19, 2019
2 parents 0ccbae2 + 24587d2 commit a807902
Show file tree
Hide file tree
Showing 24 changed files with 124 additions and 183 deletions.
11 changes: 1 addition & 10 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2524,15 +2524,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 @@ -2551,7 +2542,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
109 changes: 57 additions & 52 deletions src/librustc/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ 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 violations = traits::supertrait_def_ids(self, trait_def_id)
.filter(|&def_id| self.predicates_reference_self(def_id, true))
.map(|_| ObjectSafetyViolation::SupertraitSelf)
Expand All @@ -106,16 +107,33 @@ 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);
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))
.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);
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,
}
}

fn object_safety_violations_for_trait(self, trait_def_id: DefId) -> Vec<ObjectSafetyViolation> {
// Check methods for violations.
let mut violations: Vec<_> = self.associated_items(trait_def_id)
.filter(|item| item.kind == ty::AssocKind::Method)
Expand Down Expand Up @@ -163,14 +181,16 @@ impl<'tcx> TyCtxt<'tcx> {
fn predicates_reference_self(
self,
trait_def_id: DefId,
supertraits_only: bool) -> bool
{
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 self_ty = self.types.self_param;
let has_self_ty = |t: Ty<'tcx>| t.walk().any(|t| t == self_ty);
predicates
.predicates
.iter()
Expand All @@ -179,7 +199,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 +219,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 Down Expand Up @@ -229,11 +249,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().is_param(0)
}
ty::Predicate::Projection(..) |
ty::Predicate::Trait(..) |
ty::Predicate::Subtype(..) |
ty::Predicate::RegionOutlives(..) |
ty::Predicate::WellFormed(..) |
Expand All @@ -248,11 +268,11 @@ 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,
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.
Expand All @@ -263,36 +283,15 @@ impl<'tcx> TyCtxt<'tcx> {
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,
}
}

/// 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,
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 Down Expand Up @@ -323,7 +322,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, t)
}) {
let span = self.def_span(method.def_id);
return Some(MethodViolationCode::WhereClauseReferencesSelf(span));
}
Expand All @@ -337,7 +338,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.types.self_param {
if !self.receiver_is_dispatchable(method, receiver_ty) {
return Some(MethodViolationCode::UndispatchableReceiver);
} else {
Expand Down Expand Up @@ -404,7 +405,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 @@ -555,7 +559,7 @@ impl<'tcx> TyCtxt<'tcx> {
// Self: Unsize<U>
let unsize_predicate = ty::TraitRef {
def_id: unsize_did,
substs: self.mk_substs_trait(self.mk_self_type(), &[unsized_self_ty.into()]),
substs: self.mk_substs_trait(self.types.self_param, &[unsized_self_ty.into()]),
}.to_predicate();

// U: Trait<Arg1, ..., ArgN>
Expand Down Expand Up @@ -608,11 +612,11 @@ 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,
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 @@ -654,10 +658,11 @@ impl<'tcx> TyCtxt<'tcx> {

let mut supertraits: Option<Vec<ty::PolyTraitRef<'tcx>>> = None;
let mut error = false;
let self_ty = self.types.self_param;
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
9 changes: 5 additions & 4 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ pub struct CommonTypes<'tcx> {
pub f32: Ty<'tcx>,
pub f64: Ty<'tcx>,
pub never: Ty<'tcx>,
pub self_param: Ty<'tcx>,
pub err: Ty<'tcx>,

/// Dummy type used for the `Self` of a `TraitRef` created for converting
Expand Down Expand Up @@ -915,6 +916,10 @@ impl<'tcx> CommonTypes<'tcx> {
u128: mk(Uint(ast::UintTy::U128)),
f32: mk(Float(ast::FloatTy::F32)),
f64: mk(Float(ast::FloatTy::F64)),
self_param: mk(ty::Param(ty::ParamTy {
index: 0,
name: kw::SelfUpper.as_interned_str(),
})),

trait_object_dummy_self: mk(Infer(ty::FreshTy(0))),
}
Expand Down Expand Up @@ -2566,10 +2571,6 @@ impl<'tcx> TyCtxt<'tcx> {
})
}

#[inline]
pub fn mk_self_type(self) -> Ty<'tcx> {
self.mk_ty_param(0, kw::SelfUpper.as_interned_str())
}

pub fn mk_param_from_def(self, param: &ty::GenericParamDef) -> Kind<'tcx> {
match param.kind {
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
5 changes: 3 additions & 2 deletions src/librustc/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,9 @@ impl<'tcx> Instance<'tcx> {
) -> Option<Instance<'tcx>> {
debug!("resolve(def_id={:?}, substs={:?})", def_id, substs);
let fn_sig = tcx.fn_sig(def_id);
let is_vtable_shim =
fn_sig.inputs().skip_binder().len() > 0 && fn_sig.input(0).skip_binder().is_self();
let is_vtable_shim = fn_sig.inputs().skip_binder().len() > 0
&& fn_sig.input(0).skip_binder().is_param(0)
&& tcx.generics_of(def_id).has_self;
if is_vtable_shim {
debug!(" => associated item with unsizeable self: Self");
Some(Instance {
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,6 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
// resulting from the final codegen session.
if
layout.ty.has_param_types() ||
layout.ty.has_self_ty() ||
!self.param_env.caller_bounds.is_empty()
{
return;
Expand Down Expand Up @@ -1767,7 +1766,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
let tail = tcx.struct_tail_erasing_lifetimes(pointee, param_env);
match tail.sty {
ty::Param(_) | ty::Projection(_) => {
debug_assert!(tail.has_param_types() || tail.has_self_ty());
debug_assert!(tail.has_param_types());
Ok(SizeSkeleton::Pointer {
non_zero,
tail: tcx.erase_regions(&tail)
Expand Down
Loading

0 comments on commit a807902

Please sign in to comment.