From ea1d561c46790cde27e30c48178f3edb0f3cd573 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Mon, 17 Nov 2014 00:57:36 -0500 Subject: [PATCH] Support variable assignment in local blocks. This should fix #347 but more testing is needed --- src/cgaladv.cc | 5 +- src/color.cc | 5 +- src/context.cc | 2 +- src/context.h | 2 +- src/control.cc | 58 +++++++++++------ src/csgops.cc | 5 +- src/evalcontext.cc | 22 +++++++ src/evalcontext.h | 5 +- src/import.cc | 4 +- src/linearextrude.cc | 5 +- src/localscope.cc | 22 +------ src/localscope.h | 2 +- src/modcontext.cc | 5 +- src/modcontext.h | 4 +- src/module.cc | 12 ++-- src/module.h | 6 +- src/offset.cc | 5 +- src/parser.y | 8 +-- src/primitives.cc | 4 +- src/projection.cc | 5 +- src/render.cc | 5 +- src/rotateextrude.cc | 5 +- src/surface.cc | 4 +- src/text.cc | 4 +- src/transform.cc | 5 +- .../scad/misc/scope-assignment-tests.scad | 65 +++++++++++++++++++ tests/CMakeLists.txt | 1 + .../scope-assignment-tests-expected.echo | 23 +++++++ 28 files changed, 208 insertions(+), 90 deletions(-) create mode 100644 testdata/scad/misc/scope-assignment-tests.scad create mode 100644 tests/regression/echotest/scope-assignment-tests-expected.echo diff --git a/src/cgaladv.cc b/src/cgaladv.cc index c6c66ff6..d2ff497f 100644 --- a/src/cgaladv.cc +++ b/src/cgaladv.cc @@ -39,10 +39,10 @@ class CgaladvModule : public AbstractModule public: cgaladv_type_e type; CgaladvModule(cgaladv_type_e type) : type(type) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *CgaladvModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *CgaladvModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { CgaladvNode *node = new CgaladvNode(inst, type); @@ -62,6 +62,7 @@ AbstractNode *CgaladvModule::instantiate(const Context *ctx, const ModuleInstant Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); Value convexity, path, subdiv_type, level; diff --git a/src/color.cc b/src/color.cc index 7792f0bf..109316c0 100644 --- a/src/color.cc +++ b/src/color.cc @@ -42,7 +42,7 @@ class ColorModule : public AbstractModule public: ColorModule(); virtual ~ColorModule(); - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; private: boost::unordered_map webcolors; @@ -205,7 +205,7 @@ ColorModule::~ColorModule() { } -AbstractNode *ColorModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *ColorModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { ColorNode *node = new ColorNode(inst); @@ -218,6 +218,7 @@ AbstractNode *ColorModule::instantiate(const Context *ctx, const ModuleInstantia Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); Value v = c.lookup_variable("c"); if (v.type() == Value::VECTOR) { diff --git a/src/context.cc b/src/context.cc index 2fdeceeb..6a05eb2e 100644 --- a/src/context.cc +++ b/src/context.cc @@ -152,7 +152,7 @@ Value Context::evaluate_function(const std::string &name, const EvalContext *eva return Value(); } -AbstractNode *Context::instantiate_module(const ModuleInstantiation &inst, const EvalContext *evalctx) const +AbstractNode *Context::instantiate_module(const ModuleInstantiation &inst, EvalContext *evalctx) const { if (this->parent) return this->parent->instantiate_module(inst, evalctx); PRINTB("WARNING: Ignoring unknown module '%s'.", inst.name()); diff --git a/src/context.h b/src/context.h index 3465c946..29c887ce 100644 --- a/src/context.h +++ b/src/context.h @@ -15,7 +15,7 @@ public: const Context *getParent() const { return this->parent; } virtual Value evaluate_function(const std::string &name, const class EvalContext *evalctx) const; - virtual class AbstractNode *instantiate_module(const class ModuleInstantiation &inst, const EvalContext *evalctx) const; + virtual class AbstractNode *instantiate_module(const class ModuleInstantiation &inst, EvalContext *evalctx) const; void setVariables(const AssignmentList &args, const class EvalContext *evalctx = NULL); diff --git a/src/control.cc b/src/control.cc index 79ce6910..5f629dab 100644 --- a/src/control.cc +++ b/src/control.cc @@ -29,6 +29,7 @@ #include "node.h" #include "evalcontext.h" #include "modcontext.h" +#include "expression.h" #include "builtin.h" #include "printutils.h" #include @@ -55,7 +56,7 @@ public: // methods : type(type) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; static void for_eval(AbstractNode &node, const ModuleInstantiation &inst, size_t l, const Context *ctx, const EvalContext *evalctx); @@ -99,7 +100,14 @@ void ControlModule::for_eval(AbstractNode &node, const ModuleInstantiation &inst for_eval(node, inst, l+1, &c, evalctx); } } else if (l > 0) { - std::vector instantiatednodes = inst.instantiateChildren(ctx); + // At this point, the for loop variables have been set and we can initialize + // the local scope (as they may depend on the for loop variables + Context c(ctx); + BOOST_FOREACH(const Assignment &ass, inst.scope.assignments) { + c.set_variable(ass.first, ass.second->evaluate(&c)); + } + + std::vector instantiatednodes = inst.instantiateChildren(&c); node.children.insert(node.children.end(), instantiatednodes.begin(), instantiatednodes.end()); } } @@ -155,12 +163,16 @@ AbstractNode* ControlModule::getChild(const Value& value, const EvalContext* mod return modulectx->getChild(n)->evaluate(modulectx); } -AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleInstantiation *inst, EvalContext *evalctx) const { AbstractNode *node = NULL; - if (type == CHILD) - { + if (this->type != FOR && this->type != INT_FOR) { + evalctx->applyScope(); + } + + switch (this->type) { + case CHILD: { printDeprecation("DEPRECATED: child() will be removed in future releases. Use children() instead."); int n = 0; if (evalctx->numArgs() > 0) { @@ -192,9 +204,9 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns } return node; } + break; - if (type == CHILDREN) - { + case CHILDREN: { const EvalContext *modulectx = getLastModuleCtx(evalctx); if (modulectx==NULL) { return NULL; @@ -251,14 +263,10 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns } return NULL; } + break; - if (type == INT_FOR) - node = new AbstractIntersectionNode(inst); - else + case ECHO: { node = new AbstractNode(inst); - - if (type == ECHO) - { std::stringstream msg; msg << "ECHO: "; for (size_t i = 0; i < inst->arguments.size(); i++) { @@ -273,9 +281,10 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns } PRINTB("%s", msg.str()); } + break; - if (type == ASSIGN) - { + case ASSIGN: { + node = new AbstractNode(inst); Context c(evalctx); for (size_t i = 0; i < evalctx->numArgs(); i++) { if (!evalctx->getArgName(i).empty()) @@ -284,14 +293,20 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns std::vector instantiatednodes = inst->instantiateChildren(&c); node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); } + break; - if (type == FOR || type == INT_FOR) - { + case FOR: + node = new AbstractNode(inst); for_eval(*node, *inst, 0, evalctx, evalctx); - } + break; - if (type == IF) - { + case INT_FOR: + node = new AbstractIntersectionNode(inst); + for_eval(*node, *inst, 0, evalctx, evalctx); + break; + + case IF: { + node = new AbstractNode(inst); const IfElseModuleInstantiation *ifelse = dynamic_cast(inst); if (evalctx->numArgs() > 0 && evalctx->getArgValue(0).toBool()) { std::vector instantiatednodes = ifelse->instantiateChildren(evalctx); @@ -302,7 +317,8 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); } } - + break; + } return node; } diff --git a/src/csgops.cc b/src/csgops.cc index 28d15157..4ba31f29 100644 --- a/src/csgops.cc +++ b/src/csgops.cc @@ -38,11 +38,12 @@ class CsgModule : public AbstractModule public: OpenSCADOperator type; CsgModule(OpenSCADOperator type) : type(type) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *CsgModule::instantiate(const Context*, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *CsgModule::instantiate(const Context*, const ModuleInstantiation *inst, EvalContext *evalctx) const { + evalctx->applyScope(); CsgNode *node = new CsgNode(inst, type); std::vector instantiatednodes = inst->instantiateChildren(evalctx); node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); diff --git a/src/evalcontext.cc b/src/evalcontext.cc index 1b3624e7..b8de1d17 100644 --- a/src/evalcontext.cc +++ b/src/evalcontext.cc @@ -8,6 +8,12 @@ #include +EvalContext::EvalContext(const Context *parent, + const AssignmentList &args, const class LocalScope *const scope) + : Context(parent), eval_arguments(args), scope(scope) +{ +} + const std::string &EvalContext::getArgName(size_t i) const { assert(i < this->eval_arguments.size()); @@ -31,6 +37,22 @@ ModuleInstantiation *EvalContext::getChild(size_t i) const return this->scope ? this->scope->children[i] : NULL; } +/*! + When instantiating a module which can take a scope as parameter (i.e. non-leaf nodes), + use this method to apply the local scope definitions to the evaluation context. + This will enable variables defined in local blocks. + NB! for loops are special as the local block may depend on variables evaluated by the + for loop parameters. The for loop code will handle this specially. +*/ +void EvalContext::applyScope() +{ + if (this->scope) { + BOOST_FOREACH(const Assignment &ass, this->scope->assignments) { + this->set_variable(ass.first, ass.second->evaluate(this)); + } + } +} + #ifdef DEBUG std::string EvalContext::dump(const AbstractModule *mod, const ModuleInstantiation *inst) { diff --git a/src/evalcontext.h b/src/evalcontext.h index 928e96f6..5cdd7f8b 100644 --- a/src/evalcontext.h +++ b/src/evalcontext.h @@ -12,8 +12,7 @@ public: typedef std::vector InstanceList; EvalContext(const Context *parent, - const AssignmentList &args, const class LocalScope *const scope = NULL) - : Context(parent), eval_arguments(args), scope(scope) {} + const AssignmentList &args, const class LocalScope *const scope = NULL); virtual ~EvalContext() {} size_t numArgs() const { return this->eval_arguments.size(); } @@ -23,6 +22,8 @@ public: size_t numChildren() const; ModuleInstantiation *getChild(size_t i) const; + void applyScope(); + #ifdef DEBUG virtual std::string dump(const class AbstractModule *mod, const ModuleInstantiation *inst); #endif diff --git a/src/import.cc b/src/import.cc index 3841dc02..ad667592 100644 --- a/src/import.cc +++ b/src/import.cc @@ -61,10 +61,10 @@ class ImportModule : public AbstractModule public: import_type_e type; ImportModule(import_type_e type = TYPE_UNKNOWN) : type(type) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *ImportModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *ImportModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { AssignmentList args; args += Assignment("file"), Assignment("layer"), Assignment("convexity"), Assignment("origin"), Assignment("scale"); diff --git a/src/linearextrude.cc b/src/linearextrude.cc index c3ba1ee6..746b1a82 100644 --- a/src/linearextrude.cc +++ b/src/linearextrude.cc @@ -46,10 +46,10 @@ class LinearExtrudeModule : public AbstractModule { public: LinearExtrudeModule() { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *LinearExtrudeModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *LinearExtrudeModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { LinearExtrudeNode *node = new LinearExtrudeNode(inst); @@ -58,6 +58,7 @@ AbstractNode *LinearExtrudeModule::instantiate(const Context *ctx, const ModuleI Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); node->fn = c.lookup_variable("$fn").toDouble(); node->fs = c.lookup_variable("$fs").toDouble(); diff --git a/src/localscope.cc b/src/localscope.cc index 4f31a54b..9c41035e 100644 --- a/src/localscope.cc +++ b/src/localscope.cc @@ -43,32 +43,14 @@ std::string LocalScope::dump(const std::string &indent) const } // FIXME: Two parameters here is a hack. Rather have separate types of scopes, or check the type of the first parameter. Note const vs. non-const -std::vector LocalScope::instantiateChildren(const Context *evalctx, FileContext *filectx) const +std::vector LocalScope::instantiateChildren(const Context *evalctx) const { - Context *c = filectx; - - if (!c) { - c = new Context(evalctx); - - // FIXME: If we make c a ModuleContext, child() doesn't work anymore - // c->functions_p = &this->functions; - // c->modules_p = &this->modules; - - // Uncommenting the following would allow assignments in local scopes, - // but would cause duplicate evaluation of module scopes - // BOOST_FOREACH (const Assignment &ass, this->assignments) { - // c->set_variable(ass.first, ass.second->evaluate(c)); - // } - } - std::vector childnodes; BOOST_FOREACH (ModuleInstantiation *modinst, this->children) { - AbstractNode *node = modinst->evaluate(c); + AbstractNode *node = modinst->evaluate(evalctx); if (node) childnodes.push_back(node); } - if (c != filectx) delete c; - return childnodes; } diff --git a/src/localscope.h b/src/localscope.h index 83e4f105..e1b9dd6a 100644 --- a/src/localscope.h +++ b/src/localscope.h @@ -11,7 +11,7 @@ public: size_t numElements() const { return assignments.size() + children.size(); } std::string dump(const std::string &indent) const; - std::vector instantiateChildren(const class Context *evalctx, class FileContext *filectx = NULL) const; + std::vector instantiateChildren(const class Context *evalctx) const; void addChild(ModuleInstantiation *ch); AssignmentList assignments; diff --git a/src/modcontext.cc b/src/modcontext.cc index c6210f42..859a1c95 100644 --- a/src/modcontext.cc +++ b/src/modcontext.cc @@ -71,6 +71,7 @@ void ModuleContext::initializeModule(const class Module &module) BOOST_FOREACH(const Assignment &ass, module.scope.assignments) { this->set_variable(ass.first, ass.second->evaluate(this)); } + // Experimental code. See issue #399 // evaluateAssignments(module.scope.assignments); } @@ -130,7 +131,7 @@ Value ModuleContext::evaluate_function(const std::string &name, const EvalContex return Context::evaluate_function(name, evalctx); } -AbstractNode *ModuleContext::instantiate_module(const ModuleInstantiation &inst, const EvalContext *evalctx) const +AbstractNode *ModuleContext::instantiate_module(const ModuleInstantiation &inst, EvalContext *evalctx) const { const AbstractModule *foundm = this->findLocalModule(inst.name()); if (foundm) return foundm->instantiate(this, &inst, evalctx); @@ -207,7 +208,7 @@ Value FileContext::evaluate_function(const std::string &name, const EvalContext return ModuleContext::evaluate_function(name, evalctx); } -AbstractNode *FileContext::instantiate_module(const ModuleInstantiation &inst, const EvalContext *evalctx) const +AbstractNode *FileContext::instantiate_module(const ModuleInstantiation &inst, EvalContext *evalctx) const { const AbstractModule *foundm = this->findLocalModule(inst.name()); if (foundm) return foundm->instantiate(this, &inst, evalctx); diff --git a/src/modcontext.h b/src/modcontext.h index 2e33c962..1a850314 100644 --- a/src/modcontext.h +++ b/src/modcontext.h @@ -21,7 +21,7 @@ public: virtual Value evaluate_function(const std::string &name, const EvalContext *evalctx) const; virtual AbstractNode *instantiate_module(const ModuleInstantiation &inst, - const EvalContext *evalctx) const; + EvalContext *evalctx) const; const AbstractModule *findLocalModule(const std::string &name) const; const AbstractFunction *findLocalFunction(const std::string &name) const; @@ -47,7 +47,7 @@ public: virtual ~FileContext() {} virtual Value evaluate_function(const std::string &name, const EvalContext *evalctx) const; virtual AbstractNode *instantiate_module(const ModuleInstantiation &inst, - const EvalContext *evalctx) const; + EvalContext *evalctx) const; private: const FileModule::ModuleContainer &usedlibs; diff --git a/src/module.cc b/src/module.cc index 95224be8..634d3573 100644 --- a/src/module.cc +++ b/src/module.cc @@ -46,7 +46,7 @@ AbstractModule::~AbstractModule() { } -AbstractNode *AbstractModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *AbstractModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { (void)ctx; // avoid unusued parameter warning @@ -185,7 +185,7 @@ private: const ModuleInstantiation &inst; }; -AbstractNode *Module::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *Module::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { ModRecursionGuard g(*inst); if (g.recursion_detected()) { @@ -193,6 +193,10 @@ AbstractNode *Module::instantiate(const Context *ctx, const ModuleInstantiation return NULL; } + // At this point we know that nobody will modify the dependencies of the local scope + // passed to this instance, so we can populate the context + evalctx->applyScope(); + ModuleContext c(ctx, evalctx); // set $children first since we might have variables depending on it c.set_variable("$children", Value(double(inst->scope.children.size()))); @@ -342,7 +346,7 @@ bool FileModule::handleDependencies() return somethingchanged; } -AbstractNode *FileModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *FileModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { assert(evalctx == NULL); FileContext c(*this, ctx); @@ -353,7 +357,7 @@ AbstractNode *FileModule::instantiate(const Context *ctx, const ModuleInstantiat #endif AbstractNode *node = new AbstractNode(inst); - std::vector instantiatednodes = this->scope.instantiateChildren(ctx, &c); + std::vector instantiatednodes = this->scope.instantiateChildren(&c); node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); return node; diff --git a/src/module.h b/src/module.h index 7962ec26..8d1f1e13 100644 --- a/src/module.h +++ b/src/module.h @@ -68,7 +68,7 @@ public: virtual ~AbstractModule(); virtual bool is_experimental() const { return feature != NULL; } virtual bool is_enabled() const { return (feature == NULL) || feature->is_enabled(); } - virtual class AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const class EvalContext *evalctx = NULL) const; + virtual class AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, class EvalContext *evalctx = NULL) const; virtual std::string dump(const std::string &indent, const std::string &name) const; virtual double lookup_double_variable_with_default(Context &c, std::string variable, double def) const; virtual std::string lookup_string_variable_with_default(Context &c, std::string variable, std::string def) const; @@ -81,7 +81,7 @@ public: Module(const Feature& feature) : AbstractModule(feature) { } virtual ~Module(); - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx = NULL) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx = NULL) const; virtual std::string dump(const std::string &indent, const std::string &name) const; static const std::string& stack_element(int n) { return module_stack[n]; }; static int stack_size() { return module_stack.size(); }; @@ -108,7 +108,7 @@ public: void registerInclude(const std::string &localpath, const std::string &fullpath); bool includesChanged() const; bool handleDependencies(); - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx = NULL) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx = NULL) const; bool hasIncludes() const { return !this->includes.empty(); } bool usesLibraries() const { return !this->usedlibs.empty(); } bool isHandlingDependencies() const { return this->is_handling_dependencies; } diff --git a/src/offset.cc b/src/offset.cc index 1b2c7160..31d2ad96 100644 --- a/src/offset.cc +++ b/src/offset.cc @@ -46,10 +46,10 @@ class OffsetModule : public AbstractModule { public: OffsetModule() { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *OffsetModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *OffsetModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { OffsetNode *node = new OffsetNode(inst); @@ -58,6 +58,7 @@ AbstractNode *OffsetModule::instantiate(const Context *ctx, const ModuleInstanti Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); node->fn = c.lookup_variable("$fn").toDouble(); node->fs = c.lookup_variable("$fs").toDouble(); diff --git a/src/parser.y b/src/parser.y index 6c0d5379..a7a1008b 100644 --- a/src/parser.y +++ b/src/parser.y @@ -270,6 +270,7 @@ if_statement: child_statements: /* empty */ | child_statements child_statement + | child_statements assignment ; child_statement: @@ -281,13 +282,6 @@ child_statement: } ; -/* - FIXME: This allows for variable declaration in child blocks, not activated yet - | -assignment ; -*/ - - // "for" is a valid module identifier module_id: TOK_ID { $$ = $1; } diff --git a/src/primitives.cc b/src/primitives.cc index 33e59109..a5045f86 100644 --- a/src/primitives.cc +++ b/src/primitives.cc @@ -61,7 +61,7 @@ class PrimitiveModule : public AbstractModule public: primitive_type_e type; PrimitiveModule(primitive_type_e type) : type(type) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; private: Value lookup_radius(const Context &ctx, const std::string &radius_var, const std::string &diameter_var) const; }; @@ -141,7 +141,7 @@ Value PrimitiveModule::lookup_radius(const Context &ctx, const std::string &diam } } -AbstractNode *PrimitiveModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *PrimitiveModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { PrimitiveNode *node = new PrimitiveNode(inst, this->type); diff --git a/src/projection.cc b/src/projection.cc index 7afd0e46..d27b28ce 100644 --- a/src/projection.cc +++ b/src/projection.cc @@ -41,10 +41,10 @@ class ProjectionModule : public AbstractModule { public: ProjectionModule() { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *ProjectionModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *ProjectionModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { ProjectionNode *node = new ProjectionNode(inst); @@ -53,6 +53,7 @@ AbstractNode *ProjectionModule::instantiate(const Context *ctx, const ModuleInst Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); Value convexity = c.lookup_variable("convexity", true); Value cut = c.lookup_variable("cut", true); diff --git a/src/render.cc b/src/render.cc index d29a3886..83dcbbae 100644 --- a/src/render.cc +++ b/src/render.cc @@ -38,10 +38,10 @@ class RenderModule : public AbstractModule { public: RenderModule() { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *RenderModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *RenderModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { RenderNode *node = new RenderNode(inst); @@ -50,6 +50,7 @@ AbstractNode *RenderModule::instantiate(const Context *ctx, const ModuleInstanti Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); Value v = c.lookup_variable("convexity"); if (v.type() == Value::NUMBER) diff --git a/src/rotateextrude.cc b/src/rotateextrude.cc index ce52fd79..483b118e 100644 --- a/src/rotateextrude.cc +++ b/src/rotateextrude.cc @@ -44,10 +44,10 @@ class RotateExtrudeModule : public AbstractModule { public: RotateExtrudeModule() { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *RotateExtrudeModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *RotateExtrudeModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { RotateExtrudeNode *node = new RotateExtrudeNode(inst); @@ -56,6 +56,7 @@ AbstractNode *RotateExtrudeModule::instantiate(const Context *ctx, const ModuleI Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); node->fn = c.lookup_variable("$fn").toDouble(); node->fs = c.lookup_variable("$fs").toDouble(); diff --git a/src/surface.cc b/src/surface.cc index e8a93355..9f55d34a 100644 --- a/src/surface.cc +++ b/src/surface.cc @@ -52,7 +52,7 @@ class SurfaceModule : public AbstractModule { public: SurfaceModule() { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; typedef boost::unordered_map,double> img_data_t; @@ -80,7 +80,7 @@ private: img_data_t read_png_or_dat(std::string filename) const; }; -AbstractNode *SurfaceModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *SurfaceModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { SurfaceNode *node = new SurfaceNode(inst); node->center = false; diff --git a/src/text.cc b/src/text.cc index 64254b90..152e64bf 100644 --- a/src/text.cc +++ b/src/text.cc @@ -41,10 +41,10 @@ class TextModule : public AbstractModule { public: TextModule() : AbstractModule(Feature::ExperimentalTextModule) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *TextModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *TextModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { TextNode *node = new TextNode(inst); diff --git a/src/transform.cc b/src/transform.cc index 25723fec..0d5506af 100644 --- a/src/transform.cc +++ b/src/transform.cc @@ -50,10 +50,10 @@ class TransformModule : public AbstractModule public: transform_type_e type; TransformModule(transform_type_e type) : type(type) { } - virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const; + virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const; }; -AbstractNode *TransformModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const +AbstractNode *TransformModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, EvalContext *evalctx) const { TransformNode *node = new TransformNode(inst); @@ -83,6 +83,7 @@ AbstractNode *TransformModule::instantiate(const Context *ctx, const ModuleInsta Context c(ctx); c.setVariables(args, evalctx); + evalctx->applyScope(); if (this->type == SCALE) { diff --git a/testdata/scad/misc/scope-assignment-tests.scad b/testdata/scad/misc/scope-assignment-tests.scad new file mode 100644 index 00000000..a210bf1e --- /dev/null +++ b/testdata/scad/misc/scope-assignment-tests.scad @@ -0,0 +1,65 @@ +echo("union scope"); +a = 4; +union() { + a = 5; + echo("local a (5):", a); +} +echo("global a (4):", a); + + +echo("module scope:"); +module mymodule(b=6) { + b = 7; + echo("local b (7)", b); +} +mymodule(); +mymodule(8); + + +echo("module children scope:"); +module mymodule2(b2=6) { + b2 = 2; + children(0); +} +mymodule2(b2=7) { + b2 = 3; + echo("b2 (3)", b2); +} + +echo("for loop (c = 0,1,25):"); +for (i=[0:2]) { + c = (i > 1) ? i + 23 : i; + echo("c", c); +} + +echo("anonymous inner scope (scope ignored):"); +union() { + e = 2; + echo("outer e (3)", e); + { + e = 3; + echo("inner e (3)", e); + } +} + +echo("anonymous scope (scope ignored):"); +f=1; +echo("outer f (2)", f); +{ + f=2; + echo("inner f (2)", f); +} + +echo("anonymous scope reassign:"); +{ + g=1; + echo("g (2)", g); + g=2; +} + +echo("anonymous reassign using outer (scope ignored)", h); +h=5; +{ + h=h*2; // Not allowed + echo("h (undef)", h); +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 88253bb9..2d5f05f1 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1046,6 +1046,7 @@ list(APPEND ECHO_FILES ${FUNCTION_FILES} ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/value-reassignment-tests.scad ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/value-reassignment-tests2.scad ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/variable-scope-tests.scad + ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/scope-assignment-tests.scad ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/lookup-tests.scad ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/expression-shortcircuit-tests.scad ${CMAKE_SOURCE_DIR}/../testdata/scad/misc/parent_module-tests.scad diff --git a/tests/regression/echotest/scope-assignment-tests-expected.echo b/tests/regression/echotest/scope-assignment-tests-expected.echo new file mode 100644 index 00000000..2b9f5def --- /dev/null +++ b/tests/regression/echotest/scope-assignment-tests-expected.echo @@ -0,0 +1,23 @@ +WARNING: Ignoring unknown variable 'h'. +ECHO: "union scope" +ECHO: "local a (5):", 5 +ECHO: "global a (4):", 4 +ECHO: "module scope:" +ECHO: "local b (7)", 7 +ECHO: "local b (7)", 7 +ECHO: "module children scope:" +ECHO: "b2 (3)", 3 +ECHO: "for loop (c = 0,1,25):" +ECHO: "c", 0 +ECHO: "c", 1 +ECHO: "c", 25 +ECHO: "anonymous inner scope (scope ignored):" +ECHO: "outer e (3)", 3 +ECHO: "inner e (3)", 3 +ECHO: "anonymous scope (scope ignored):" +ECHO: "outer f (2)", 2 +ECHO: "inner f (2)", 2 +ECHO: "anonymous scope reassign:" +ECHO: "g (2)", 2 +ECHO: "anonymous reassign using outer (scope ignored)", undef +ECHO: "h (undef)", undef