Skip to content

Add some impls of Show (RefCell, Weak, some resolve types) #19388

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

Merged
merged 2 commits into from
Jan 1, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions src/liballoc/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,13 @@ impl<T> Clone for Weak<T> {
}
}

#[experimental = "Show is experimental."]
impl<T: fmt::Show> fmt::Show for Weak<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this instance is a good idea, because Weak pointers can form a cycle with Rc pointers, making fmt go into an infinite loop.

Copy link
Contributor

Choose a reason for hiding this comment

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

In particular, while the RFC does state that Weak should have an implementation for Show, it might be better to just print something like (still valid Weak). As the RFC says, "implementations of the Show trait are expected to never panic! and always
produce valid UTF-8 data," and an infinite loop does not satisfy this.

An alternative to this would be using some global variable (thread_local is in std, unfortunately) to keep track of recursion depth and printing some special message if that gets too high.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I guess that by definition, there should be another path to an object if it is held in a weak reference, so printing a place holder is OK. Since we might expect cycles in the presence of weak pointers, this seems like a better move than going down some arbitrary depth of stack around a cycle.

fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "(Weak)")
}
}

#[doc(hidden)]
trait RcBoxPtr<T> {
fn inner(&self) -> &RcBox<T>;
Expand Down
11 changes: 11 additions & 0 deletions src/libcore/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
use clone::Clone;
use cmp::PartialEq;
use default::Default;
use fmt;
use kinds::{Copy, Send};
use ops::{Deref, DerefMut, Drop};
use option::Option;
Expand Down Expand Up @@ -365,6 +366,16 @@ impl<T: PartialEq> PartialEq for RefCell<T> {
}
}

#[unstable]
impl<T:fmt::Show> fmt::Show for RefCell<T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.try_borrow() {
Some(val) => write!(f, "{}", val),
None => write!(f, "<borrowed RefCell>")
}
}
}

struct BorrowRef<'b> {
_borrow: &'b Cell<BorrowFlag>,
}
Expand Down
23 changes: 18 additions & 5 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ use syntax::visit::{mod, Visitor};
use std::collections::{HashMap, HashSet};
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::cell::{Cell, RefCell};
use std::fmt;
use std::mem::replace;
use std::rc::{Rc, Weak};
use std::uint;
Expand Down Expand Up @@ -178,7 +179,7 @@ impl<'a, 'v, 'tcx> Visitor<'v> for Resolver<'a, 'tcx> {
}

/// Contains data for specific types of import directives.
#[deriving(Copy)]
#[deriving(Copy,Show)]
enum ImportDirectiveSubclass {
SingleImport(Name /* target */, Name /* source */),
GlobImport
Expand Down Expand Up @@ -309,6 +310,7 @@ enum Shadowable {
}

/// One import directive.
#[deriving(Show)]
struct ImportDirective {
module_path: Vec<Name>,
subclass: ImportDirectiveSubclass,
Expand Down Expand Up @@ -338,7 +340,7 @@ impl ImportDirective {
}

/// The item that an import resolves to.
#[deriving(Clone)]
#[deriving(Clone,Show)]
struct Target {
target_module: Rc<Module>,
bindings: Rc<NameBindings>,
Expand All @@ -359,6 +361,7 @@ impl Target {
}

/// An ImportResolution represents a particular `use` directive.
#[deriving(Show)]
struct ImportResolution {
/// Whether this resolution came from a `use` or a `pub use`. Note that this
/// should *not* be used whenever resolution is being performed, this is
Expand Down Expand Up @@ -438,15 +441,15 @@ impl ImportResolution {
}

/// The link from a module up to its nearest parent node.
#[deriving(Clone)]
#[deriving(Clone,Show)]
enum ParentLink {
NoParentLink,
ModuleParentLink(Weak<Module>, Name),
BlockParentLink(Weak<Module>, NodeId)
}

/// The type of module this is.
#[deriving(Copy, PartialEq)]
#[deriving(Copy, PartialEq, Show)]
enum ModuleKind {
NormalModuleKind,
TraitModuleKind,
Expand Down Expand Up @@ -528,6 +531,15 @@ impl Module {
}
}

impl fmt::Show for Module {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}, kind: {}, {}",
self.def_id,
self.kind,
if self.is_public { "public" } else { "private" } )
}
}

bitflags! {
#[deriving(Show)]
flags DefModifiers: u8 {
Expand All @@ -537,7 +549,7 @@ bitflags! {
}

// Records a possibly-private type definition.
#[deriving(Clone)]
#[deriving(Clone,Show)]
struct TypeNsDef {
modifiers: DefModifiers, // see note in ImportResolution about how to use this
module_def: Option<Rc<Module>>,
Expand All @@ -555,6 +567,7 @@ struct ValueNsDef {

// Records the definitions (at most one for each namespace) that a name is
// bound to.
#[deriving(Show)]
struct NameBindings {
type_def: RefCell<Option<TypeNsDef>>, //< Meaning in type namespace.
value_def: RefCell<Option<ValueNsDef>>, //< Meaning in value namespace.
Expand Down