Skip to content

Commit

Permalink
auto merge of #15691 : jbclements/rust/method-field-cleanup, r=alexcr…
Browse files Browse the repository at this point in the history
…ichton

This patch applies the excellent suggestion of @pnkfelix to group the helper methods for method field access into a Trait, making the code much more readable, and much more similar to the way it was before.
  • Loading branch information
bors committed Jul 16, 2014
2 parents 6c35d51 + ca05828 commit efbbb51
Show file tree
Hide file tree
Showing 18 changed files with 180 additions and 134 deletions.
5 changes: 3 additions & 2 deletions src/librustc/metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use syntax::ast_map::{PathElem, PathElems};
use syntax::ast_map;
use syntax::ast_util::*;
use syntax::ast_util;
use syntax::ast_util::PostExpansionMethod;
use syntax::attr;
use syntax::attr::AttrMetaMethods;
use syntax::diagnostic::SpanHandler;
Expand Down Expand Up @@ -798,7 +799,7 @@ fn encode_info_for_method(ecx: &EncodeContext,
} else {
encode_symbol(ecx, ebml_w, m.def_id.node);
}
encode_method_argument_names(ebml_w, method_fn_decl(&*ast_method));
encode_method_argument_names(ebml_w, &*ast_method.pe_fn_decl());
}

ebml_w.end_tag();
Expand Down Expand Up @@ -1240,7 +1241,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
encode_method_sort(ebml_w, 'p');
encode_inlined_item(ecx, ebml_w,
IIMethodRef(def_id, true, &*m));
encode_method_argument_names(ebml_w, method_fn_decl(m));
encode_method_argument_names(ebml_w, m.pe_fn_decl());
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc/middle/astencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use middle::{ty, typeck};
use util::ppaux::ty_to_string;

