summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--examples/example017.scad2
-rw-r--r--src/ModuleCache.cc21
-rw-r--r--src/ModuleCache.h1
-rw-r--r--src/localscope.cc8
-rw-r--r--src/modcontext.cc74
-rw-r--r--src/modcontext.h3
-rw-r--r--src/module.cc16
-rw-r--r--src/module.h4
-rw-r--r--src/parser.y8
-rw-r--r--testdata/scad/misc/value-reassignment-tests.scad4
-rw-r--r--testdata/scad/misc/value-reassignment-tests2.scad9
-rw-r--r--tests/regression/echotest/value-reassignment-tests-expected.txt3
-rw-r--r--tests/regression/echotest/value-reassignment-tests2-expected.txt2
13 files changed, 114 insertions, 41 deletions
diff --git a/examples/example017.scad b/examples/example017.scad
index 9013d4e..4279546 100644
--- a/examples/example017.scad
+++ b/examples/example017.scad
@@ -1,6 +1,6 @@
// To render the DXF file from the command line:
-// openscad -x example017.dxf -D'mode="parts"' example017.scad
+// openscad -o example017.dxf -D'mode="parts"' example017.scad
// mode = "parts";
// mode = "exploded";
diff --git a/src/ModuleCache.cc b/src/ModuleCache.cc
index 83f7b7d..99c0b20 100644
--- a/src/ModuleCache.cc
+++ b/src/ModuleCache.cc
@@ -30,8 +30,14 @@ ModuleCache *ModuleCache::inst = NULL;
*/
FileModule *ModuleCache::evaluate(const std::string &filename)
{
+ FileModule *lib_mod = (this->entries.find(filename) != this->entries.end()) ?
+ &(*this->entries[filename].module) : NULL;
+
+ // Don't try to recursively evaluate - if the file changes
+ // during evaluation, that would be really bad.
+ if (lib_mod && lib_mod->isHandlingDependencies()) return lib_mod;
+
bool shouldCompile = true;
- FileModule *lib_mod = NULL;
// Create cache ID
struct stat st;
@@ -44,8 +50,7 @@ FileModule *ModuleCache::evaluate(const std::string &filename)
std::string cache_id = str(boost::format("%x.%x") % st.st_mtime % st.st_size);
// Lookup in cache
- if (this->entries.find(filename) != this->entries.end()) {
- lib_mod = &(*this->entries[filename].module);
+ if (lib_mod) {
if (this->entries[filename].cache_id == cache_id) {
shouldCompile = false;
@@ -104,7 +109,9 @@ FileModule *ModuleCache::evaluate(const std::string &filename)
print_messages_pop();
}
- if (lib_mod) lib_mod->handleDependencies();
+ if (lib_mod) {
+ lib_mod->handleDependencies();
+ }
return lib_mod;
}
@@ -114,3 +121,9 @@ void ModuleCache::clear()
this->entries.clear();
}
+FileModule *ModuleCache::lookup(const std::string &filename)
+{
+ return (this->entries.find(filename) != this->entries.end()) ?
+ &(*this->entries[filename].module) : NULL;
+}
+
diff --git a/src/ModuleCache.h b/src/ModuleCache.h
index b8ded38..7795ab7 100644
--- a/src/ModuleCache.h
+++ b/src/ModuleCache.h
@@ -9,6 +9,7 @@ class ModuleCache
public:
static ModuleCache *instance() { if (!inst) inst = new ModuleCache; return inst; }
class FileModule *evaluate(const std::string &filename);
+ class FileModule *lookup(const std::string &filename);
size_t size() { return this->entries.size(); }
void clear();
diff --git a/src/localscope.cc b/src/localscope.cc
index eecff91..dd40563 100644
--- a/src/localscope.cc
+++ b/src/localscope.cc
@@ -55,9 +55,11 @@ std::vector<AbstractNode*> LocalScope::instantiateChildren(const Context *evalct
// c->functions_p = &this->functions;
// c->modules_p = &this->modules;
- BOOST_FOREACH (const Assignment &ass, this->assignments) {
- c->set_variable(ass.first, ass.second->evaluate(c));
- }
+ // 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<AbstractNode*> childnodes;
diff --git a/src/modcontext.cc b/src/modcontext.cc
index 3b957fd..5b48009 100644
--- a/src/modcontext.cc
+++ b/src/modcontext.cc
@@ -4,6 +4,7 @@
#include "function.h"
#include "printutils.h"
#include "builtin.h"
+#include "ModuleCache.h"
#include <boost/foreach.hpp>
@@ -16,6 +17,51 @@ ModuleContext::~ModuleContext()
{
}
+// Experimental code. See issue #399
+#if 0
+void ModuleContext::evaluateAssignments(const AssignmentList &assignments)
+{
+ // First, assign all simple variables
+ std::list<std::string> undefined_vars;
+ BOOST_FOREACH(const Assignment &ass, assignments) {
+ Value tmpval = ass.second->evaluate(this);
+ if (tmpval.isUndefined()) undefined_vars.push_back(ass.first);
+ else this->set_variable(ass.first, tmpval);
+ }
+
+ // Variables which couldn't be evaluated in the first pass is attempted again,
+ // to allow for initialization out of order
+
+ boost::unordered_map<std::string, Expression *> tmpass;
+ BOOST_FOREACH (const Assignment &ass, assignments) {
+ tmpass[ass.first] = ass.second;
+ }
+
+ bool changed = true;
+ while (changed) {
+ changed = false;
+ std::list<std::string>::iterator iter = undefined_vars.begin();
+ while (iter != undefined_vars.end()) {
+ std::list<std::string>::iterator curr = iter++;
+ boost::unordered_map<std::string, Expression *>::iterator found = tmpass.find(*curr);
+ if (found != tmpass.end()) {
+ const Expression *expr = found->second;
+ Value tmpval = expr->evaluate(this);
+ // FIXME: it's not enough to check for undefined;
+ // we need to check for any undefined variable in the subexpression
+ // For now, ignore this and revisit the validity and order of variable
+ // assignments later
+ if (!tmpval.isUndefined()) {
+ changed = true;
+ this->set_variable(*curr, tmpval);
+ undefined_vars.erase(curr);
+ }
+ }
+ }
+ }
+}
+#endif
+
void ModuleContext::initializeModule(const class Module &module)
{
this->setVariables(module.definition_arguments, evalctx);
@@ -25,6 +71,8 @@ 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);
}
/*!
@@ -125,17 +173,18 @@ Value FileContext::evaluate_function(const std::string &name, const EvalContext
if (foundf) return foundf->evaluate(this, evalctx);
BOOST_FOREACH(const FileModule::ModuleContainer::value_type &m, this->usedlibs) {
- // m.second is NULL if the library wasn't be compiled (error or file-not-found)
- if (m.second &&
- m.second->scope.functions.find(name) != m.second->scope.functions.end()) {
- FileContext ctx(*m.second, this->parent);
- ctx.initializeModule(*m.second);
+ // usedmod is NULL if the library wasn't be compiled (error or file-not-found)
+ FileModule *usedmod = ModuleCache::instance()->lookup(m);
+ if (usedmod &&
+ usedmod->scope.functions.find(name) != usedmod->scope.functions.end()) {
+ FileContext ctx(*usedmod, this->parent);
+ ctx.initializeModule(*usedmod);
// FIXME: Set document path
#if 0 && DEBUG
PRINTB("New lib Context for %s func:", name);
ctx.dump(NULL, NULL);
#endif
- return m.second->scope.functions[name]->evaluate(&ctx, evalctx);
+ return usedmod->scope.functions[name]->evaluate(&ctx, evalctx);
}
}
@@ -148,17 +197,18 @@ AbstractNode *FileContext::instantiate_module(const ModuleInstantiation &inst, c
if (foundm) return foundm->instantiate(this, &inst, evalctx);
BOOST_FOREACH(const FileModule::ModuleContainer::value_type &m, this->usedlibs) {
- // m.second is NULL if the library wasn't be compiled (error or file-not-found)
- if (m.second &&
- m.second->scope.modules.find(inst.name()) != m.second->scope.modules.end()) {
- FileContext ctx(*m.second, this->parent);
- ctx.initializeModule(*m.second);
+ FileModule *usedmod = ModuleCache::instance()->lookup(m);
+ // usedmod is NULL if the library wasn't be compiled (error or file-not-found)
+ if (usedmod &&
+ usedmod->scope.modules.find(inst.name()) != usedmod->scope.modules.end()) {
+ FileContext ctx(*usedmod, this->parent);
+ ctx.initializeModule(*usedmod);
// FIXME: Set document path
#if 0 && DEBUG
PRINT("New file Context:");
ctx.dump(NULL, &inst);
#endif
- return m.second->scope.modules[inst.name()]->instantiate(&ctx, &inst, evalctx);
+ return usedmod->scope.modules[inst.name()]->instantiate(&ctx, &inst, evalctx);
}
}
diff --git a/src/modcontext.h b/src/modcontext.h
index 0b3e48c..3a05a0c 100644
--- a/src/modcontext.h
+++ b/src/modcontext.h
@@ -36,6 +36,9 @@ public:
#ifdef DEBUG
virtual void dump(const class AbstractModule *mod, const ModuleInstantiation *inst);
#endif
+private:
+// Experimental code. See issue #399
+// void evaluateAssignments(const AssignmentList &assignments);
};
class FileContext : public ModuleContext
diff --git a/src/module.cc b/src/module.cc
index 046d0c4..8fb8506 100644
--- a/src/module.cc
+++ b/src/module.cc
@@ -264,43 +264,35 @@ bool FileModule::handleDependencies()
// as it will have a relative path.
// Iterating manually since we want to modify the container while iterating
- std::vector<std::pair<std::string, FileModule*> > modified_modules;
FileModule::ModuleContainer::iterator iter = this->usedlibs.begin();
while (iter != this->usedlibs.end()) {
FileModule::ModuleContainer::iterator curr = iter++;
- FileModule *oldmodule = curr->second;
bool wasmissing = false;
// Get an absolute filename for the module
- std::string filename = curr->first;
+ std::string filename = *curr;
if (!boosty::is_absolute(filename)) {
wasmissing = true;
fs::path fullpath = find_valid_path(this->path, filename);
if (!fullpath.empty()) filename = boosty::stringy(fullpath);
}
+ FileModule *oldmodule = ModuleCache::instance()->lookup(filename);
FileModule *newmodule = ModuleCache::instance()->evaluate(filename);
// Detect appearance but not removal of files
if (newmodule && oldmodule != newmodule) {
changed = true;
#ifdef DEBUG
- PRINTB_NOCACHE(" %s: %p", filename % newmodule);
+ PRINTB_NOCACHE(" %s: %p -> %p", filename % oldmodule % newmodule);
#endif
}
- if (newmodule) {
- modified_modules.push_back(std::make_pair(filename, newmodule));
- this->usedlibs.erase(curr);
- }
- else {
+ if (!newmodule) {
// Only print warning if we're not part of an automatic reload
if (!oldmodule && !wasmissing) {
PRINTB_NOCACHE("WARNING: Failed to compile library '%s'.", filename);
}
}
}
- BOOST_FOREACH(const FileModule::ModuleContainer::value_type &mod, modified_modules) {
- this->usedlibs[mod.first] = mod.second;
- }
this->is_handling_dependencies = false;
return changed;
diff --git a/src/module.h b/src/module.h
index e28677a..6027fe6 100644
--- a/src/module.h
+++ b/src/module.h
@@ -5,6 +5,7 @@
#include <vector>
#include <list>
#include <boost/unordered_map.hpp>
+#include <boost/unordered_set.hpp>
#include <time.h>
#include <sys/stat.h>
@@ -94,8 +95,9 @@ public:
virtual AbstractNode *instantiate(const Context *ctx, const ModuleInstantiation *inst, const 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; }
- typedef boost::unordered_map<std::string, class FileModule*> ModuleContainer;
+ typedef boost::unordered_set<std::string> ModuleContainer;
ModuleContainer usedlibs;
private:
struct IncludeFile {
diff --git a/src/parser.y b/src/parser.y
index 2b07f14..5b3d019 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -127,7 +127,7 @@
input:
/* empty */ |
-TOK_USE { rootmodule->usedlibs[$1] = NULL; } input |
+TOK_USE { rootmodule->usedlibs.insert($1); } input |
statement input ;
inner_input:
@@ -136,15 +136,17 @@ statement inner_input ;
assignment:
TOK_ID '=' expr ';' {
+ bool found = false;
for (AssignmentList::iterator iter = scope_stack.top()->assignments.begin();
iter != scope_stack.top()->assignments.end();
iter++) {
if (iter->first == $1) {
- scope_stack.top()->assignments.erase(iter);
+ iter->second = $3;
+ found = true;
break;
}
}
- scope_stack.top()->assignments.push_back(Assignment($1, $3));
+ if (!found) scope_stack.top()->assignments.push_back(Assignment($1, $3));
} ;
statement:
diff --git a/testdata/scad/misc/value-reassignment-tests.scad b/testdata/scad/misc/value-reassignment-tests.scad
index 4370c11..26afa03 100644
--- a/testdata/scad/misc/value-reassignment-tests.scad
+++ b/testdata/scad/misc/value-reassignment-tests.scad
@@ -4,6 +4,6 @@
myval = 2;
i = 2;
-myval = i * 2;
-echo(myval, i); // Should output 4, 2
+myval = i * 2; // This is not (yet) allowed as it will be evaluates in place of the first assignment
+echo(myval, i); // Should output undef, 2
diff --git a/testdata/scad/misc/value-reassignment-tests2.scad b/testdata/scad/misc/value-reassignment-tests2.scad
index 9d7cf50..29a2fb7 100644
--- a/testdata/scad/misc/value-reassignment-tests2.scad
+++ b/testdata/scad/misc/value-reassignment-tests2.scad
@@ -5,5 +5,12 @@
myval = 2;
i = myval;
myval = 3;
-echo(myval, i); // Should output 3, 2
+echo(myval, i); // Should output 3, 3
+
+// NB! This feels wrong, but it's a simulation of what happens
+// when overriding a variable on the cmd-line: openscad -Dmyval=3 myfile.scad
+// Since the intention is to override a top-level variable, the evaluation of the
+// new expression must be done in the same place as the old.
+// This is currently solved by appending the text given to the -D parameter to the end
+// of the main file.
diff --git a/tests/regression/echotest/value-reassignment-tests-expected.txt b/tests/regression/echotest/value-reassignment-tests-expected.txt
index fc7b7b1..344f7ab 100644
--- a/tests/regression/echotest/value-reassignment-tests-expected.txt
+++ b/tests/regression/echotest/value-reassignment-tests-expected.txt
@@ -1 +1,2 @@
-ECHO: 4, 2
+WARNING: Ignoring unknown variable 'i'.
+ECHO: undef, 2
diff --git a/tests/regression/echotest/value-reassignment-tests2-expected.txt b/tests/regression/echotest/value-reassignment-tests2-expected.txt
index 99b37a2..efb1be7 100644
--- a/tests/regression/echotest/value-reassignment-tests2-expected.txt
+++ b/tests/regression/echotest/value-reassignment-tests2-expected.txt
@@ -1 +1 @@
-ECHO: 3, 2
+ECHO: 3, 3
contact: Jan Huwald // Impressum