Merge pull request #100 from farcaller/cleanups

Cleanups and documentation fixes

Reviewed-by: bharrisau

Author: hacknbot

Rakefile

@@ -3,9 +3,7 @@ load 'support/rake.rb'
 TOOLCHAIN = 'arm-none-eabi-'
 RUSTC = 'rustc'
 
-features = [:tft_lcd]
-
-Context.create(__FILE__, ENV['PLATFORM'], features)
+Context.create(__FILE__, ENV['PLATFORM'])
 
 provide_stdlibs
 
@@ -34,7 +32,7 @@ compile_rust :zinc_crate, {
   deps:    :core_crate,
   produce: 'main.rs'.in_source.as_rlib.in_build,
   out_dir: true,
-  recompile_on: [:triple, :platform, :features],
+  recompile_on: [:triple, :platform],
 }
 
 # zinc runtime support lib
@@ -51,18 +49,16 @@ compile_rust :zinc_isr, {
   source:  'hal/isr.rs'.in_source,
   deps:    :core_crate,
   produce: 'isr.o'.in_intermediate,
-  recompile_on: [:triple, :features],
+  recompile_on: [:triple],
 }
 
 # zinc scheduler assembly
-# TODO(farcaller): make platform-specific
-if features.include?(:multitasking)
-  compile_c :zinc_isr_sched, {
-    source:  'hal/cortex_m3/sched.S'.in_source,
-    produce: 'isr_sched.o'.in_intermediate,
-    recompile_on: [:triple, :features],
-  }
-end
+# TODO(farcaller): broken until implemented in PT.
+# compile_c :zinc_isr_sched, {
+#   source:  'hal/cortex_m3/sched.S'.in_source,
+#   produce: 'isr_sched.o'.in_intermediate,
+#   recompile_on: [:triple],
+# }
 
 # platform tree
 compile_rust :platformtree_crate, {
@@ -82,7 +78,7 @@ rust_tests :zinc_test, {
   source:  'main.rs'.in_source,
   deps:    [:core_crate],
   produce: 'zinc_test'.in_build,
-  recompile_on: [:platform, :features],
+  recompile_on: [:platform],
   build_for: :host,
 }
 
@@ -95,6 +91,7 @@ compile_rust :macro_platformtree, {
   build_for: :host,
 }
 
+desc "Build API documentation"
 task build_docs: [:build_docs_html]
 
 task build_docs_html: [] do |t|
@@ -112,13 +109,14 @@ app_tasks = Context.instance.applications.map do |a|
       :macro_platformtree,
     ],
     produce: "app_#{a}.o".in_intermediate(a),
-    recompile_on: [:triple, :platform, :features],
+    recompile_on: [:triple, :platform],
   }
 
   link_binary "app_#{a}_elf".to_sym, {
     script: 'layout.ld'.in_platform,
-    deps: ["app_#{a}".to_sym, :zinc_isr, :zinc_support] +
-          (features.include?(:multitasking) ? [:zinc_isr_sched] : []),
+    deps: ["app_#{a}".to_sym, :zinc_isr, :zinc_support],
+    # TODO(farcaller): broken until implemented in PT.
+    # (features.include?(:multitasking) ? [:zinc_isr_sched] : []),
     produce: "app_#{a}.elf".in_build,
   }
 

apps/app_mbed_lcd.rs renamed to apps/old_app_mbed_lcd.rs

@@ -54,6 +54,7 @@ pub struct Attribute {
 }
 
 impl Attribute {
+  /// Creates a new attribute with given span.
   pub fn new(value: AttributeValue, key_span: Span, value_span: Span)
       -> Attribute {
     Attribute {
@@ -63,6 +64,7 @@ impl Attribute {
     }
   }
 
+  /// Creates a new attribute with DUMMY_SP for key and value.
   pub fn new_nosp(value: AttributeValue) -> Attribute {
     Attribute {
       value: value,
@@ -72,8 +74,14 @@ impl Attribute {
   }
 }
 
+/// Node builder is a function that generates code based on the node content or
+/// mutates other nodes.
 pub type NodeBuilderFn = fn(&mut Builder, &mut ExtCtxt, Rc<Node>);
 
+/// Subnodes is effectively an ordered map.
+///
+/// We still address nodes by path for most of use cases, but we need to know
+/// the original order of appearance (it makes things deterministic).
 pub struct Subnodes {
   by_index: Vec<Rc<Node>>,
   by_path: HashMap<String, Weak<Node>>,
@@ -87,20 +95,28 @@ impl Subnodes {
     }
   }
 
+  /// Adds a node to subnodes.
+  ///
+  /// The node must not be present in the subnodes.
   pub fn push(&mut self, node: Rc<Node>) {
     let weak = node.downgrade();
     self.by_path.insert(node.path.clone(), weak);
     self.by_index.push(node);
   }
 
+  /// Returns a vector representation of subnodes.
   pub fn as_vec<'a>(&'a self) -> &'a Vec<Rc<Node>> {
     &self.by_index
   }
 
+  /// Returns a map representation of subnodes.
   pub fn as_map<'a>(&'a self) -> &'a HashMap<String, Weak<Node>> {
     &self.by_path
   }
 