use syntax::{ast, ast_map, ast_util, codemap, fold};
use syntax::ast_util::PostExpansionMethod;
use syntax::codemap::Span;
use syntax::fold::Folder;
use syntax::parse::token;
Expand Down Expand Up @@ -136,7 +137,7 @@ pub fn decode_inlined_item(cdata: &cstore::crate_metadata,
let ident = match ii {
ast::IIItem(i) => i.ident,
ast::IIForeign(i) => i.ident,
ast::IIMethod(_, _, m) => ast_util::method_ident(&*m),
ast::IIMethod(_, _, m) => m.pe_ident(),
};
debug!("Fn named: {}", token::get_ident(ident));
debug!("< Decoded inlined fn: {}::{}",
Expand Down
8 changes: 3 additions & 5 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ use util::nodemap::NodeSet;
use std::collections::HashSet;
use syntax::ast;
use syntax::ast_map;
use syntax::ast_util;
use syntax::ast_util::{local_def, is_local};
use syntax::ast_util::{local_def, is_local, PostExpansionMethod};
use syntax::attr::AttrMetaMethods;
use syntax::attr;
use syntax::codemap;
Expand Down Expand Up @@ -213,7 +212,7 @@ impl<'a> MarkSymbolVisitor<'a> {
visit::walk_trait_method(self, &*trait_method, ctxt);
}
ast_map::NodeMethod(method) => {
visit::walk_block(self, ast_util::method_body(&*method), ctxt);
visit::walk_block(self, method.pe_body(), ctxt);
}
ast_map::NodeForeignItem(foreign_item) => {
visit::walk_foreign_item(self, &*foreign_item, ctxt);
Expand Down Expand Up @@ -521,8 +520,7 @@ impl<'a> Visitor<()> for DeadVisitor<'a> {
// Overwrite so that we don't warn the trait method itself.
fn visit_trait_method(&mut self, trait_method: &ast::TraitMethod, _: ()) {
match *trait_method {
ast::Provided(ref method) => visit::walk_block(self,
ast_util::method_body(&**method), ()),
ast::Provided(ref method) => visit::walk_block(self, method.pe_body(), ()),
ast::Required(_) => ()
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use middle::typeck::MethodCall;
use util::ppaux;

use syntax::ast;
use syntax::ast_util;
use syntax::ast_util::PostExpansionMethod;
use syntax::codemap::Span;
use syntax::visit;
use syntax::visit::Visitor;
Expand Down Expand Up @@ -95,7 +95,7 @@ impl<'a> Visitor<()> for EffectCheckVisitor<'a> {
visit::FkItemFn(_, _, fn_style, _) =>
(true, fn_style == ast::UnsafeFn),
visit::FkMethod(_, _, method) =>
(true, ast_util::method_fn_style(method) == ast::UnsafeFn),
(true, method.pe_fn_style() == ast::UnsafeFn),
_ => (false, false),
};

Expand Down
21 changes: 10 additions & 11 deletions src/librustc/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ use util::nodemap::{NodeMap, NodeSet};

use syntax::ast;
use syntax::ast_map;
use syntax::ast_util;
use syntax::ast_util::{is_local, local_def};
use syntax::ast_util::{is_local, local_def, PostExpansionMethod};
use syntax::attr;
use syntax::codemap::Span;
use syntax::parse::token;
Expand Down Expand Up @@ -264,10 +263,10 @@ impl<'a> Visitor<()> for EmbargoVisitor<'a> {

if public_ty || public_trait {
for method in methods.iter() {
let meth_public = match ast_util::method_explicit_self(&**method).node {
let meth_public = match method.pe_explicit_self().node {
ast::SelfStatic => public_ty,
_ => true,
} && ast_util::method_vis(&**method) == ast::Public;
} && method.pe_vis() == ast::Public;
if meth_public || tr.is_some() {
self.exported_items.insert(method.id);
}
Expand Down Expand Up @@ -457,8 +456,8 @@ impl<'a> PrivacyVisitor<'a> {
let imp = self.tcx.map.get_parent_did(closest_private_id);
match ty::impl_trait_ref(self.tcx, imp) {
Some(..) => return Allowable,
_ if ast_util::method_vis(&**m) == ast::Public => return Allowable,
_ => ast_util::method_vis(&**m)
_ if m.pe_vis() == ast::Public => return Allowable,
_ => m.pe_vis()
}
}
Some(ast_map::NodeTraitMethod(_)) => {
Expand Down Expand Up @@ -1079,7 +1078,7 @@ impl<'a> SanePrivacyVisitor<'a> {
"visibility qualifiers have no effect on trait \
impls");
for m in methods.iter() {
check_inherited(m.span, ast_util::method_vis(&**m), "");
check_inherited(m.span, m.pe_vis(), "");
}
}

Expand Down Expand Up @@ -1111,7 +1110,7 @@ impl<'a> SanePrivacyVisitor<'a> {
for m in methods.iter() {
match *m {
ast::Provided(ref m) => {
check_inherited(m.span, ast_util::method_vis(&**m),
check_inherited(m.span, m.pe_vis(),
"unnecessary visibility");
}
ast::Required(ref m) => {
Expand Down Expand Up @@ -1149,7 +1148,7 @@ impl<'a> SanePrivacyVisitor<'a> {
match item.node {
ast::ItemImpl(_, _, _, ref methods) => {
for m in methods.iter() {
check_inherited(tcx, m.span, ast_util::method_vis(&**m));
check_inherited(tcx, m.span, m.pe_vis());
}
}
ast::ItemForeignMod(ref fm) => {
Expand All @@ -1175,7 +1174,7 @@ impl<'a> SanePrivacyVisitor<'a> {
match *m {
ast::Required(..) => {}
ast::Provided(ref m) => check_inherited(tcx, m.span,
ast_util::method_vis(&**m)),
m.pe_vis()),
}
}
}
Expand Down Expand Up @@ -1345,7 +1344,7 @@ impl<'a> Visitor<()> for VisiblePrivateTypesVisitor<'a> {
// methods will be visible as `Public::foo`.
let mut found_pub_static = false;
for method in methods.iter() {
if ast_util::method_explicit_self(&**method).node == ast::SelfStatic &&
if method.pe_explicit_self().node == ast::SelfStatic &&
self.exported_items.contains(&method.id) {
found_pub_static = true;
visit::walk_method_helper(self, &**method, ());
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use std::collections::HashSet;
use syntax::abi;
use syntax::ast;
use syntax::ast_map;
use syntax::ast_util::is_local;
use syntax::ast_util::{is_local, PostExpansionMethod};
use syntax::ast_util;
use syntax::attr::{InlineAlways, InlineHint, InlineNever, InlineNone};
use syntax::attr;
Expand Down Expand Up @@ -68,7 +68,7 @@ fn item_might_be_inlined(item: &ast::Item) -> bool {
fn method_might_be_inlined(tcx: &ty::ctxt, method: &ast::Method,
impl_src: ast::DefId) -> bool {
if attributes_specify_inlining(method.attrs.as_slice()) ||
generics_require_inlining(ast_util::method_generics(&*method)) {
generics_require_inlining(method.pe_generics()) {
return true
}
if is_local(impl_src) {
Expand Down Expand Up @@ -200,7 +200,7 @@ impl<'a> ReachableContext<'a> {
}
}
Some(ast_map::NodeMethod(method)) => {
if generics_require_inlining(ast_util::method_generics(&*method)) ||
if generics_require_inlining(method.pe_generics()) ||
attributes_specify_inlining(method.attrs.as_slice()) {
true
} else {
Expand Down Expand Up @@ -316,14 +316,14 @@ impl<'a> ReachableContext<'a> {
// Keep going, nothing to get exported
}
ast::Provided(ref method) => {
visit::walk_block(self, ast_util::method_body(&**method), ())
visit::walk_block(self, method.pe_body(), ())
}
}
}
ast_map::NodeMethod(method) => {
let did = self.tcx.map.get_parent_did(search_item);
if method_might_be_inlined(self.tcx, &*method, did) {
visit::walk_block(self, ast_util::method_body(&*method), ())
visit::walk_block(self, method.pe_body(), ())
}
}
// Nothing to recurse on for these
Expand Down
20 changes: 9 additions & 11 deletions src/librustc/middle/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ use util::nodemap::{NodeMap, DefIdSet, FnvHashMap};

use syntax::ast::*;
use syntax::ast;
use syntax::ast_util;
use syntax::ast_util::{local_def};
use syntax::ast_util::{local_def, PostExpansionMethod};
use syntax::ast_util::{walk_pat, trait_method_to_ty_method};
use syntax::ext::mtwt;
use syntax::parse::token::special_names;
Expand Down Expand Up @@ -1299,20 +1298,20 @@ impl<'a> Resolver<'a> {
// For each method...
for method in methods.iter() {
// Add the method to the module.
let ident = ast_util::method_ident(&**method);
let ident = method.pe_ident();
let method_name_bindings =
self.add_child(ident,
new_parent.clone(),
ForbidDuplicateValues,
method.span);
let def = match ast_util::method_explicit_self(&**method).node {
let def = match method.pe_explicit_self().node {
SelfStatic => {
// Static methods become
// `def_static_method`s.
DefStaticMethod(local_def(method.id),
FromImpl(local_def(
item.id)),
ast_util::method_fn_style(&**method))
method.pe_fn_style())
}
_ => {
// Non-static methods become
Expand All @@ -1321,7 +1320,7 @@ impl<'a> Resolver<'a> {
}
};

let is_public = ast_util::method_vis(&**method) == ast::Public;
let is_public = method.pe_vis() == ast::Public;
method_name_bindings.define_value(def,
method.span,
is_public);
Expand Down Expand Up @@ -4004,15 +4003,14 @@ impl<'a> Resolver<'a> {
fn resolve_method(&mut self,
rib_kind: RibKind,
method: &Method) {
let method_generics = ast_util::method_generics(method);
let method_generics = method.pe_generics();
let type_parameters = HasTypeParameters(method_generics,
FnSpace,
method.id,
rib_kind);

self.resolve_function(rib_kind, Some(ast_util::method_fn_decl(method)),
type_parameters,
ast_util::method_body(method));
self.resolve_function(rib_kind, Some(method.pe_fn_decl()), type_parameters,
method.pe_body());
}

fn with_current_self_type<T>(&mut self, self_type: &Ty, f: |&mut Resolver| -> T) -> T {
Expand Down Expand Up @@ -4083,7 +4081,7 @@ impl<'a> Resolver<'a> {
fn check_trait_method(&self, method: &Method) {
// If there is a TraitRef in scope for an impl, then the method must be in the trait.
for &(did, ref trait_ref) in self.current_trait_ref.iter() {
let method_name = ast_util::method_ident(method).name;
let method_name = method.pe_ident().name;

if self.method_map.borrow().find(&(method_name, did)).is_none() {
let path_str = self.path_idents_to_string(&trait_ref.path);
Expand Down
14 changes: 7 additions & 7 deletions src/librustc/middle/save/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use std::os;

use syntax::ast;
use syntax::ast_util;
use syntax::ast_util::PostExpansionMethod;
use syntax::ast::{NodeId,DefId};
use syntax::ast_map::NodeItem;
use syntax::attr;
Expand Down Expand Up @@ -333,7 +334,7 @@ impl <'l> DxrVisitor<'l> {
},
};

qualname.push_str(get_ident(ast_util::method_ident(&*method)).get());
qualname.push_str(get_ident(method.pe_ident()).get());
let qualname = qualname.as_slice();

// record the decl for this def (if it has one)
Expand All @@ -349,18 +350,17 @@ impl <'l> DxrVisitor<'l> {
decl_id,
scope_id);

let m_decl = ast_util::method_fn_decl(&*method);
self.process_formals(&m_decl.inputs, qualname, e);
self.process_formals(&method.pe_fn_decl().inputs, qualname, e);

// walk arg and return types
for arg in m_decl.inputs.iter() {
for arg in method.pe_fn_decl().inputs.iter() {
self.visit_ty(&*arg.ty, e);
}
self.visit_ty(m_decl.output, e);
self.visit_ty(method.pe_fn_decl().output, e);
// walk the fn body
self.visit_block(ast_util::method_body(&*method), DxrVisitorEnv::new_nested(method.id));
self.visit_block(method.pe_body(), DxrVisitorEnv::new_nested(method.id));

self.process_generic_params(ast_util::method_generics(&*method),
self.process_generic_params(method.pe_generics(),
method.span,
qualname,
method.id,
Expand Down
17 changes: 9 additions & 8 deletions src/librustc/middle/trans/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ use std::rc::{Rc, Weak};
use syntax::util::interner::Interner;
use syntax::codemap::{Span, Pos};
use syntax::{abi, ast, codemap, ast_util, ast_map};
use syntax::ast_util::PostExpansionMethod;
use syntax::owned_slice::OwnedSlice;
use syntax::parse::token;
use syntax::parse::token::special_idents;
Expand Down Expand Up @@ -1138,10 +1139,10 @@ pub fn create_function_debug_context(cx: &CrateContext,
}
}
ast_map::NodeMethod(ref method) => {
(ast_util::method_ident(&**method),
ast_util::method_fn_decl(&**method),
ast_util::method_generics(&**method),
ast_util::method_body(&**method),
(method.pe_ident(),
method.pe_fn_decl(),
method.pe_generics(),
method.pe_body(),
method.span,
true)
}
Expand All @@ -1167,10 +1168,10 @@ pub fn create_function_debug_context(cx: &CrateContext,
ast_map::NodeTraitMethod(ref trait_method) => {
match **trait_method {
ast::Provided(ref method) => {
(ast_util::method_ident(&**method),
ast_util::method_fn_decl(&**method),
ast_util::method_generics(&**method),
ast_util::method_body(&**method),
(method.pe_ident(),
method.pe_fn_decl(),
method.pe_generics(),
method.pe_body(),
method.span,
true)
}
Expand Down
7 changes: 3 additions & 4 deletions src/librustc/middle/trans/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use middle::trans::common::*;
use middle::ty;

use syntax::ast;
use syntax::ast_util::local_def;
use syntax::ast_util::{local_def, PostExpansionMethod};
use syntax::ast_util;

pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: ast::DefId)
Expand Down Expand Up @@ -128,12 +128,11 @@ pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: ast::DefId)
let impl_tpt = ty::lookup_item_type(ccx.tcx(), impl_did);
let unparameterized =
impl_tpt.generics.types.is_empty() &&
ast_util::method_generics(&*mth).ty_params.is_empty();
mth.pe_generics().ty_params.is_empty();

if unparameterized {
let llfn = get_item_val(ccx, mth.id);
trans_fn(ccx, ast_util::method_fn_decl(&*mth),
ast_util::method_body(&*mth), llfn,
trans_fn(ccx, &*mth.pe_fn_decl(), &*mth.pe_body(), llfn,
&param_substs::empty(), mth.id, []);
}
local_def(mth.id)
Expand Down
Loading

0 comments on commit efbbb51

Please sign in to comment.