Some light refactoring attempts, didn't get very far..

10 files changed, 102 insertions, 87 deletions

diff --git a/openscad.pro b/openscad.pro
index 50a419d..344363a 100644
--- a/openscad.pro
+++ b/openscad.pro
@@ -187,20 +187,14 @@ HEADERS += src/renderer.h \
            src/system-gl.h \
            src/stl-utils.h
 
-SOURCES += src/openscad.cc \
-           src/mainwin.cc \
+SOURCES += src/mathc99.cc \
+	   src/linalg.cc \
            src/handle_dep.cc \
-           src/renderer.cc \
-           src/rendersettings.cc \
-           src/ThrownTogetherRenderer.cc \
-           src/glview.cc \
-           src/export.cc \
            src/value.cc \
            src/expr.cc \
            src/func.cc \
            src/module.cc \
            src/node.cc \
-           src/builtin.cc \
            src/context.cc \
            src/csgterm.cc \
            src/polyset.cc \
@@ -213,28 +207,38 @@ SOURCES += src/openscad.cc \
            src/surface.cc \
            src/control.cc \
            src/render.cc \
-           src/import.cc \
            src/dxfdata.cc \
-           src/dxftess.cc \
-           src/dxftess-glu.cc \
-           src/dxftess-cgal.cc \
            src/dxfdim.cc \
            src/linearextrude.cc \
            src/rotateextrude.cc \
-           src/highlighter.cc \
            src/printutils.cc \
+           src/progress.cc \
+           \
+           src/nodedumper.cc \
+           src/traverser.cc \
+           src/PolySetEvaluator.cc \
+           src/PolySetCache.cc \
+           src/Tree.cc \
+           \
+           src/rendersettings.cc \
+           src/highlighter.cc \
            src/Preferences.cc \
            src/OpenCSGWarningDialog.cc \
-           src/progress.cc \
            src/editor.cc \
-           src/traverser.cc \
-           src/nodedumper.cc \
+           src/glview.cc \
+           \
+           src/builtin.cc \
+           src/export.cc \
+           src/import.cc \
+           src/renderer.cc \
+           src/ThrownTogetherRenderer.cc \
+           src/dxftess.cc \
+           src/dxftess-glu.cc \
+           src/dxftess-cgal.cc \
            src/CSGTermEvaluator.cc \
-           src/Tree.cc \
-	   src/mathc99.cc \
-	   src/linalg.cc \
-           src/PolySetCache.cc \
-           src/PolySetEvaluator.cc
+           \
+           src/openscad.cc \
+           src/mainwin.cc
 
 opencsg {
   HEADERS += src/OpenCSGRenderer.h
diff --git a/src/CGALEvaluator.cc b/src/CGALEvaluator.cc
index 684ab42..a6b2f06 100644
--- a/src/CGALEvaluator.cc
+++ b/src/CGALEvaluator.cc
@@ -99,7 +99,7 @@ CGAL_Nef_polyhedron CGALEvaluator::applyToChildren(const AbstractNode &node, CGA
 		const AbstractNode *chnode = item.first;
 		const CGAL_Nef_polyhedron &chN = item.second;
 		// FIXME: Don't use deep access to modinst members
-		if (chnode->modinst->tag_background) continue;
+		if (chnode->modinst->isBackground()) continue;
 
     // NB! We insert into the cache here to ensure that all children of
     // a node is a valid object. If we inserted as we created them, the 
@@ -127,7 +127,7 @@ CGAL_Nef_polyhedron CGALEvaluator::applyHull(const CgaladvNode &node)
 		const AbstractNode *chnode = item.first;
 		const CGAL_Nef_polyhedron &chN = item.second;
 		// FIXME: Don't use deep access to modinst members
-		if (chnode->modinst->tag_background) continue;
+		if (chnode->modinst->isBackground()) continue;
 		if (dim == 0) {
 			dim = chN.dim;
 		}
diff --git a/src/CSGTermEvaluator.cc b/src/CSGTermEvaluator.cc
index fc76d56..65209dd 100644
--- a/src/CSGTermEvaluator.cc
+++ b/src/CSGTermEvaluator.cc
@@ -56,10 +56,10 @@ void CSGTermEvaluator::applyToChildren(const AbstractNode &node, CSGTermEvaluato
 			}
 		}
 	}