+  /// A helper method to move data from other subnodes into current instance.
+  ///
+  /// Used as a helper for wrapping in RefCell.
   pub fn clone_from(&mut self, other: Subnodes) {
     self.by_index = other.by_index;
     self.by_path = other.by_path;
@@ -113,31 +129,52 @@ impl Subnodes {
 /// path_span. Attributes are stored by name, subnodes are stored by path.
 /// Type_name, if present, must specify the type path for the node's
 /// materialized object.
+///
+/// Two nodes are equal if their full paths are equal.
 pub struct Node {
+  /// Node name, might be optional.
   pub name: Option<String>,
+
+  /// Name span if name is present or path span otherwise.
   pub name_span: Span,
 
+  /// Node path, which is unique among all sibling nodes.
   pub path: String,
+
+  /// Path span.
   pub path_span: Span,
 
+  /// A map of node's attributes.
   pub attributes: RefCell<HashMap<String, Rc<Attribute>>>,
-  subnodes: RefCell<Subnodes>,
-  pub parent: Option<Weak<Node>>,
 
-  type_name: RefCell<Option<String>>,
-  type_params: RefCell<Vec<String>>,
+  /// A weak reference to parent node, None for root nodes.
+  pub parent: Option<Weak<Node>>,
 
-  /// A function that materializes this node.
+  /// A function that materializes this node, i.e. generates some actionable
+  /// code.
+  ///
+  /// Materializers are exectuted in order of dependencies resolution, so having
+  /// a fully built tree of dependencies is a must.
   pub materializer: Cell<Option<NodeBuilderFn>>,
 
   /// Present iff this node will modify state of any other nodes.
+  ///
+  /// Mutators are executed before materializers in no specific order.
   pub mutator: Cell<Option<NodeBuilderFn>>,
 
   /// List of nodes that must be materialized before this node.
+  ///
+  /// Generally, a node must depend on something to be materialized. The root
+  /// node that all other nodes must depend on implicitly or explicitly is
+  /// mcu::clock, which must always be present in PT.
   pub depends_on: RefCell<Vec<Weak<Node>>>,
 
   /// List of nodes that may be materialized before this node.
   pub rev_depends_on: RefCell<Vec<Weak<Node>>>,
+
+  subnodes: RefCell<Subnodes>,
+  type_name: RefCell<Option<String>>,
+  type_params: RefCell<Vec<String>>,
 }
 
 impl Node {
@@ -160,41 +197,60 @@ impl Node {
     }
   }
 
+  /// Set type name for the generated struct.
+  ///
+  /// If this node generates an object in main(), type_name references the type
+  /// of that object, e.g. for DHT22 driver that would be
+  /// `zinc::drivers::dht22::DHT22`.
   pub fn set_type_name(&self, tn: String) {
     let mut borrow = self.type_name.borrow_mut();
     borrow.deref_mut().clone_from(&Some(tn));
   }
 
+  /// Get type name for the generated struct.
   pub fn type_name(&self) -> Option<String> {
     self.type_name.borrow().clone()
   }
 
+  /// Get type parameters for the generated object.
   pub fn type_params(&self) -> Vec<String> {
     self.type_params.borrow().clone()
   }
 
+  /// Set type parameters for the generated object, including lifetimes.
+  ///
+  /// A default lifetime if this is going to end as a task argument is 'a. Other
+  /// lifetimes or fully-qualified types may be used as well. DHT22 driver uses
+  /// this to provide `zinc::hal::timer::Timer` and `zinc::hal::pin::GPIO` for
+  /// its public struct of `pub struct DHT22<'a, T, P>`.
   pub fn set_type_params(&self, params: Vec<String>) {
     let mut borrow = self.type_params.borrow_mut();
     borrow.deref_mut().clone_from(&params);
   }
 
+  /// Returns a cloned vec of node's subnodes.
   pub fn subnodes(&self) -> Vec<Rc<Node>> {
     self.subnodes.borrow().as_vec().clone()
   }
 
+  /// Invokes the closure for each node from node's subnodes passing a path and
+  /// weak node reference.
   pub fn with_subnodes_map(&self, f: |&HashMap<String, Weak<Node>>|) {
     let borrow = self.subnodes.borrow();
     f(borrow.as_map());
   }
 
+  /// Sets the node's subnodes from a passed object.
   pub fn set_subnodes(&self, new: Subnodes) {
     self.subnodes.borrow_mut().clone_from(new);
   }
 
+  /// Returns a clones string of current node's path.
   pub fn path(&self) -> String {
     self.path.clone()
   }
 
+  /// Returns a fully-qualified path of the current node.
   pub fn full_path(&self) -> String {
     let pp = match self.parent {
       Some(ref parent) => parent.clone().upgrade().unwrap().full_path() + "::",
@@ -372,6 +428,9 @@ impl fmt::Show for Node {
 ///
 /// Root nodes are stored by path in `nodes`, All the nmaed nodes are also
 /// stored by name in `named`.
+///
+/// TODO(farcaller): this could be really refactored into transient root node
+/// object that can depend on mcu::clock.
 pub struct PlatformTree {
   nodes: HashMap<String, Rc<Node>>,
   named: HashMap<String, Weak<Node>>,

apps/app_sched.rs renamed to apps/old_app_sched.rs

@@ -15,6 +15,7 @@
 
 //! Platform tree operations crate
 
+#![experimental]
 #![feature(quote)]
 #![crate_name="platformtree"]
 #![crate_type="rlib"]

apps/app_systick.rs renamed to apps/old_app_systick.rs

@@ -19,7 +19,8 @@ use syntax::ast;
 use syntax::codemap::MacroBang;
 use syntax::codemap::{CodeMap, Span, mk_sp, BytePos, ExpnInfo, NameAndSpan};
 use syntax::codemap;
-use syntax::diagnostic::{Emitter, RenderSpan, Level, mk_span_handler, mk_handler};
+use syntax::diagnostic::{Emitter, RenderSpan, Level, mk_span_handler};
+use syntax::diagnostic::mk_handler;
 use syntax::ext::base::ExtCtxt;
 use syntax::ext::expand::ExpansionConfig;
 use syntax::ext::quote::rt::ExtParseUtils;
@@ -30,55 +31,28 @@ use builder::Builder;
 use node;
 use parser::Parser;
 
-use hamcrest::{success,Matcher,MatchResult,SelfDescribing};
-use std::fmt::Show;
-
-pub struct EqualToString<T> {
-  expected: T
-}
-
-impl<T: Show> SelfDescribing for EqualToString<T> {
-  fn describe(&self) -> String {
-    format!("{}", self.expected)
-  }
-}
-
-impl<T : PartialEq+Show> Matcher<T> for EqualToString<T> {
-  fn matches(&self, actual: T) -> MatchResult {
-    if self.expected.eq(&actual) {
-      success()
-    }
-    else {
-      Err(format!("was {}", actual))
-    }
-  }
-}
-
-pub fn equal_to<T : PartialEq+Show>(expected: T) -> Box<EqualToString<T>> {
-  box EqualToString { expected: expected }
-}
-
-pub fn equal_to_s(expected: &str) -> Box<EqualToString<String>> {
-  equal_to(expected.to_string())
-}
-
+/// Tests if the source fails to be parsed by PT parser.
 pub fn fails_to_parse(src: &str) {
   with_parsed_tts(src, |_, failed, pt| {
     assert!(unsafe{*failed} == true);
     assert!(pt.is_none());
   });
 }
 
+/// Tests if the source fails to be built by PT builder.
 pub fn fails_to_build(src: &str) {
   with_parsed(src, |cx, failed, pt| {
+    assert!(unsafe{*failed} == false);
     Builder::build(cx, pt);
-    assert!(unsafe{*failed} == true);
   });
 }
 
-pub fn with_parsed(src: &str, block: |&mut ExtCtxt, *mut bool, Rc<node::PlatformTree>|) {
+/// Yields an ExtCtxt, parser error state and parsed PT.
+///
+/// TODO(farcaller): get rid of that bool, it's broken.
+pub fn with_parsed(src: &str,
+    block: |&mut ExtCtxt, *mut bool, Rc<node::PlatformTree>|) {
   with_parsed_tts(src, |cx, failed, pt| {
-    assert!(unsafe{*failed} == false);
     block(cx, failed, pt.unwrap());
   });
 }

platformtree/node.rs

@@ -71,8 +71,8 @@ fn build_dht22(builder: &mut Builder, cx: &mut ExtCtxt, node: Rc<node::Node>) {
 #[cfg(test)]
 mod test {
   use builder::Builder;
-  use test_helpers::{assert_equal_source, with_parsed, equal_to, equal_to_s};
-  use hamcrest::{assert_that,is};
+  use test_helpers::{assert_equal_source, with_parsed};
+  use hamcrest::{assert_that, is, equal_to};
 
   #[test]
   fn builds_lpc17xx_pt() {
@@ -96,7 +96,7 @@ mod test {
 
       let pin_node = pt.get_by_name("pin").unwrap();
       assert_that(pin_node.get_string_attr("direction").unwrap(),
-          is(equal_to_s("out")));
+          is(equal_to("out".to_string())));
     });
   }
 }