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.

Sign up for GitHub

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

HIR - Move Generics from MethodSig to TraitItem and ImplItem #44852

Closed
wants to merge 6 commits into from
Closed

HIR - Move Generics from MethodSig to TraitItem and ImplItem #44852

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
26395e4
Lifting Generics from MethodSig to TraitItem and ImplItem since we wa…
sunjay Sep 22, 2017
a9924c6
Fixed tidy errors
sunjay Sep 22, 2017
ea6b18e
Removed redundant generics visits
sunjay Sep 25, 2017
ad16f9c
Lifted generics into TraitItem and ImplItem from MethodSig -- HIR now…
sunjay Sep 25, 2017
540d08b
Fixed tidy errors
sunjay Sep 25, 2017
037aa16
Fixing rustdoc
sunjay Sep 26, 2017
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
8 changes: 4 additions & 4 deletions src/librustc/hir/intravisit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,9 +773,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(..) |
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this is a removal. If you trace the code up to the usages of walk_fn, I don't see where else this is used. I think it should be alright, but it's worth noting the consequences of this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll try to do a look to get a feel for whether there will be impact -- but as we said before, really this call is just moving into visit_trait_item and visit_impl_item, so it's not like it's a pure removal.

FnKind::Closure(_) => {}
}
}
Expand All @@ -795,6 +793,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 @@ -803,7 +802,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 @@ -845,6 +843,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 @@ -853,6 +852,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,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 @@ -1599,6 +1600,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 @@ -1725,7 +1727,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,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 @@ -1255,6 +1254,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 @@ -1299,6 +1299,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
9 changes: 5 additions & 4 deletions src/librustc/hir/print.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,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 @@ -888,7 +889,7 @@ impl<'a> State<'a> {
m.constness,
m.abi,
Some(name),
&m.generics,
generics,
vis,
arg_names,
body_id)
Expand All @@ -904,12 +905,12 @@ 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 @@ -937,7 +938,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,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 @@ -695,6 +694,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::TraitItem {
hir_id: _,
name,
ref attrs,
ref generics,
ref node,
span
} = *self;
Expand All @@ -703,6 +703,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::TraitItem {
id.hash_stable(hcx, hasher);
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 @@ -731,6 +732,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::ImplItem {
ref vis,
defaultness,
ref attrs,
ref generics,
ref node,
span
} = *self;
Expand All @@ -741,6 +743,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,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 @@ -421,7 +421,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
5 changes: 2 additions & 3 deletions src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,6 @@ impl<'a, 'tcx> Visitor<'tcx> for Resolver<'a> {
ItemRibKind
}
FnKind::Method(_, sig, _, _) => {
self.visit_generics(&sig.generics);
MethodRibKind(!sig.decl.has_self())
}
FnKind::Closure(_) => ClosureRibKind(node_id),
Expand Down Expand Up @@ -1861,7 +1860,7 @@ impl<'a> Resolver<'a> {
}
TraitItemKind::Method(ref sig, _) => {
let type_parameters =
HasTypeParameters(&sig.generics,
HasTypeParameters(&trait_item.generics,
MethodRibKind(!sig.decl.has_self()));
this.with_type_parameter_rib(type_parameters, |this| {
visit::walk_trait_item(this, trait_item)
Expand Down Expand Up @@ -2078,7 +2077,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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),
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 it still a bug if this isn't a method? If so, we should add the match back in. The match was taken out because technically all impl items have generics now, not just methods.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it has to be a bug -- this code is currently only used for methods, I believe, but I expect we'll be using it or something similar for all trait/impl items before we are done.

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