From 7bcfe2ee1067d1304c9a2813c111f10a89984e45 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Fri, 28 Mar 2014 10:29:55 -0700 Subject: 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. --- src/libstd/cell.rs | 20 ++++---------------- src/libstd/option.rs | 5 +++-- 2 files changed, 7 insertions(+), 18 deletions(-) (limited to 'src/libstd') diff --git a/src/libstd/cell.rs b/src/libstd/cell.rs index eb114e89510..ec6994728f7 100644 --- a/src/libstd/cell.rs +++ b/src/libstd/cell.rs @@ -176,21 +176,9 @@ impl RefCell { } } -impl RefCell { - /// Returns a copy of the contained value. - /// - /// # Failure - /// - /// Fails if the value is currently mutably borrowed. - #[inline] - pub fn get(&self) -> T { - (*self.borrow()).clone() - } -} - impl Clone for RefCell { fn clone(&self) -> RefCell { - RefCell::new(self.get()) + RefCell::new(self.borrow().clone()) } } @@ -216,7 +204,7 @@ impl<'b, T> Drop for Ref<'b, T> { impl<'b, T> Deref for Ref<'b, T> { #[inline] fn deref<'a>(&'a self) -> &'a T { - unsafe{ &*self.parent.value.get() } + unsafe { &*self.parent.value.get() } } } @@ -236,14 +224,14 @@ impl<'b, T> Drop for RefMut<'b, T> { impl<'b, T> Deref for RefMut<'b, T> { #[inline] fn deref<'a>(&'a self) -> &'a T { - unsafe{ &*self.parent.value.get() } + unsafe { &*self.parent.value.get() } } } impl<'b, T> DerefMut for RefMut<'b, T> { #[inline] fn deref_mut<'a>(&'a mut self) -> &'a mut T { - unsafe{ &mut *self.parent.value.get() } + unsafe { &mut *self.parent.value.get() } } } diff --git a/src/libstd/option.rs b/src/libstd/option.rs index be1c87ba788..ce2c1378fc1 100644 --- a/src/libstd/option.rs +++ b/src/libstd/option.rs @@ -651,7 +651,8 @@ mod tests { impl ::ops::Drop for R { fn drop(&mut self) { let ii = &*self.i; - ii.set(ii.get() + 1); + let i = ii.borrow().clone(); + ii.set(i + 1); } } @@ -667,7 +668,7 @@ mod tests { let opt = Some(x); let _y = opt.unwrap(); } - assert_eq!(i.get(), 1); + assert_eq!(*i.borrow(), 1); } #[test] -- cgit 1.4.1-3-g733a5 From 3961957bd61b7f9ab350c0a6621a1ad18716c332 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 2 Apr 2014 06:55:33 -0700 Subject: std: Remove `RefCell::set()` --- src/librustc/driver/driver.rs | 4 +-- src/librustc/middle/entry.rs | 6 ++-- src/librustc/middle/resolve.rs | 58 ++++++++++++++++----------------- src/librustc/middle/trans/base.rs | 2 +- src/librustc/middle/trans/common.rs | 2 +- src/librustc/middle/typeck/check/mod.rs | 2 +- src/libstd/cell.rs | 10 ------ src/libstd/option.rs | 2 +- src/test/run-pass/cycle-collection.rs | 2 +- src/test/run-pass/issue-980.rs | 4 +-- 10 files changed, 39 insertions(+), 53 deletions(-) (limited to 'src/libstd') diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index d5dba02ed28..af5b3f8b0cd 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -212,9 +212,7 @@ pub fn phase_2_configure_and_expand(sess: &Session, let time_passes = sess.time_passes(); sess.building_library.set(session::building_library(&sess.opts, &krate)); - sess.crate_types.set(session::collect_crate_types(sess, - krate.attrs - .as_slice())); + *sess.crate_types.borrow_mut() = session::collect_crate_types(sess, krate.attrs.as_slice()); time(time_passes, "gated feature checking", (), |_| front::feature_gate::check_crate(sess, &krate)); diff --git a/src/librustc/middle/entry.rs b/src/librustc/middle/entry.rs index 7adfd6e0af0..441a3a36729 100644 --- a/src/librustc/middle/entry.rs +++ b/src/librustc/middle/entry.rs @@ -123,13 +123,13 @@ fn find_item(item: &Item, ctxt: &mut EntryContext) { fn configure_main(this: &mut EntryContext) { if this.start_fn.is_some() { - this.session.entry_fn.set(this.start_fn); + *this.session.entry_fn.borrow_mut() = this.start_fn; this.session.entry_type.set(Some(session::EntryStart)); } else if this.attr_main_fn.is_some() { - this.session.entry_fn.set(this.attr_main_fn); + *this.session.entry_fn.borrow_mut() = this.attr_main_fn; this.session.entry_type.set(Some(session::EntryMain)); } else if this.main_fn.is_some() { - this.session.entry_fn.set(this.main_fn); + *this.session.entry_fn.borrow_mut() = this.main_fn; this.session.entry_type.set(Some(session::EntryMain)); } else { if !this.session.building_library.get() { diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 526ec66800a..143b02f96d2 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -553,20 +553,20 @@ impl NameBindings { let type_def = self.type_def.borrow().clone(); match type_def { None => { - self.type_def.set(Some(TypeNsDef { + *self.type_def.borrow_mut() = Some(TypeNsDef { is_public: is_public, module_def: Some(module_), type_def: None, type_span: Some(sp) - })); + }); } Some(type_def) => { - self.type_def.set(Some(TypeNsDef { + *self.type_def.borrow_mut() = Some(TypeNsDef { is_public: is_public, module_def: Some(module_), type_span: Some(sp), type_def: type_def.type_def - })); + }); } } } @@ -584,12 +584,12 @@ impl NameBindings { None => { let module = @Module::new(parent_link, def_id, kind, external, is_public); - self.type_def.set(Some(TypeNsDef { + *self.type_def.borrow_mut() = Some(TypeNsDef { is_public: is_public, module_def: Some(module), type_def: None, type_span: None, - })) + }); } Some(type_def) => { match type_def.module_def { @@ -599,12 +599,12 @@ impl NameBindings { kind, external, is_public); - self.type_def.set(Some(TypeNsDef { + *self.type_def.borrow_mut() = Some(TypeNsDef { is_public: is_public, module_def: Some(module), type_def: type_def.type_def, type_span: None, - })) + }); } Some(module_def) => module_def.kind.set(kind), } @@ -618,31 +618,31 @@ impl NameBindings { let type_def = self.type_def.borrow().clone(); match type_def { None => { - self.type_def.set(Some(TypeNsDef { + *self.type_def.borrow_mut() = Some(TypeNsDef { module_def: None, type_def: Some(def), type_span: Some(sp), is_public: is_public, - })); + }); } Some(type_def) => { - self.type_def.set(Some(TypeNsDef { + *self.type_def.borrow_mut() = Some(TypeNsDef { type_def: Some(def), type_span: Some(sp), module_def: type_def.module_def, is_public: is_public, - })); + }); } } } /// Records a value definition. fn define_value(&self, def: Def, sp: Span, is_public: bool) { - self.value_def.set(Some(ValueNsDef { + *self.value_def.borrow_mut() = Some(ValueNsDef { def: def, value_span: Some(sp), is_public: is_public, - })); + }); } /// Returns the module node if applicable. @@ -2417,8 +2417,8 @@ impl<'a> Resolver<'a> { match value_result { BoundResult(target_module, name_bindings) => { debug!("(resolving single import) found value target"); - import_resolution.value_target.set( - Some(Target::new(target_module, name_bindings))); + *import_resolution.value_target.borrow_mut() = + Some(Target::new(target_module, name_bindings)); import_resolution.value_id.set(directive.id); value_used_public = name_bindings.defined_in_public_namespace(ValueNS); } @@ -2431,8 +2431,8 @@ impl<'a> Resolver<'a> { BoundResult(target_module, name_bindings) => { debug!("(resolving single import) found type target: {:?}", { name_bindings.type_def.borrow().clone().unwrap().type_def }); - import_resolution.type_target.set( - Some(Target::new(target_module, name_bindings))); + *import_resolution.type_target.borrow_mut() = + Some(Target::new(target_module, name_bindings)); import_resolution.type_id.set(directive.id); type_used_public = name_bindings.defined_in_public_namespace(TypeNS); } @@ -2537,10 +2537,10 @@ impl<'a> Resolver<'a> { // Simple: just copy the old import resolution. let new_import_resolution = @ImportResolution::new(id, is_public); - new_import_resolution.value_target.set( - get(&target_import_resolution.value_target)); - new_import_resolution.type_target.set( - get(&target_import_resolution.type_target)); + *new_import_resolution.value_target.borrow_mut() = + get(&target_import_resolution.value_target); + *new_import_resolution.type_target.borrow_mut() = + get(&target_import_resolution.type_target); import_resolutions.insert (*ident, new_import_resolution); @@ -2554,8 +2554,7 @@ impl<'a> Resolver<'a> { // Continue. } Some(value_target) => { - dest_import_resolution.value_target.set( - Some(value_target)); + *dest_import_resolution.value_target.borrow_mut() = Some(value_target); } } match *target_import_resolution.type_target.borrow() { @@ -2563,8 +2562,7 @@ impl<'a> Resolver<'a> { // Continue. } Some(type_target) => { - dest_import_resolution.type_target.set( - Some(type_target)); + *dest_import_resolution.type_target.borrow_mut() = Some(type_target); } } dest_import_resolution.is_public.set(is_public); @@ -2636,14 +2634,14 @@ impl<'a> Resolver<'a> { // Merge the child item into the import resolution. if name_bindings.defined_in_public_namespace(ValueNS) { debug!("(resolving glob import) ... for value target"); - dest_import_resolution.value_target.set( - Some(Target::new(containing_module, name_bindings))); + *dest_import_resolution.value_target.borrow_mut() = + Some(Target::new(containing_module, name_bindings)); dest_import_resolution.value_id.set(id); } if name_bindings.defined_in_public_namespace(TypeNS) { debug!("(resolving glob import) ... for type target"); - dest_import_resolution.type_target.set( - Some(Target::new(containing_module, name_bindings))); + *dest_import_resolution.type_target.borrow_mut() = + Some(Target::new(containing_module, name_bindings)); dest_import_resolution.type_id.set(id); } dest_import_resolution.is_public.set(is_public); diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 2f4163e8296..96f75705cf8 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1209,7 +1209,7 @@ pub fn init_function<'a>( param_substs: Option<@param_substs>) { let entry_bcx = fcx.new_temp_block("entry-block"); - fcx.entry_bcx.set(Some(entry_bcx)); + *fcx.entry_bcx.borrow_mut() = Some(entry_bcx); // Use a dummy instruction as the insertion point for all allocas. // This is later removed in FunctionContext::cleanup. diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index 4833a233423..26520e98c13 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -322,7 +322,7 @@ impl<'a> FunctionContext<'a> { .unwrap()); } // Remove the cycle between fcx and bcx, so memory can be freed - self.entry_bcx.set(None); + *self.entry_bcx.borrow_mut() = None; } pub fn get_llreturn(&self) -> BasicBlockRef { diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index bbf2e60dfe3..085e387dfe9 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -3333,7 +3333,7 @@ pub fn check_block_with_expected(fcx: &FnCtxt, }; }); - fcx.ps.set(prev); + *fcx.ps.borrow_mut() = prev; } pub fn check_const(ccx: &CrateCtxt, diff --git a/src/libstd/cell.rs b/src/libstd/cell.rs index ec6994728f7..40c6c3ebccf 100644 --- a/src/libstd/cell.rs +++ b/src/libstd/cell.rs @@ -164,16 +164,6 @@ impl RefCell { None => fail!("RefCell already borrowed") } } - - /// Sets the value, replacing what was there. - /// - /// # Failure - /// - /// Fails if the value is currently borrowed. - #[inline] - pub fn set(&self, value: T) { - *self.borrow_mut() = value; - } } impl Clone for RefCell { diff --git a/src/libstd/option.rs b/src/libstd/option.rs index ce2c1378fc1..ca1ea0169e6 100644 --- a/src/libstd/option.rs +++ b/src/libstd/option.rs @@ -652,7 +652,7 @@ mod tests { fn drop(&mut self) { let ii = &*self.i; let i = ii.borrow().clone(); - ii.set(i + 1); + *ii.borrow_mut() = i + 1; } } diff --git a/src/test/run-pass/cycle-collection.rs b/src/test/run-pass/cycle-collection.rs index ca1e18eb87b..c6f353136ba 100644 --- a/src/test/run-pass/cycle-collection.rs +++ b/src/test/run-pass/cycle-collection.rs @@ -19,7 +19,7 @@ enum taggy { fn f() { let a_box = @RefCell::new(nil); - a_box.set(cons(a_box)); + *a_box.borrow_mut() = cons(a_box); } pub fn main() { diff --git a/src/test/run-pass/issue-980.rs b/src/test/run-pass/issue-980.rs index f6dc4adcf9b..06f95bae559 100644 --- a/src/test/run-pass/issue-980.rs +++ b/src/test/run-pass/issue-980.rs @@ -23,7 +23,7 @@ struct Pointy { pub fn main() { let m = @RefCell::new(Pointy { x : no_pointy }); - m.set(Pointy { + *m.borrow_mut() = Pointy { x: yes_pointy(m) - }); + }; } -- cgit 1.4.1-3-g733a5