Skip to content

Commit

Permalink
Updated maybe_owned_vec with review feedback.
Browse files Browse the repository at this point in the history
Namely:

 * Added conversion traits both to and from the various vector types,
   analogous to how `str::MaybeOwned` works with `str::IntoMaybeOwned`
   and `str::Str`.  This led me to add the `FixedLen` variant of
   `MaybeOwnedVector` for interoperability with `std::slice`.

 * Revised client example code to make use of `into_maybe_owned`

 * Added implementations of `Show` and `CloneableVector` for
   `MaybeOwnedVector`.

 * As suggested by kballard, added `into_vec` method that is analogous
   to `CloneableVector::into_owned` except it produces a `Vec` rather
   than a `~[T]`.
  • Loading branch information
pnkfelix committed May 2, 2014
1 parent b737418 commit 4a122a3
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 26 deletions.
27 changes: 18 additions & 9 deletions src/libgraphviz/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

/*! Generate files suitable for use with [Graphviz](http://www.graphviz.org/)
The `render` function generates output (e.g. a `output.dot` file) for
The `render` function generates output (e.g. an `output.dot` file) for
use with [Graphviz](http://www.graphviz.org/) by walking a labelled
graph. (Graphviz can then automatically lay out the nodes and edges
of the graph, and also optionally render the graph as an image or
Expand All @@ -37,16 +37,18 @@ pairs of ints, representing the edges (the node set is implicit).
Each node label is derived directly from the int representing the node,
while the edge labels are all empty strings.
This example also illustrates how to use the `Borrowed` variant of
`MaybeOwnedVector` to return a slice into the edge list, rather than
constructing a copy from scratch.
This example also illustrates how to use `MaybeOwnedVector` to return
an owned vector or a borrowed slice as appropriate: we construct the
node vector from scratch, but borrow the edge list (rather than
constructing a copy of all the edges from scratch).
The output from this example renders five nodes, with the first four
forming a diamond-shaped acyclic graph and then pointing to the fifth
which is cyclic.
```rust
use dot = graphviz;
use graphviz::maybe_owned_vec::IntoMaybeOwnedVector;
type Nd = int;
type Ed = (int,int);
Expand Down Expand Up @@ -77,12 +79,12 @@ impl<'a> dot::GraphWalk<'a, Nd, Ed> for Edges {
}
nodes.sort();
nodes.dedup();
nodes.move_iter().collect()
nodes.into_maybe_owned()
}
fn edges(&'a self) -> dot::Edges<'a,Ed> {
let &Edges(ref edges) = self;
dot::maybe_owned_vec::Borrowed(edges.as_slice())
edges.as_slice().into_maybe_owned()
}
fn source(&self, e: &Ed) -> Nd { let &(s,_) = e; s }
Expand Down Expand Up @@ -119,9 +121,16 @@ This example also illustrates how to use a type (in this case the edge
type) that shares substructure with the graph: the edge type here is a
direct reference to the `(source,target)` pair stored in the graph's
internal vector (rather than passing around a copy of the pair
itself). Note that in this case, this implies that `fn edges(&'a
self)` must construct a fresh `Vec<&'a (uint,uint)>` from the
`Vec<(uint,uint)>` edges stored in `self`.
itself). Note that this implies that `fn edges(&'a self)` must
construct a fresh `Vec<&'a (uint,uint)>` from the `Vec<(uint,uint)>`
edges stored in `self`.
Since both the set of nodes and the set of edges are always
constructed from scratch via iterators, we use the `collect()` method
from the `Iterator` trait to collect the nodes and edges into freshly
constructed growable `Vec` values (rather use the `into_maybe_owned`
from the `IntoMaybeOwnedVector` trait as was used in the first example
above).
The output from this example renders four nodes that make up the
Hasse-diagram for the subsets of the set `{x, y}`. Each edge is
Expand Down
95 changes: 78 additions & 17 deletions src/libgraphviz/maybe_owned_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,44 +8,69 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::container::Container;
use std::fmt;
use std::iter::FromIterator;
use std::slice;

// Note: Once Dynamically Sized Types (DST) lands, this should be
// replaced with something like `enum Owned<'a, Sized? U>{ Owned(~U),
// Borrowed(&'a U) }`; and then `U` could be instantiated with `[T]`
// or `str`, etc.
// Note 1: It is not clear whether the flexibility of providing both
// the `Growable` and `FixedLen` variants is sufficiently useful.
// Consider restricting to just a two variant enum.

/// MaybeOwnedVector<'a,T> abstracts over `Vec<T>` and `&'a [T]`.
// Note 2: Once Dynamically Sized Types (DST) lands, it might be
// reasonable to replace this with something like `enum MaybeOwned<'a,
// Sized? U>{ Owned(~U), Borrowed(&'a U) }`; and then `U` could be
// instantiated with `[T]` or `str`, etc. Of course, that would imply
// removing the `Growable` variant, which relates to note 1 above.
// Alternatively, we might add `MaybeOwned` for the general case but
// keep some form of `MaybeOwnedVector` to avoid unnecessary copying
// of the contents of `Vec<T>`, since we anticipate that to be a
// frequent way to dynamically construct a vector.

/// MaybeOwnedVector<'a,T> abstracts over `Vec<T>`, `~[T]`, `&'a [T]`.
///
/// Some clients will have a pre-allocated vector ready to hand off in
/// a slice; others will want to create the set on the fly and hand
/// off ownership.
#[deriving(Eq)]
/// off ownership, via either `Growable` or `FixedLen` depending on
/// which kind of vector they have constucted. (The `FixedLen`
/// variant is provided for interoperability with `std::slice` methods
/// that return `~[T]`.)
pub enum MaybeOwnedVector<'a,T> {
Growable(Vec<T>),
FixedLen(~[T]),
Borrowed(&'a [T]),
}

/// Trait for moving into a `MaybeOwnedVector`
pub trait IntoMaybeOwnedVector<'a,T> {
/// Moves self into a `MaybeOwnedVector`
fn into_maybe_owned(self) -> MaybeOwnedVector<'a,T>;
}

impl<'a,T> IntoMaybeOwnedVector<'a,T> for Vec<T> {
#[inline]
fn into_maybe_owned(self) -> MaybeOwnedVector<'a,T> { Growable(self) }
}

impl<'a,T> IntoMaybeOwnedVector<'a,T> for ~[T] {
#[inline]
fn into_maybe_owned(self) -> MaybeOwnedVector<'a,T> { FixedLen(self) }
}

impl<'a,T> IntoMaybeOwnedVector<'a,T> for &'a [T] {
#[inline]
fn into_maybe_owned(self) -> MaybeOwnedVector<'a,T> { Borrowed(self) }
}

impl<'a,T> MaybeOwnedVector<'a,T> {
pub fn iter(&'a self) -> slice::Items<'a,T> {
match self {
&Growable(ref v) => v.iter(),
&FixedLen(ref v) => v.iter(),
&Borrowed(ref v) => v.iter(),
}
}
}

impl<'a,T> Container for MaybeOwnedVector<'a,T> {
fn len(&self) -> uint {
match self {
&Growable(ref v) => v.len(),
&Borrowed(ref v) => v.len(),
}
}
}

// The `Vector` trait is provided in the prelude and is implemented on
// both `&'a [T]` and `Vec<T>`, so it makes sense to try to support it
// seamlessly. The other vector related traits from the prelude do
Expand All @@ -59,13 +84,49 @@ impl<'b,T> slice::Vector<T> for MaybeOwnedVector<'b,T> {
fn as_slice<'a>(&'a self) -> &'a [T] {
match self {
&Growable(ref v) => v.as_slice(),
&FixedLen(ref v) => v.as_slice(),
&Borrowed(ref v) => v.as_slice(),
}
}
}

impl<'a,T> FromIterator<T> for MaybeOwnedVector<'a,T> {
fn from_iter<I:Iterator<T>>(iterator: I) -> MaybeOwnedVector<T> {
// If we are building from scratch, might as well build the
// most flexible variant.
Growable(FromIterator::from_iter(iterator))
}
}

impl<'a,T:fmt::Show> fmt::Show for MaybeOwnedVector<'a,T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.as_slice().fmt(f)
}
}

impl<'a,T:Clone> CloneableVector<T> for MaybeOwnedVector<'a,T> {
/// Returns a copy of `self`.
fn to_owned(&self) -> ~[T] {
self.as_slice().to_owned()
}

/// Convert `self` into an owned slice, not making a copy if possible.
fn into_owned(self) -> ~[T] {
match self {
Growable(v) => v.as_slice().to_owned(),
FixedLen(v) => v,
Borrowed(v) => v.to_owned(),
}
}
}

impl<'a,T:Clone> MaybeOwnedVector<'a,T> {
/// Convert `self` into a growable `Vec`, not making a copy if possible.
pub fn into_vec(self) -> Vec<T> {
match self {
Growable(v) => v,
FixedLen(v) => Vec::from_slice(v.as_slice()),
Borrowed(v) => Vec::from_slice(v),
}
}
}

8 comments on commit 4a122a3

@bors
Copy link
Contributor

@bors bors commented on 4a122a3 May 2, 2014

Choose a reason for hiding this comment

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

saw approval from alexcrichton
at pnkfelix@4a122a3

@bors
Copy link
Contributor

@bors bors commented on 4a122a3 May 2, 2014

Choose a reason for hiding this comment

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

merging pnkfelix/rust/add-libgraphviz-crate = 4a122a3 into auto

@bors
Copy link
Contributor

@bors bors commented on 4a122a3 May 2, 2014

Choose a reason for hiding this comment

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

pnkfelix/rust/add-libgraphviz-crate = 4a122a3 merged ok, testing candidate = 8600441a

@bors
Copy link
Contributor

@bors bors commented on 4a122a3 May 2, 2014

Choose a reason for hiding this comment

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

saw approval from alexcrichton
at pnkfelix@4a122a3

@bors
Copy link
Contributor

@bors bors commented on 4a122a3 May 2, 2014

Choose a reason for hiding this comment

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

merging pnkfelix/rust/add-libgraphviz-crate = 4a122a3 into auto

@bors
Copy link
Contributor

@bors bors commented on 4a122a3 May 2, 2014

Choose a reason for hiding this comment

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

pnkfelix/rust/add-libgraphviz-crate = 4a122a3 merged ok, testing candidate = 485998d4

Please sign in to comment.