-	if (t1 && node.modinst->tag_highlight) {
+	if (t1 && node.modinst->isHighlight()) {
 		this->highlights.push_back(t1);
 	}
-	if (t1 && node.modinst->tag_background) {
+	if (t1 && node.modinst->isBackground()) {
 		this->background.push_back(t1);
 		t1.reset(); // don't propagate background tagged nodes
 	}
@@ -94,10 +94,10 @@ static shared_ptr<CSGTerm> evaluate_csg_term_from_ps(const State &state,
 	std::stringstream stream;
 	stream << node.name() << node.index();
 	shared_ptr<CSGTerm> t(new CSGTerm(ps, state.matrix(), state.color(), stream.str()));
-	if (modinst->tag_highlight) {
+	if (modinst->isHighlight()) {
 		highlights.push_back(t);
 	}
-	if (modinst->tag_background) {
+	if (modinst->isBackground()) {
 		background.push_back(t);
 		t.reset();
 	}
diff --git a/src/PolySetCGALEvaluator.cc b/src/PolySetCGALEvaluator.cc
index 3285b46..3cd6005 100644
--- a/src/PolySetCGALEvaluator.cc
+++ b/src/PolySetCGALEvaluator.cc
@@ -26,7 +26,7 @@ PolySet *PolySetCGALEvaluator::evaluatePolySet(const ProjectionNode &node)
 	// Before projecting, union all children
 	CGAL_Nef_polyhedron sum;
 	BOOST_FOREACH (AbstractNode * v, node.getChildren()) {
-		if (v->modinst->tag_background) continue;
+		if (v->modinst->isBackground()) continue;
 		CGAL_Nef_polyhedron N = this->cgalevaluator.evaluateCGALMesh(*v);
 		if (N.dim == 3) {
 			if (sum.empty()) sum = N.copy();
@@ -259,7 +259,7 @@ PolySet *PolySetCGALEvaluator::evaluatePolySet(const LinearExtrudeNode &node)
 		// to a single DxfData, then tesselate this into a PolySet
 		CGAL_Nef_polyhedron sum;
 		BOOST_FOREACH (AbstractNode * v, node.getChildren()) {
-			if (v->modinst->tag_background) continue;
+			if (v->modinst->isBackground()) continue;
 			CGAL_Nef_polyhedron N = this->cgalevaluator.evaluateCGALMesh(*v);
 			if (N.dim != 2) {
 				PRINT("ERROR: linear_extrude() is not defined for 3D child objects!");
@@ -357,7 +357,7 @@ PolySet *PolySetCGALEvaluator::evaluatePolySet(const RotateExtrudeNode &node)
 		// to a single DxfData, then tesselate this into a PolySet
 		CGAL_Nef_polyhedron sum;
 		BOOST_FOREACH (AbstractNode * v, node.getChildren()) {
-			if (v->modinst->tag_background) continue;
+			if (v->modinst->isBackground()) continue;
 			CGAL_Nef_polyhedron N = this->cgalevaluator.evaluateCGALMesh(*v);
 			if (N.dim != 2) {
 				PRINT("ERROR: rotate_extrude() is not defined for 3D child objects!");
diff --git a/src/context.cc b/src/context.cc
index 6d0cb3a..f636b05 100644
--- a/src/context.cc
+++ b/src/context.cc
@@ -45,6 +45,7 @@ Context::Context(const Context *parent, const Module *library)
 	ctx_stack.push_back(this);
 	if (parent) document_path = parent->document_path;
 	if (library) {
+		// FIXME: Don't access module members directly
 		this->functions_p = &library->functions;
 		this->modules_p = &library->modules;
 		this->usedlibs_p = &library->usedlibs;
@@ -148,24 +149,24 @@ Value Context::evaluate_function(const std::string &name,
 
 AbstractNode *Context::evaluate_module(const ModuleInstantiation &inst) const
 {
-	if (this->modules_p && this->modules_p->find(inst.modname) != this->modules_p->end()) {
-		AbstractModule *m = this->modules_p->find(inst.modname)->second;
-		std::string replacement = Builtins::instance()->isDeprecated(inst.modname);
+	if (this->modules_p && this->modules_p->find(inst.name()) != this->modules_p->end()) {
+		AbstractModule *m = this->modules_p->find(inst.name())->second;
+		std::string replacement = Builtins::instance()->isDeprecated(inst.name());
 		if (!replacement.empty()) {
-			PRINTF("DEPRECATED: The %s() module will be removed in future releases. Use %s() instead.", inst.modname.c_str(), replacement.c_str());
+			PRINTF("DEPRECATED: The %s() module will be removed in future releases. Use %s() instead.", inst.name().c_str(), replacement.c_str());
 		}
 		return m->evaluate(this, &inst);
 	}
 	if (this->usedlibs_p) {
 		BOOST_FOREACH(const ModuleContainer::value_type &m, *this->usedlibs_p) {
-			if (m.second->modules.find(inst.modname) != m.second->modules.end()) {
+			if (m.second->modules.find(inst.name()) != m.second->modules.end()) {
 				Context ctx(this->parent, m.second);
-				return m.second->modules[inst.modname]->evaluate(&ctx, &inst);
+				return m.second->modules[inst.name()]->evaluate(&ctx, &inst);
 			}
 		}
 	}
 	if (this->parent) return this->parent->evaluate_module(inst);
-	PRINTF("WARNING: Ignoring unknown module '%s'.", inst.modname.c_str());
+	PRINTF("WARNING: Ignoring unknown module '%s'.", inst.name().c_str());
 	return NULL;
 }
 
diff --git a/src/mainwin.cc b/src/mainwin.cc
index 3243847..a169ab1 100644
--- a/src/mainwin.cc
+++ b/src/mainwin.cc
@@ -582,7 +582,7 @@ void MainWindow::load()
 AbstractNode *MainWindow::find_root_tag(AbstractNode *n)
 {
 	BOOST_FOREACH (AbstractNode *v, n->children) {
-		if (v->modinst->tag_root) return v;
+		if (v->modinst->isRoot()) return v;
 		if (AbstractNode *vroot = find_root_tag(v)) return vroot;
 	}
 	return NULL;
@@ -1504,7 +1504,7 @@ void MainWindow::actionFlushCaches()
 #endif
 	dxf_dim_cache.clear();
 	dxf_cross_cache.clear();
-	Module::libs_cache.clear();
+	Module::clear_library_cache();
 }
 
 void MainWindow::viewModeActionsUncheck()
diff --git a/src/module.cc b/src/module.cc
index 269e128..ba0112d 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -68,7 +68,6 @@ std::string ModuleInstantiation::dump(const std::string &indent) const
 {
 	std::stringstream dump;
 	dump << indent;
-	if (!label.empty()) dump << label <<": ";
 	dump << modname + "(";
 	for (size_t i=0; i < argnames.size(); i++) {
 		if (i > 0) dump << ", ";
@@ -96,10 +95,11 @@ AbstractNode *ModuleInstantiation::evaluate(const Context *ctx) const
 	if (this->ctx) {
 		PRINTF("WARNING: Ignoring recursive module instantiation of '%s'.", modname.c_str());
 	} else {
+		// FIXME: Casting away const..
 		ModuleInstantiation *that = (ModuleInstantiation*)this;
 		that->argvalues.clear();
-		BOOST_FOREACH (Expression *v, that->argexpr) {
-			that->argvalues.push_back(v->evaluate(ctx));
+		BOOST_FOREACH (Expression *expr, that->argexpr) {
+			that->argvalues.push_back(expr->evaluate(ctx));
 		}
 		that->ctx = ctx;
 		node = ctx->evaluate_module(*this);
@@ -113,9 +113,9 @@ std::vector<AbstractNode*> ModuleInstantiation::evaluateChildren(const Context *
 {
 	if (!ctx) ctx = this->ctx;
 	std::vector<AbstractNode*> childnodes;
-	BOOST_FOREACH (ModuleInstantiation *v, this->children) {
-		AbstractNode *n = v->evaluate(ctx);
-		if (n != NULL) childnodes.push_back(n);
+	BOOST_FOREACH (ModuleInstantiation *modinst, this->children) {
+		AbstractNode *node = modinst->evaluate(ctx);
+		if (node) childnodes.push_back(node);
 	}
 	return childnodes;
 }
@@ -124,9 +124,9 @@ std::vector<AbstractNode*> IfElseModuleInstantiation::evaluateElseChildren(const
 {
 	if (!ctx) ctx = this->ctx;
 	std::vector<AbstractNode*> childnodes;
-	BOOST_FOREACH (ModuleInstantiation *v, this->else_children) {
-		AbstractNode *n = v->evaluate(this->ctx);
-		if (n != NULL) childnodes.push_back(n);
+	BOOST_FOREACH (ModuleInstantiation *modinst, this->else_children) {
+		AbstractNode *node = modinst->evaluate(ctx);
+		if (node != NULL) childnodes.push_back(node);
 	}
 	return childnodes;
 }
@@ -200,3 +200,8 @@ std::string Module::dump(const std::string &indent, const std::string &name) con
 	}
 	return dump.str();
 }
+
+void Module::clear_library_cache()
+{
+	Module::libs_cache.clear();
+}
diff --git a/src/module.h b/src/module.h
index c28ab34..557b7c5 100644
--- a/src/module.h
+++ b/src/module.h
@@ -9,28 +9,38 @@
 class ModuleInstantiation
 {
 public:
-	std::string label;
-	std::string modname;
+	ModuleInstantiation(const std::string &name = "") 
+	: ctx(NULL), 
+		tag_root(false), tag_highlight(false), tag_background(false), modname(name) { }
+	virtual ~ModuleInstantiation();
+
+	std::string dump(const std::string &indent) const;
+	class AbstractNode *evaluate(const class Context *ctx) const;
+	std::vector<AbstractNode*> evaluateChildren(const Context *ctx = NULL) const;
+
+	const std::string &name() const { return this->modname; }
+	bool isBackground() const { return this->tag_background; }
+	bool isHighlight() const { return this->tag_highlight; }
+	bool isRoot() const { return this->tag_root; }
+
 	std::vector<std::string> argnames;
-	std::vector<class Expression*> argexpr;
 	std::vector<Value> argvalues;
+	std::vector<class Expression*> argexpr;
 	std::vector<ModuleInstantiation*> children;
+	const Context *ctx;
 
 	bool tag_root;
 	bool tag_highlight;
 	bool tag_background;
-	const class Context *ctx;
-
-	ModuleInstantiation() : tag_root(false), tag_highlight(false), tag_background(false), ctx(NULL) { }
-	virtual ~ModuleInstantiation();
+protected:
+	std::string modname;
 
-	std::string dump(const std::string &indent) const;
-	class AbstractNode *evaluate(const Context *ctx) const;
-	std::vector<AbstractNode*> evaluateChildren(const Context *ctx = NULL) const;
+	friend class Module;
 };
 
 class IfElseModuleInstantiation : public ModuleInstantiation {
 public:
+	IfElseModuleInstantiation() : ModuleInstantiation("if") { }
 	virtual ~IfElseModuleInstantiation();
 	std::vector<AbstractNode*> evaluateElseChildren(const Context *ctx = NULL) const;
 
@@ -48,18 +58,18 @@ public:
 class Module : public AbstractModule
 {
 public:
-	typedef boost::unordered_map<std::string, class Module*> ModuleContainer;
-	ModuleContainer usedlibs;
+	Module() { }
+	virtual ~Module();
+	virtual AbstractNode *evaluate(const Context *ctx, const ModuleInstantiation *inst) const;
+	virtual std::string dump(const std::string &indent, const std::string &name) const;
+
+	void addChild(ModuleInstantiation *ch) { this->children.push_back(ch); }
 
-	struct libs_cache_ent {
-		Module *mod;
-		std::string cache_id, msg;
-	};
-	static boost::unordered_map<std::string, libs_cache_ent> libs_cache;
 	static Module *compile_library(std::string filename);
+	static void clear_library_cache();
 
-	std::vector<std::string> argnames;
-	std::vector<Expression*> argexpr;
+	typedef boost::unordered_map<std::string, class Module*> ModuleContainer;
+	ModuleContainer usedlibs;
 
 	std::vector<std::string> assignments_var;
 	std::vector<Expression*> assignments_expr;
@@ -71,11 +81,17 @@ public:
 
 	std::vector<ModuleInstantiation*> children;
 
-	Module() { }
-	virtual ~Module();
+	std::vector<std::string> argnames;
+	std::vector<Expression*> argexpr;
 
-	virtual AbstractNode *evaluate(const Context *ctx, const ModuleInstantiation *inst) const;
-	virtual std::string dump(const std::string &indent, const std::string &name) const;
+protected:
+
+private:
+	struct libs_cache_ent {
+		Module *mod;
+		std::string cache_id, msg;
+	};
+	static boost::unordered_map<std::string, libs_cache_ent> libs_cache;
 };
 
 #endif
diff --git a/src/node.cc b/src/node.cc
index eccb9a6..e61174f 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -97,7 +97,7 @@ void AbstractNode::progress_report() const
 std::ostream &operator<<(std::ostream &stream, const AbstractNode &node)
 {
 	// FIXME: Don't use deep access to modinst members
-	if (node.modinst->tag_background) stream << "%";
+	if (node.modinst->isBackground()) stream << "%";
 	stream << node.toString();
 	return stream;
 }
diff --git a/src/parser.y b/src/parser.y
index d703a81..b0df50d 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -146,7 +146,7 @@ statement:
 	'{' inner_input '}' |
 	module_instantiation {
 		if ($1) {
-			module->children.push_back($1);
+			module->addChild($1);
 		} else {
 			delete $1;
 		}
@@ -193,10 +193,8 @@ statement:
 children_instantiation:
 	module_instantiation {
 		$$ = new ModuleInstantiation();
-		if ($1) {
+		if ($1) { 
 			$$->children.push_back($1);
-		} else {
-			delete $1;
 		}
 	} |
 	'{' module_instantiation_list '}' {
@@ -206,7 +204,6 @@ children_instantiation:
 if_statement:
 	TOK_IF '(' expr ')' children_instantiation {
 		$$ = new IfElseModuleInstantiation();
-		$$->modname = "if";
 		$$->argnames.push_back("");
 		$$->argexpr.push_back($3);
 
@@ -262,8 +259,7 @@ module_instantiation_list:
 	module_instantiation_list module_instantiation {
 		$$ = $1;
 		if ($$) {
-			if ($2)
-				$$->children.push_back($2);
+			if ($2) $$->children.push_back($2);
 		} else {
 			delete $2;
 		}
@@ -271,19 +267,12 @@ module_instantiation_list:
 
 single_module_instantiation:
 	TOK_ID '(' arguments_call ')' {
-		$$ = new ModuleInstantiation();
-		$$->modname = $1;
+		$$ = new ModuleInstantiation($1);
 		$$->argnames = $3->argnames;
 		$$->argexpr = $3->argexpr;
 		free($1);
 		delete $3;
 	} |
-	TOK_ID ':' single_module_instantiation {
-		$$ = $3;
-		if ($$)
-			$$->label = $1;
-		free($1);
-	} |
 	'!' single_module_instantiation {
 		$$ = $2;
 		if ($$)