Skip to content

Commit 7bcfe2e

Browse files
committed
std: Remove RefCell::get()
It's surprising that `RefCell::get()` is implicitly doing a clone on a value. This patch removes it and replaces all users with either `.borrow()` when we can autoderef, or `.borrow().clone()` when we cannot.
1 parent bb31cb8 commit 7bcfe2e

File tree

16 files changed

+66
-68
lines changed

16 files changed

+66
-68
lines changed

src/librustc/front/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ impl<'a> fold::Folder for TestHarnessGenerator<'a> {
9090
fn fold_item(&mut self, i: @ast::Item) -> SmallVector<@ast::Item> {
9191
self.cx.path.borrow_mut().push(i.ident);
9292
debug!("current path: {}",
93-
ast_util::path_name_i(self.cx.path.get().as_slice()));
93+
ast_util::path_name_i(self.cx.path.borrow().as_slice()));
9494

9595
if is_test_fn(&self.cx, i) || is_bench_fn(&self.cx, i) {
9696
match i.node {
@@ -104,7 +104,7 @@ impl<'a> fold::Folder for TestHarnessGenerator<'a> {
104104
debug!("this is a test function");
105105
let test = Test {
106106
span: i.span,
107-
path: self.cx.path.get(),
107+
path: self.cx.path.borrow().clone(),
108108
bench: is_bench_fn(&self.cx, i),
109109
ignore: is_ignored(&self.cx, i),
110110
should_fail: should_fail(i)

src/librustc/metadata/encoder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,7 +1345,7 @@ fn encode_info_for_items(ecx: &EncodeContext,
13451345
}
13461346

13471347
ebml_w.end_tag();
1348-
return /*bad*/(*index).get();
1348+
return /*bad*/index.borrow().clone();
13491349
}
13501350

13511351

@@ -1365,7 +1365,7 @@ fn create_index<T:Clone + Hash + 'static>(
13651365

13661366
let mut buckets_frozen = Vec::new();
13671367
for bucket in buckets.iter() {
1368-
buckets_frozen.push(@/*bad*/(**bucket).get());
1368+
buckets_frozen.push(@/*bad*/bucket.borrow().clone());
13691369
}
13701370
return buckets_frozen;
13711371
}

src/librustc/middle/dead.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ fn create_and_seed_worklist(tcx: &ty::ctxt,
270270
}
271271

272272
// Seed entry point
273-
match tcx.sess.entry_fn.get() {
273+
match *tcx.sess.entry_fn.borrow() {
274274
Some((id, _)) => worklist.push(id),
275275
None => ()
276276
}

src/librustc/middle/resolve.rs

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,10 @@ struct ImportResolution {
385385
type_id: Cell<NodeId>,
386386
}
387387

388+
fn get<T: Clone>(cell: &RefCell<T>) -> T {
389+
cell.borrow().clone()
390+
}
391+
388392
impl ImportResolution {
389393
fn new(id: NodeId, is_public: bool) -> ImportResolution {
390394
ImportResolution {
@@ -400,8 +404,8 @@ impl ImportResolution {
400404
fn target_for_namespace(&self, namespace: Namespace)
401405
-> Option<Target> {
402406
match namespace {
403-
TypeNS => return self.type_target.get(),
404-
ValueNS => return self.value_target.get(),
407+
TypeNS => return self.type_target.borrow().clone(),
408+
ValueNS => return self.value_target.borrow().clone(),
405409
}
406410
}
407411

@@ -546,7 +550,8 @@ impl NameBindings {
546550
// Merges the module with the existing type def or creates a new one.
547551
let module_ = @Module::new(parent_link, def_id, kind, external,
548552
is_public);
549-
match self.type_def.get() {
553+
let type_def = self.type_def.borrow().clone();
554+
match type_def {
550555
None => {
551556
self.type_def.set(Some(TypeNsDef {
552557
is_public: is_public,
@@ -574,7 +579,8 @@ impl NameBindings {
574579
external: bool,
575580
is_public: bool,
576581
_sp: Span) {
577-
match self.type_def.get() {
582+
let type_def = self.type_def.borrow().clone();
583+
match type_def {
578584
None => {
579585
let module = @Module::new(parent_link, def_id, kind,
580586
external, is_public);
@@ -609,7 +615,8 @@ impl NameBindings {
609615
/// Records a type definition.
610616
fn define_type(&self, def: Def, sp: Span, is_public: bool) {
611617
// Merges the type with the existing type def or creates a new one.
612-
match self.type_def.get() {
618+
let type_def = self.type_def.borrow().clone();
619+
match type_def {
613620
None => {
614621
self.type_def.set(Some(TypeNsDef {
615622
module_def: None,
@@ -662,17 +669,17 @@ impl NameBindings {
662669

663670
fn defined_in_namespace(&self, namespace: Namespace) -> bool {
664671
match namespace {
665-
TypeNS => return self.type_def.get().is_some(),
666-
ValueNS => return self.value_def.get().is_some()
672+
TypeNS => return self.type_def.borrow().is_some(),
673+
ValueNS => return self.value_def.borrow().is_some()
667674
}
668675
}
669676

670677
fn defined_in_public_namespace(&self, namespace: Namespace) -> bool {
671678
match namespace {
672-
TypeNS => match self.type_def.get() {
679+
TypeNS => match *self.type_def.borrow() {
673680
Some(def) => def.is_public, None => false
674681
},
675-
ValueNS => match self.value_def.get() {
682+
ValueNS => match *self.value_def.borrow() {
676683
Some(def) => def.is_public, None => false
677684
}
678685
}
@@ -681,7 +688,7 @@ impl NameBindings {
681688
fn def_for_namespace(&self, namespace: Namespace) -> Option<Def> {
682689
match namespace {
683690
TypeNS => {
684-
match self.type_def.get() {
691+
match *self.type_def.borrow() {
685692
None => None,
686693
Some(type_def) => {
687694
match type_def.type_def {
@@ -702,7 +709,7 @@ impl NameBindings {
702709
}
703710
}
704711
ValueNS => {
705-
match self.value_def.get() {
712+
match *self.value_def.borrow() {
706713
None => None,
707714
Some(value_def) => Some(value_def.def)
708715
}
@@ -714,13 +721,13 @@ impl NameBindings {
714721
if self.defined_in_namespace(namespace) {
715722
match namespace {
716723
TypeNS => {
717-
match self.type_def.get() {
724+
match *self.type_def.borrow() {
718725
None => None,
719726
Some(type_def) => type_def.type_span
720727
}
721728
}
722729
ValueNS => {
723-
match self.value_def.get() {
730+
match *self.value_def.borrow() {
724731
None => None,
725732
Some(value_def) => value_def.value_span
726733
}
@@ -1620,7 +1627,8 @@ impl<'a> Resolver<'a> {
16201627
match def {
16211628
DefMod(def_id) | DefForeignMod(def_id) | DefStruct(def_id) |
16221629
DefTy(def_id) => {
1623-
match child_name_bindings.type_def.get() {
1630+
let type_def = child_name_bindings.type_def.borrow().clone();
1631+
match type_def {
16241632
Some(TypeNsDef { module_def: Some(module_def), .. }) => {
16251633
debug!("(building reduced graph for external crate) \
16261634
already created module");
@@ -1812,7 +1820,8 @@ impl<'a> Resolver<'a> {
18121820
// Process the static methods. First,
18131821
// create the module.
18141822
let type_module;
1815-
match child_name_bindings.type_def.get() {
1823+
let type_def = child_name_bindings.type_def.borrow().clone();
1824+
match type_def {
18161825
Some(TypeNsDef {
18171826
module_def: Some(module_def),
18181827
..
@@ -2421,7 +2430,7 @@ impl<'a> Resolver<'a> {
24212430
match type_result {
24222431
BoundResult(target_module, name_bindings) => {
24232432
debug!("(resolving single import) found type target: {:?}",
2424-
{name_bindings.type_def.get().unwrap().type_def});
2433+
{ name_bindings.type_def.borrow().clone().unwrap().type_def });
24252434
import_resolution.type_target.set(
24262435
Some(Target::new(target_module, name_bindings)));
24272436
import_resolution.type_id.set(directive.id);
@@ -2433,8 +2442,8 @@ impl<'a> Resolver<'a> {
24332442
}
24342443
}
24352444

2436-
if import_resolution.value_target.get().is_none() &&
2437-
import_resolution.type_target.get().is_none() {
2445+
if import_resolution.value_target.borrow().is_none() &&
2446+
import_resolution.type_target.borrow().is_none() {
24382447
let msg = format!("unresolved import: there is no \
24392448
`{}` in `{}`",
24402449
token::get_ident(source),
@@ -2452,7 +2461,7 @@ impl<'a> Resolver<'a> {
24522461
// record what this import resolves to for later uses in documentation,
24532462
// this may resolve to either a value or a type, but for documentation
24542463
// purposes it's good enough to just favor one over the other.
2455-
let value_private = match import_resolution.value_target.get() {
2464+
let value_private = match *import_resolution.value_target.borrow() {
24562465
Some(target) => {
24572466
let def = target.bindings.def_for_namespace(ValueNS).unwrap();
24582467
self.def_map.borrow_mut().insert(directive.id, def);
@@ -2463,7 +2472,7 @@ impl<'a> Resolver<'a> {
24632472
// _exists is false.
24642473
None => None,
24652474
};
2466-
let type_private = match import_resolution.type_target.get() {
2475+
let type_private = match *import_resolution.type_target.borrow() {
24672476
Some(target) => {
24682477
let def = target.bindings.def_for_namespace(TypeNS).unwrap();
24692478
self.def_map.borrow_mut().insert(directive.id, def);
@@ -2513,7 +2522,7 @@ impl<'a> Resolver<'a> {
25132522
for (ident, target_import_resolution) in import_resolutions.iter() {
25142523
debug!("(resolving glob import) writing module resolution \
25152524
{:?} into `{}`",
2516-
target_import_resolution.type_target.get().is_none(),
2525+
target_import_resolution.type_target.borrow().is_none(),
25172526
self.module_to_str(module_));
25182527

25192528
if !target_import_resolution.is_public.get() {
@@ -2529,9 +2538,9 @@ impl<'a> Resolver<'a> {
25292538
let new_import_resolution =
25302539
@ImportResolution::new(id, is_public);
25312540
new_import_resolution.value_target.set(
2532-
target_import_resolution.value_target.get());
2541+
get(&target_import_resolution.value_target));
25332542
new_import_resolution.type_target.set(
2534-
target_import_resolution.type_target.get());
2543+
get(&target_import_resolution.type_target));
25352544

25362545
import_resolutions.insert
25372546
(*ident, new_import_resolution);
@@ -2540,7 +2549,7 @@ impl<'a> Resolver<'a> {
25402549
// Merge the two import resolutions at a finer-grained
25412550
// level.
25422551

2543-
match target_import_resolution.value_target.get() {
2552+
match *target_import_resolution.value_target.borrow() {
25442553
None => {
25452554
// Continue.
25462555
}
@@ -2549,7 +2558,7 @@ impl<'a> Resolver<'a> {
25492558
Some(value_target));
25502559
}
25512560
}
2552-
match target_import_resolution.type_target.get() {
2561+
match *target_import_resolution.type_target.borrow() {
25532562
None => {
25542563
// Continue.
25552564
}
@@ -2692,7 +2701,7 @@ impl<'a> Resolver<'a> {
26922701
Success((target, used_proxy)) => {
26932702
// Check to see whether there are type bindings, and, if
26942703
// so, whether there is a module within.
2695-
match target.bindings.type_def.get() {
2704+
match *target.bindings.type_def.borrow() {
26962705
Some(type_def) => {
26972706
match type_def.module_def {
26982707
None => {
@@ -3004,7 +3013,7 @@ impl<'a> Resolver<'a> {
30043013
match resolve_result {
30053014
Success((target, _)) => {
30063015
let bindings = &*target.bindings;
3007-
match bindings.type_def.get() {
3016+
match *bindings.type_def.borrow() {
30083017
Some(type_def) => {
30093018
match type_def.module_def {
30103019
None => {
@@ -4526,8 +4535,8 @@ impl<'a> Resolver<'a> {
45264535
debug!("(resolve bare identifier pattern) succeeded in \
45274536
finding {} at {:?}",
45284537
token::get_ident(name),
4529-
target.bindings.value_def.get());
4530-
match target.bindings.value_def.get() {
4538+
target.bindings.value_def.borrow());
4539+
match *target.bindings.value_def.borrow() {
45314540
None => {
45324541
fail!("resolved name in the value namespace to a \
45334542
set of name bindings with no def?!");

src/librustc/middle/trans/base.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,7 @@ pub fn make_return_pointer(fcx: &FunctionContext, output_type: ty::t)
11281128
llvm::LLVMGetParam(fcx.llfn, 0)
11291129
} else {
11301130
let lloutputtype = type_of::type_of(fcx.ccx, output_type);
1131-
let bcx = fcx.entry_bcx.get().unwrap();
1131+
let bcx = fcx.entry_bcx.borrow().clone().unwrap();
11321132
Alloca(bcx, lloutputtype, "__make_return_pointer")
11331133
}
11341134
}
@@ -1399,7 +1399,7 @@ pub fn trans_closure(ccx: &CrateContext,
13991399

14001400
// Create the first basic block in the function and keep a handle on it to
14011401
// pass to finish_fn later.
1402-
let bcx_top = fcx.entry_bcx.get().unwrap();
1402+
let bcx_top = fcx.entry_bcx.borrow().clone().unwrap();
14031403
let mut bcx = bcx_top;
14041404
let block_ty = node_id_type(bcx, body.id);
14051405

@@ -1547,7 +1547,7 @@ fn trans_enum_variant_or_tuple_like_struct(ccx: &CrateContext,
15471547

15481548
let arg_datums = create_datums_for_fn_args(&fcx, arg_tys.as_slice());
15491549

1550-
let bcx = fcx.entry_bcx.get().unwrap();
1550+
let bcx = fcx.entry_bcx.borrow().clone().unwrap();
15511551

15521552
if !type_is_zero_size(fcx.ccx, result_ty) {
15531553
let repr = adt::represent_type(ccx, result_ty);
@@ -1752,7 +1752,7 @@ pub fn register_fn_llvmty(ccx: &CrateContext,
17521752
}
17531753

17541754
pub fn is_entry_fn(sess: &Session, node_id: ast::NodeId) -> bool {
1755-
match sess.entry_fn.get() {
1755+
match *sess.entry_fn.borrow() {
17561756
Some((entry_id, _)) => node_id == entry_id,
17571757
None => false
17581758
}

src/librustc/middle/trans/closure.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext,
464464
let arena = TypedArena::new();
465465
let fcx = new_fn_ctxt(ccx, llfn, -1, true, f.sig.output, None, None, &arena);
466466
init_function(&fcx, true, f.sig.output, None);
467-
let bcx = fcx.entry_bcx.get().unwrap();
467+
let bcx = fcx.entry_bcx.borrow().clone().unwrap();
468468

469469
let args = create_datums_for_fn_args(&fcx,
470470
ty::ty_fn_args(closure_ty)

src/librustc/middle/trans/glue.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ fn make_generic_glue(ccx: &CrateContext,
463463
// llfn is expected be declared to take a parameter of the appropriate
464464
// type, so we don't need to explicitly cast the function parameter.
465465

466-
let bcx = fcx.entry_bcx.get().unwrap();
466+
let bcx = fcx.entry_bcx.borrow().clone().unwrap();
467467
let llrawptr0 = unsafe { llvm::LLVMGetParam(llfn, fcx.arg_pos(0) as c_uint) };
468468
let bcx = helper(bcx, llrawptr0, t);
469469
finish_fn(&fcx, bcx);

src/librustc/middle/trans/intrinsic.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ pub fn trans_intrinsic(ccx: &CrateContext,
199199

200200
set_always_inline(fcx.llfn);
201201

202-
let mut bcx = fcx.entry_bcx.get().unwrap();
202+
let mut bcx = fcx.entry_bcx.borrow().clone().unwrap();
203203
let first_real_arg = fcx.arg_pos(0u);
204204

205205
let name = token::get_ident(item.ident);

src/librustc/middle/trans/reflect.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ impl<'a> Reflector<'a> {
300300
//
301301
llvm::LLVMGetParam(llfdecl, fcx.arg_pos(0u) as c_uint)
302302
};
303-
let bcx = fcx.entry_bcx.get().unwrap();
303+
let bcx = fcx.entry_bcx.borrow().clone().unwrap();
304304
let arg = BitCast(bcx, arg, llptrty);
305305
let ret = adt::trans_get_discr(bcx, repr, arg, Some(Type::i64(ccx)));
306306
Store(bcx, ret, fcx.llretptr.get().unwrap());

src/librustc/middle/typeck/check/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,8 +2209,8 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
22092209
fcx.write_ty(expr.id, fty);
22102210

22112211
let (inherited_purity, id) =
2212-
ty::determine_inherited_purity((fcx.ps.get().purity,
2213-
fcx.ps.get().def),
2212+
ty::determine_inherited_purity((fcx.ps.borrow().purity,
2213+
fcx.ps.borrow().def),
22142214
(purity, expr.id),
22152215
sigil);
22162216

src/librustc/middle/typeck/check/regionck.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) {
398398
expr.repr(rcx.fcx.tcx()), rcx.repeating_scope);
399399

400400
let method_call = MethodCall::expr(expr.id);
401-
let has_method_map = rcx.fcx.inh.method_map.get().contains_key(&method_call);
401+
let has_method_map = rcx.fcx.inh.method_map.borrow().contains_key(&method_call);
402402

403403
// Check any autoderefs or autorefs that appear.
404404
for &adjustment in rcx.fcx.inh.adjustments.borrow().find(&expr.id).iter() {
@@ -498,7 +498,7 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) {
498498
ast::ExprUnary(ast::UnDeref, base) => {
499499
// For *a, the lifetime of a must enclose the deref
500500
let method_call = MethodCall::expr(expr.id);
501-
let base_ty = match rcx.fcx.inh.method_map.get().find(&method_call) {
501+
let base_ty = match rcx.fcx.inh.method_map.borrow().find(&method_call) {
502502
Some(method) => {
503503
constrain_call(rcx, None, expr, Some(base), [], true);
504504
ty::ty_fn_ret(method.ty)
@@ -852,7 +852,7 @@ fn constrain_autoderefs(rcx: &mut Rcx,
852852
i, derefs);
853853

854854
let method_call = MethodCall::autoderef(deref_expr.id, i as u32);
855-
derefd_ty = match rcx.fcx.inh.method_map.get().find(&method_call) {
855+
derefd_ty = match rcx.fcx.inh.method_map.borrow().find(&method_call) {
856856
Some(method) => {
857857
// Treat overloaded autoderefs as if an AutoRef adjustment
858858
// was applied on the base type, as that is always the case.

src/librustc/middle/typeck/infer/error_reporting.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1286,6 +1286,6 @@ impl LifeGiver {
12861286
}
12871287

12881288
fn get_generated_lifetimes(&self) -> Vec<ast::Lifetime> {
1289-
self.generated.get()
1289+
self.generated.borrow().clone()
12901290
}
12911291
}

0 commit comments

Comments
 (0)