Fixes two problems related to : lookup was dynamic rather than lexical, assignment was done after all local variables causing it not to be copyable

4 files changed, 50 insertions, 6 deletions

diff --git a/src/context.cc b/src/context.cc
index a7273a4..b5420ff 100644
--- a/src/context.cc
+++ b/src/context.cc
@@ -38,6 +38,13 @@ namespace fs = boost::filesystem;
 
 std::vector<const Context*> Context::ctx_stack;
 
+// $children is not a config_variable. config_variables have dynamic scope, 
+// meaning they are passed down the call chain implicitly.
+// $children is simply misnamed and shouldn't have included the '$'.
+static bool is_config_variable(const std::string &name) {
+	return name[0] == '$' && name != "$children";
+}
+
 /*!
 	Initializes this context. Optionally initializes a context for an external library
 */
@@ -80,10 +87,8 @@ void Context::setVariables(const AssignmentList &args,
 
 void Context::set_variable(const std::string &name, const Value &value)
 {
-	if (name[0] == '$')
-		this->config_variables[name] = value;
-	else
-		this->variables[name] = value;
+	if (is_config_variable(name)) this->config_variables[name] = value;
+	else this->variables[name] = value;
 }
 
 void Context::set_constant(const std::string &name, const Value &value)
@@ -98,7 +103,7 @@ void Context::set_constant(const std::string &name, const Value &value)
 
 Value Context::lookup_variable(const std::string &name, bool silent) const
 {
-	if (name[0] == '$') {
+	if (is_config_variable(name)) {
 		for (int i = ctx_stack.size()-1; i >= 0; i--) {
 			const ValueMap &confvars = ctx_stack[i]->config_variables;
 			if (confvars.find(name) != confvars.end())
diff --git a/src/module.cc b/src/module.cc
index 8fb8506..f66c8ef 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -180,8 +180,9 @@ AbstractNode *Module::instantiate(const Context *ctx, const ModuleInstantiation
 	}
 
 	ModuleContext c(ctx, evalctx);
-	c.initializeModule(*this);
+	// set $children first since we might have variables depending on it
 	c.set_variable("$children", Value(double(inst->scope.children.size())));
+	c.initializeModule(*this);
 	// FIXME: Set document path to the path of the module
 #if 0 && DEBUG
 	c.dump(this, inst);
diff --git a/testdata/scad/misc/variable-scope-tests.scad b/testdata/scad/misc/variable-scope-tests.scad
index 104d1a4..2a65d0d 100644
--- a/testdata/scad/misc/variable-scope-tests.scad
+++ b/testdata/scad/misc/variable-scope-tests.scad
@@ -11,6 +11,35 @@ module special_module2(b) {
 
 special_module(23, $fn=5);
 
+echo("$children scope");
+module child_module_2() {
+  echo("$children should be 4: ", $children);
+  for(i=[0:$children-1]) child(i);
+}
+
+module child_module_1() {
+  echo("$children should be 1: ", $children);
+  child_module_2() {
+    echo("$children should be 1: ", $children);
+    child(0);
+    echo("child_module_2 child 0");
+    echo("child_module_2 child 1");
+  }
+}
+
+child_module_1() echo("child_module_1 child");
+
+echo("copy $children");
+module copy_children_module() {
+  kids = $children;
+  echo("copy_children_module: ", kids, $children);
+}
+
+copy_children_module() {
+  cube();
+  sphere();
+}
+
 echo("inner variables shadows parameter");
 module inner_variables(a, b) {
   b = 24;
diff --git a/tests/regression/echotest/variable-scope-tests-expected.txt b/tests/regression/echotest/variable-scope-tests-expected.txt
index 5994778..2a82090 100644
--- a/tests/regression/echotest/variable-scope-tests-expected.txt
+++ b/tests/regression/echotest/variable-scope-tests-expected.txt
@@ -3,6 +3,15 @@ ECHO: 23, 5
 WARNING: Ignoring unknown variable 'a'.
 ECHO: undef
 ECHO: 23, 5
+ECHO: "$children scope"
+ECHO: "$children should be 1: ", 1
+ECHO: "$children should be 4: ", 4
+ECHO: "$children should be 1: ", 1
+ECHO: "child_module_1 child"
+ECHO: "child_module_2 child 0"
+ECHO: "child_module_2 child 1"
+ECHO: "copy $children"
+ECHO: "copy_children_module: ", 2, 2
 ECHO: "inner variables shadows parameter"
 ECHO: 5, 24
 ECHO: "user-defined special variables as parameter"