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

Move Generics from MethodSig to TraitItem and ImplItem #44766

Merged
merged 11 commits into from
Oct 24, 2017
8 changes: 4 additions & 4 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,9 +780,7 @@ pub fn walk_fn_kind<'v, V: Visitor<'v>>(visitor: &mut V, function_kind: FnKind<'
FnKind::ItemFn(_, generics, ..) => {
visitor.visit_generics(generics);
}
FnKind::Method(_, sig, ..) => {
visitor.visit_generics(&sig.generics);
}
FnKind::Method(..) |
FnKind::Closure(_) => {}
}
}
Expand All @@ -802,6 +800,7 @@ pub fn walk_fn<'v, V: Visitor<'v>>(visitor: &mut V,
pub fn walk_trait_item<'v, V: Visitor<'v>>(visitor: &mut V, trait_item: &'v TraitItem) {
visitor.visit_name(trait_item.span, trait_item.name);
walk_list!(visitor, visit_attribute, &trait_item.attrs);
visitor.visit_generics(&trait_item.generics);
match trait_item.node {
TraitItemKind::Const(ref ty, default) => {
visitor.visit_id(trait_item.id);
Expand All @@ -810,7 +809,6 @@ pub fn walk_trait_item<'v, V: Visitor<'v>>(visitor: &mut V, trait_item: &'v Trai
}
TraitItemKind::Method(ref sig, TraitMethod::Required(ref names)) => {
visitor.visit_id(trait_item.id);
visitor.visit_generics(&sig.generics);
visitor.visit_fn_decl(&sig.decl);
for name in names {
visitor.visit_name(name.span, name.node);
Expand Down Expand Up @@ -852,6 +850,7 @@ pub fn walk_impl_item<'v, V: Visitor<'v>>(visitor: &mut V, impl_item: &'v ImplIt
ref vis,
ref defaultness,
ref attrs,
ref generics,
ref node,
span
} = *impl_item;
Expand All @@ -860,6 +859,7 @@ pub fn walk_impl_item<'v, V: Visitor<'v>>(visitor: &mut V, impl_item: &'v ImplIt
visitor.visit_vis(vis);
visitor.visit_defaultness(defaultness);
walk_list!(visitor, visit_attribute, attrs);
visitor.visit_generics(generics);
match *node {
ImplItemKind::Const(ref ty, body) => {
visitor.visit_id(impl_item.id);
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,7 @@ impl<'a> LoweringContext<'a> {
hir_id,
name: this.lower_ident(i.ident),
attrs: this.lower_attrs(&i.attrs),
generics: this.lower_generics(&i.generics),
node: match i.node {
TraitItemKind::Const(ref ty, ref default) => {
hir::TraitItemKind::Const(this.lower_ty(ty),
Expand Down Expand Up @@ -1603,6 +1604,7 @@ impl<'a> LoweringContext<'a> {
hir_id,
name: this.lower_ident(i.ident),
attrs: this.lower_attrs(&i.attrs),
generics: this.lower_generics(&i.generics),
vis: this.lower_visibility(&i.vis, None),
defaultness: this.lower_defaultness(i.defaultness, true /* [1] */),
node: match i.node {
Expand Down Expand Up @@ -1729,7 +1731,6 @@ impl<'a> LoweringContext<'a> {

fn lower_method_sig(&mut self, sig: &MethodSig) -> hir::MethodSig {
hir::MethodSig {
generics: self.lower_generics(&sig.generics),
abi: sig.abi,
unsafety: self.lower_unsafety(sig.unsafety),
constness: self.lower_constness(sig.constness),
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,6 @@ pub struct MethodSig {
pub constness: Constness,
pub abi: Abi,
pub decl: P<FnDecl>,
pub generics: Generics,
}

// The bodies for items are stored "out of line", in a separate
Expand All @@ -1316,6 +1315,7 @@ pub struct TraitItem {
pub name: Name,
pub hir_id: HirId,
pub attrs: HirVec<Attribute>,
pub generics: Generics,
pub node: TraitItemKind,
pub span: Span,
}
Expand Down Expand Up @@ -1360,6 +1360,7 @@ pub struct ImplItem {
pub vis: Visibility,
pub defaultness: Defaultness,
pub attrs: HirVec<Attribute>,
pub generics: Generics,
pub node: ImplItemKind,
pub span: Span,
}
Expand Down
11 changes: 7 additions & 4 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ impl<'a> State<'a> {
pub fn print_method_sig(&mut self,
name: ast::Name,
m: &hir::MethodSig,
generics: &hir::Generics,
vis: &hir::Visibility,
arg_names: &[Spanned<ast::Name>],
body_id: Option<hir::BodyId>)
Expand All @@ -889,7 +890,7 @@ impl<'a> State<'a> {
m.constness,
m.abi,
Some(name),
&m.generics,
generics,
vis,
arg_names,
body_id)
Expand All @@ -905,12 +906,14 @@ impl<'a> State<'a> {
self.print_associated_const(ti.name, &ty, default, &hir::Inherited)?;
}
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Required(ref arg_names)) => {
self.print_method_sig(ti.name, sig, &hir::Inherited, arg_names, None)?;
self.print_method_sig(ti.name, sig, &ti.generics, &hir::Inherited, arg_names,
None)?;
self.s.word(";")?;
}
hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Provided(body)) => {
self.head("")?;
self.print_method_sig(ti.name, sig, &hir::Inherited, &[], Some(body))?;
self.print_method_sig(ti.name, sig, &ti.generics, &hir::Inherited, &[],
Some(body))?;
self.nbsp()?;
self.end()?; // need to close a box
self.end()?; // need to close a box
Expand Down Expand Up @@ -938,7 +941,7 @@ impl<'a> State<'a> {
}
hir::ImplItemKind::Method(ref sig, body) => {
self.head("")?;
self.print_method_sig(ii.name, sig, &ii.vis, &[], Some(body))?;
self.print_method_sig(ii.name, sig, &ii.generics, &ii.vis, &[], Some(body))?;
self.nbsp()?;
self.end()?; // need to close a box
self.end()?; // need to close a box
Expand Down
7 changes: 5 additions & 2 deletions src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,7 @@ impl_stable_hash_for!(struct hir::MethodSig {
unsafety,
constness,
abi,
decl,
generics
decl
});

impl_stable_hash_for!(struct hir::TypeBinding {
Expand Down Expand Up @@ -709,13 +708,15 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::TraitItem {
hir_id: _,
name,
ref attrs,
ref generics,
ref node,
span
} = *self;

hcx.hash_hir_item_like(attrs, |hcx| {
name.hash_stable(hcx, hasher);
attrs.hash_stable(hcx, hasher);
generics.hash_stable(hcx, hasher);
node.hash_stable(hcx, hasher);
span.hash_stable(hcx, hasher);
});
Expand Down Expand Up @@ -744,6 +745,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::ImplItem {
ref vis,
defaultness,
ref attrs,
ref generics,
ref node,
span
} = *self;
Expand All @@ -753,6 +755,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::ImplItem {
vis.hash_stable(hcx, hasher);
defaultness.hash_stable(hcx, hasher);
attrs.hash_stable(hcx, hasher);
generics.hash_stable(hcx, hasher);
node.hash_stable(hcx, hasher);
span.hash_stable(hcx, hasher);
});
Expand Down
11 changes: 5 additions & 6 deletions src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,10 @@ fn item_might_be_inlined(item: &hir::Item) -> bool {
}

fn method_might_be_inlined<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
sig: &hir::MethodSig,
impl_item: &hir::ImplItem,
impl_src: DefId) -> bool {
if attr::requests_inline(&impl_item.attrs) ||
generics_require_inlining(&sig.generics) {
generics_require_inlining(&impl_item.generics) {
return true
}
if let Some(impl_node_id) = tcx.hir.as_local_node_id(impl_src) {
Expand Down Expand Up @@ -176,8 +175,8 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
Some(hir_map::NodeImplItem(impl_item)) => {
match impl_item.node {
hir::ImplItemKind::Const(..) => true,
hir::ImplItemKind::Method(ref sig, _) => {
if generics_require_inlining(&sig.generics) ||
hir::ImplItemKind::Method(..) => {
if generics_require_inlining(&impl_item.generics) ||
attr::requests_inline(&impl_item.attrs) {
true
} else {
Expand Down Expand Up @@ -293,9 +292,9 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
hir::ImplItemKind::Const(_, body) => {
self.visit_nested_body(body);
}
hir::ImplItemKind::Method(ref sig, body) => {
hir::ImplItemKind::Method(_, body) => {
let did = self.tcx.hir.get_parent_did(search_item);
if method_might_be_inlined(self.tcx, sig, impl_item, did) {
if method_might_be_inlined(self.tcx, impl_item, did) {
self.visit_nested_body(body)
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
if let hir::TraitItemKind::Method(ref sig, _) = trait_item.node {
self.visit_early_late(
Some(self.hir_map.get_parent(trait_item.id)),
&sig.decl, &sig.generics,
&sig.decl, &trait_item.generics,
|this| intravisit::walk_trait_item(this, trait_item))
} else {
intravisit::walk_trait_item(self, trait_item);
Expand All @@ -423,7 +423,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
if let hir::ImplItemKind::Method(ref sig, _) = impl_item.node {
self.visit_early_late(
Some(self.hir_map.get_parent(impl_item.id)),
&sig.decl, &sig.generics,
&sig.decl, &impl_item.generics,
|this| intravisit::walk_impl_item(this, impl_item))
} else {
intravisit::walk_impl_item(self, impl_item);
Expand Down
8 changes: 3 additions & 5 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,12 +718,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
_: Span,
node_id: NodeId) {
let rib_kind = match function_kind {
FnKind::ItemFn(_, generics, ..) => {
self.visit_generics(generics);
FnKind::ItemFn(..) => {
ItemRibKind
}
FnKind::Method(_, sig, _, _) => {
self.visit_generics(&sig.generics);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about whether this change has any side-effects we don't want. I don't think I found a place to add this back in after I deleted it.

Copy link
Contributor

@nikomatsakis nikomatsakis Sep 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, we will need to add this back somewhere =) I think the problem is that the FnKind enum has to change. Methods should operate more analogously with ItemFn -- so we should add a &'a Generics to the Method variant, and then when we construct FnKind::Method (here and here) we can add the data from the trait or impl item respectively. Then we can restore the call to visit_generics that occurs here, I suppose.

This was wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, let me read a bit more into this actually. I think what I said is not wrong but a few more tweaks are likely needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, so I think it's fine to remove the call here -- it is being moved into the walk_trait_item and walk_impl_item code, effectively.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change really okay? If ItemFn visits generics, shouldn't Method visit them too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. It happens as part of visit_trait_item or visit_impl_item, I think.

MethodRibKind(!sig.decl.has_self())
}
FnKind::Closure(_) => ClosureRibKind(node_id),
Expand Down Expand Up @@ -1880,7 +1878,7 @@ impl<'a> Resolver<'a> {
}
TraitItemKind::Method(ref sig, _) => {
let type_parameters =
HasTypeParameters(&sig.generics,
HasTypeParameters(&trait_item.generics,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll need to add this type parameter rib to each variant in this match statement because each HasTypeParamters will need a different rib kind. Since nothing other than methods has generics right now, would it work to save that change until we are actually implementing associated type generics?

(this applies to both trait items and impl items)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it depends. I would think we just want to rename MethodRibKind to something else like TraitOrImplItemRibKind, but it might be important to distinguish those cases for error messages? I kinda' doubt it, but have to look around.

MethodRibKind(!sig.decl.has_self()));
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_trait_item(this, trait_item)
Expand Down Expand Up @@ -2097,7 +2095,7 @@ impl<'a> Resolver<'a> {
// We also need a new scope for the method-
// specific type parameters.
let type_parameters =
HasTypeParameters(&sig.generics,
HasTypeParameters(&impl_item.generics,
MethodRibKind(!sig.decl.has_self()));
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_impl_item(this, impl_item);
Expand Down
9 changes: 6 additions & 3 deletions src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,23 +354,24 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
body: Option<&'l ast::Block>,
id: ast::NodeId,
name: ast::Ident,
generics: &'l ast::Generics,
vis: ast::Visibility,
span: Span) {
debug!("process_method: {}:{}", id, name);

if let Some(mut method_data) = self.save_ctxt.get_method_data(id, name.name, span) {

let sig_str = ::make_signature(&sig.decl, &sig.generics);
let sig_str = ::make_signature(&sig.decl, &generics);
if body.is_some() {
self.nest_tables(id, |v| {
v.process_formals(&sig.decl.inputs, &method_data.qualname)
});
}

self.process_generic_params(&sig.generics, span, &method_data.qualname, id);
self.process_generic_params(&generics, span, &method_data.qualname, id);

method_data.value = sig_str;
method_data.sig = sig::method_signature(id, name, sig, &self.save_ctxt);
method_data.sig = sig::method_signature(id, name, generics, sig, &self.save_ctxt);
self.dumper.dump_def(vis == ast::Visibility::Public, method_data);
}

Expand Down Expand Up @@ -1007,6 +1008,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
body.as_ref().map(|x| &**x),
trait_item.id,
trait_item.ident,
&trait_item.generics,
ast::Visibility::Public,
trait_item.span);
}
Expand Down Expand Up @@ -1066,6 +1068,7 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
Some(body),
impl_item.id,
impl_item.ident,
&impl_item.generics,
impl_item.vis.clone(),
impl_item.span);
}
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_save_analysis/sig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,14 @@ pub fn variant_signature(variant: &ast::Variant, scx: &SaveContext) -> Option<Si

pub fn method_signature(id: NodeId,
ident: ast::Ident,
generics: &ast::Generics,
m: &ast::MethodSig,
scx: &SaveContext)
-> Option<Signature> {
if !scx.config.signatures {
return None;
}
make_method_signature(id, ident, m, scx).ok()
make_method_signature(id, ident, generics, m, scx).ok()
}

pub fn assoc_const_signature(id: NodeId,
Expand Down Expand Up @@ -895,6 +896,7 @@ fn make_assoc_const_signature(id: NodeId,

fn make_method_signature(id: NodeId,
ident: ast::Ident,
generics: &ast::Generics,
m: &ast::MethodSig,
scx: &SaveContext)
-> Result {
Expand All @@ -915,7 +917,7 @@ fn make_method_signature(id: NodeId,

let mut sig = name_and_generics(text,
0,
&m.generics,
generics,
id,
ident,
scx)?;
Expand Down
14 changes: 5 additions & 9 deletions src/librustc_typeck/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,15 +568,11 @@ fn compare_number_of_generics<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let num_trait_m_type_params = trait_m_generics.types.len();
if num_impl_m_type_params != num_trait_m_type_params {
let impl_m_node_id = tcx.hir.as_local_node_id(impl_m.def_id).unwrap();
let span = match tcx.hir.expect_impl_item(impl_m_node_id).node {
ImplItemKind::Method(ref impl_m_sig, _) => {
if impl_m_sig.generics.is_parameterized() {
impl_m_sig.generics.span
} else {
impl_m_span
}
}
_ => bug!("{:?} is not a method", impl_m),
let impl_m_item = tcx.hir.expect_impl_item(impl_m_node_id);
let span = if impl_m_item.generics.is_parameterized() {
impl_m_item.generics.span
} else {
impl_m_span
};

let mut err = struct_span_err!(tcx.sess,
Expand Down
Loading