From ea1d561c46790cde27e30c48178f3edb0f3cd573 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Mon, 17 Nov 2014 00:57:36 -0500 Subject: [PATCH 1/8] 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 From 6a1bc270fd39990112cff066d2b55c7aa4d0a358 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Thu, 20 Nov 2014 18:30:58 -0500 Subject: [PATCH 2/8] Removed unused member variable --- src/evalcontext.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/evalcontext.h b/src/evalcontext.h index 5cdd7f8b..20600f76 100644 --- a/src/evalcontext.h +++ b/src/evalcontext.h @@ -30,6 +30,5 @@ public: private: const AssignmentList &eval_arguments; - std::vector > eval_values; const LocalScope *const scope; }; From d381ec8bcf20352a03a9b3c0d8277bb7aba364a5 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Thu, 20 Nov 2014 18:31:24 -0500 Subject: [PATCH 3/8] #347 Deprecated assign() --- src/builtin.cc | 11 ++++++----- src/modcontext.cc | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/builtin.cc b/src/builtin.cc index c20f9f03..d91d17ad 100644 --- a/src/builtin.cc +++ b/src/builtin.cc @@ -77,11 +77,12 @@ void Builtins::initialize() register_builtin_dxf_rotate_extrude(); register_builtin_text(); - this->deprecations["dxf_linear_extrude"] = "linear_extrude"; - this->deprecations["dxf_rotate_extrude"] = "rotate_extrude"; - this->deprecations["import_stl"] = "import"; - this->deprecations["import_dxf"] = "import"; - this->deprecations["import_off"] = "import"; + this->deprecations["dxf_linear_extrude"] = "linear_extrude()"; + this->deprecations["dxf_rotate_extrude"] = "rotate_extrude()"; + this->deprecations["import_stl"] = "import()"; + this->deprecations["import_dxf"] = "import()"; + this->deprecations["import_off"] = "import()"; + this->deprecations["assign"] = "a regular assignment"; } std::string Builtins::isDeprecated(const std::string &name) diff --git a/src/modcontext.cc b/src/modcontext.cc index 859a1c95..3dc711a6 100644 --- a/src/modcontext.cc +++ b/src/modcontext.cc @@ -116,7 +116,7 @@ const AbstractModule *ModuleContext::findLocalModule(const std::string &name) co } std::string replacement = Builtins::instance()->isDeprecated(name); if (!replacement.empty()) { - PRINT_DEPRECATION("DEPRECATED: The %s() module will be removed in future releases. Use %s() instead.", name % replacement); + PRINT_DEPRECATION("DEPRECATED: The %s() module will be removed in future releases. Use %s instead.", name % replacement); } return m; } From 6b61e9b1a720055761b535ac9e2c246dffbceb2f Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Mon, 24 Nov 2014 21:57:36 -0500 Subject: [PATCH 4/8] sync --- releases/2014.QX.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/releases/2014.QX.md b/releases/2014.QX.md index 17d0fa94..15500944 100644 --- a/releases/2014.QX.md +++ b/releases/2014.QX.md @@ -6,6 +6,7 @@ * min() and max() can now take a wvector argument * concat() * 2D minkowski and holes +* chr() **Program Features:** * Added --viewall cmd-line parameter @@ -14,6 +15,12 @@ * Qt5, retina * SVG import/export * AMF import/export +* Color schemes for viewer and editor can be user-edited +* Improved editor +* Splash screen +* Docking of GUI components +* Toolbar +* More robust STL export **Bugfixes/improvements:** * Internal cavity fix From 5072f0752acf663569776b962371073c1a76608f Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Tue, 25 Nov 2014 00:43:48 -0500 Subject: [PATCH 5/8] Don't use assign now that it's deprecated --- testdata/scad/misc/search-tests-unicode.scad | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/testdata/scad/misc/search-tests-unicode.scad b/testdata/scad/misc/search-tests-unicode.scad index d863eff9..f9f841a6 100644 --- a/testdata/scad/misc/search-tests-unicode.scad +++ b/testdata/scad/misc/search-tests-unicode.scad @@ -2,16 +2,13 @@ //Helper function that pretty prints our search test //Expected result is checked against execution of a search() invocation and OK/FAIL is indicated -module test_search_and_echo( exp_res, search_to_find, search_to_search, search_up_to_num_matches = undef) -{ - if(undef != search_up_to_num_matches) - { - assign( test_res = search(search_to_find, search_to_search, search_up_to_num_matches) ) +module test_search_and_echo( exp_res, search_to_find, search_to_search, search_up_to_num_matches = undef) { + if (undef != search_up_to_num_matches) { + test_res = search(search_to_find, search_to_search, search_up_to_num_matches); echo(str("Expect ", exp_res, " for search(", search_to_find, ", ", search_to_search, ", ", search_up_to_num_matches, ")=", test_res, ". ", (exp_res == test_res)?"OK":"FAIL" )); } - else - { - assign( test_res = search(search_to_find, search_to_search) ) + else { + test_res = search(search_to_find, search_to_search); echo(str("Expect ", exp_res, " for search(", search_to_find, ", ", search_to_search, ")=", test_res, ". ", (exp_res == test_res)?"OK":"FAIL" )); } } From d195d050086694f26f413787da7a327d4ed3f309 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Tue, 25 Nov 2014 00:44:12 -0500 Subject: [PATCH 6/8] Test assignment in if, else and assign scopes --- .../scad/misc/scope-assignment-tests.scad | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/testdata/scad/misc/scope-assignment-tests.scad b/testdata/scad/misc/scope-assignment-tests.scad index a210bf1e..08ce921d 100644 --- a/testdata/scad/misc/scope-assignment-tests.scad +++ b/testdata/scad/misc/scope-assignment-tests.scad @@ -32,6 +32,19 @@ for (i=[0:2]) { echo("c", c); } +echo("if scope:"); +if (true) { + d = 8; + echo("d (8)", d); +} + +echo("else scope:"); +if (false) { +} else { + d = 9; + echo("d (9)", d); +} + echo("anonymous inner scope (scope ignored):"); union() { e = 2; @@ -63,3 +76,10 @@ h=5; h=h*2; // Not allowed echo("h (undef)", h); } + +echo("override variable in assign scope:"); +assign(i=9) { + i=10; + echo("i (10)", i); +} + From 23c9dee2657fd43d1b83102df4dd66c7951419af Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Tue, 25 Nov 2014 00:45:00 -0500 Subject: [PATCH 7/8] bugfix: Correctly handle else scopes, handle overrides inside assign scopes --- src/cgaladv.cc | 2 +- src/color.cc | 2 +- src/control.cc | 10 ++++++---- src/csgops.cc | 2 +- src/evalcontext.cc | 16 ---------------- src/evalcontext.h | 2 -- src/linearextrude.cc | 2 +- src/localscope.cc | 13 +++++++++++++ src/localscope.h | 1 + src/module.cc | 2 +- src/offset.cc | 2 +- src/projection.cc | 2 +- src/render.cc | 2 +- src/rotateextrude.cc | 2 +- src/transform.cc | 2 +- 15 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/cgaladv.cc b/src/cgaladv.cc index d2ff497f..1822cac0 100644 --- a/src/cgaladv.cc +++ b/src/cgaladv.cc @@ -62,7 +62,7 @@ AbstractNode *CgaladvModule::instantiate(const Context *ctx, const ModuleInstant Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); Value convexity, path, subdiv_type, level; diff --git a/src/color.cc b/src/color.cc index 109316c0..eb7fb2e0 100644 --- a/src/color.cc +++ b/src/color.cc @@ -218,7 +218,7 @@ AbstractNode *ColorModule::instantiate(const Context *ctx, const ModuleInstantia Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); Value v = c.lookup_variable("c"); if (v.type() == Value::VECTOR) { diff --git a/src/control.cc b/src/control.cc index 5f629dab..b2518670 100644 --- a/src/control.cc +++ b/src/control.cc @@ -167,10 +167,6 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns { AbstractNode *node = NULL; - 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."); @@ -285,11 +281,15 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns case ASSIGN: { node = new AbstractNode(inst); + // We create a new context to avoid parameters from influencing each other + // -> parallel evaluation. This is to be backwards compatible. Context c(evalctx); for (size_t i = 0; i < evalctx->numArgs(); i++) { if (!evalctx->getArgName(i).empty()) c.set_variable(evalctx->getArgName(i), evalctx->getArgValue(i)); } + // Let any local variables override the parameters + inst->scope.apply(c); std::vector instantiatednodes = inst->instantiateChildren(&c); node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); } @@ -309,10 +309,12 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns node = new AbstractNode(inst); const IfElseModuleInstantiation *ifelse = dynamic_cast(inst); if (evalctx->numArgs() > 0 && evalctx->getArgValue(0).toBool()) { + inst->scope.apply(*evalctx); std::vector instantiatednodes = ifelse->instantiateChildren(evalctx); node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); } else { + ifelse->else_scope.apply(*evalctx); std::vector instantiatednodes = ifelse->instantiateElseChildren(evalctx); node->children.insert(node->children.end(), instantiatednodes.begin(), instantiatednodes.end()); } diff --git a/src/csgops.cc b/src/csgops.cc index 4ba31f29..038348b9 100644 --- a/src/csgops.cc +++ b/src/csgops.cc @@ -43,7 +43,7 @@ public: AbstractNode *CsgModule::instantiate(const Context*, const ModuleInstantiation *inst, EvalContext *evalctx) const { - evalctx->applyScope(); + inst->scope.apply(*evalctx); 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 b8de1d17..35c6f092 100644 --- a/src/evalcontext.cc +++ b/src/evalcontext.cc @@ -37,22 +37,6 @@ 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 20600f76..d591c39f 100644 --- a/src/evalcontext.h +++ b/src/evalcontext.h @@ -22,8 +22,6 @@ 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/linearextrude.cc b/src/linearextrude.cc index 746b1a82..1e191f6b 100644 --- a/src/linearextrude.cc +++ b/src/linearextrude.cc @@ -58,7 +58,7 @@ AbstractNode *LinearExtrudeModule::instantiate(const Context *ctx, const ModuleI Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); 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 9c41035e..2c4185f2 100644 --- a/src/localscope.cc +++ b/src/localscope.cc @@ -54,3 +54,16 @@ std::vector LocalScope::instantiateChildren(const Context *evalct return childnodes; } +/*! + 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 LocalScope::apply(Context &ctx) const +{ + BOOST_FOREACH(const Assignment &ass, this->assignments) { + ctx.set_variable(ass.first, ass.second->evaluate(&ctx)); + } +} diff --git a/src/localscope.h b/src/localscope.h index e1b9dd6a..52808d6b 100644 --- a/src/localscope.h +++ b/src/localscope.h @@ -13,6 +13,7 @@ public: std::string dump(const std::string &indent) const; std::vector instantiateChildren(const class Context *evalctx) const; void addChild(ModuleInstantiation *ch); + void apply(Context &ctx) const; AssignmentList assignments; ModuleInstantiationList children; diff --git a/src/module.cc b/src/module.cc index 634d3573..44c2da6b 100644 --- a/src/module.cc +++ b/src/module.cc @@ -195,7 +195,7 @@ AbstractNode *Module::instantiate(const Context *ctx, const ModuleInstantiation // 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(); + inst->scope.apply(*evalctx); ModuleContext c(ctx, evalctx); // set $children first since we might have variables depending on it diff --git a/src/offset.cc b/src/offset.cc index 31d2ad96..faf3a38c 100644 --- a/src/offset.cc +++ b/src/offset.cc @@ -58,7 +58,7 @@ AbstractNode *OffsetModule::instantiate(const Context *ctx, const ModuleInstanti Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); node->fn = c.lookup_variable("$fn").toDouble(); node->fs = c.lookup_variable("$fs").toDouble(); diff --git a/src/projection.cc b/src/projection.cc index d27b28ce..57c1d1f9 100644 --- a/src/projection.cc +++ b/src/projection.cc @@ -53,7 +53,7 @@ AbstractNode *ProjectionModule::instantiate(const Context *ctx, const ModuleInst Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); 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 83dcbbae..4d109fbf 100644 --- a/src/render.cc +++ b/src/render.cc @@ -50,7 +50,7 @@ AbstractNode *RenderModule::instantiate(const Context *ctx, const ModuleInstanti Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); Value v = c.lookup_variable("convexity"); if (v.type() == Value::NUMBER) diff --git a/src/rotateextrude.cc b/src/rotateextrude.cc index 483b118e..b2e8d0eb 100644 --- a/src/rotateextrude.cc +++ b/src/rotateextrude.cc @@ -56,7 +56,7 @@ AbstractNode *RotateExtrudeModule::instantiate(const Context *ctx, const ModuleI Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); node->fn = c.lookup_variable("$fn").toDouble(); node->fs = c.lookup_variable("$fs").toDouble(); diff --git a/src/transform.cc b/src/transform.cc index 0d5506af..7259cd67 100644 --- a/src/transform.cc +++ b/src/transform.cc @@ -83,7 +83,7 @@ AbstractNode *TransformModule::instantiate(const Context *ctx, const ModuleInsta Context c(ctx); c.setVariables(args, evalctx); - evalctx->applyScope(); + inst->scope.apply(*evalctx); if (this->type == SCALE) { From 945be0020f0624d5018387de077cd54b2a38d069 Mon Sep 17 00:00:00 2001 From: Marius Kintel Date: Tue, 25 Nov 2014 00:45:19 -0500 Subject: [PATCH 8/8] Updated tests --- .../echotest/scope-assignment-tests-expected.echo | 7 +++++++ .../regression/echotest/variable-scope-tests-expected.echo | 1 + 2 files changed, 8 insertions(+) diff --git a/tests/regression/echotest/scope-assignment-tests-expected.echo b/tests/regression/echotest/scope-assignment-tests-expected.echo index 2b9f5def..5b457901 100644 --- a/tests/regression/echotest/scope-assignment-tests-expected.echo +++ b/tests/regression/echotest/scope-assignment-tests-expected.echo @@ -11,6 +11,10 @@ ECHO: "for loop (c = 0,1,25):" ECHO: "c", 0 ECHO: "c", 1 ECHO: "c", 25 +ECHO: "if scope:" +ECHO: "d (8)", 8 +ECHO: "else scope:" +ECHO: "d (9)", 9 ECHO: "anonymous inner scope (scope ignored):" ECHO: "outer e (3)", 3 ECHO: "inner e (3)", 3 @@ -21,3 +25,6 @@ ECHO: "anonymous scope reassign:" ECHO: "g (2)", 2 ECHO: "anonymous reassign using outer (scope ignored)", undef ECHO: "h (undef)", undef +ECHO: "override variable in assign scope:" +DEPRECATED: The assign() module will be removed in future releases. Use a regular assignment instead. +ECHO: "i (10)", 10 diff --git a/tests/regression/echotest/variable-scope-tests-expected.echo b/tests/regression/echotest/variable-scope-tests-expected.echo index 2a820905..7f18cf47 100644 --- a/tests/regression/echotest/variable-scope-tests-expected.echo +++ b/tests/regression/echotest/variable-scope-tests-expected.echo @@ -18,6 +18,7 @@ ECHO: "user-defined special variables as parameter" ECHO: 7 ECHO: 7 ECHO: "assign only visible in children's scope" +DEPRECATED: The assign() module will be removed in future releases. Use a regular assignment instead. WARNING: Ignoring unknown variable 'c'. ECHO: undef ECHO: 5