diff options
| -rw-r--r-- | examples/example018.scad | 2 | ||||
| -rw-r--r-- | src/ModuleCache.cc | 98 | ||||
| -rw-r--r-- | src/ModuleCache.h | 3 | ||||
| -rw-r--r-- | src/control.cc | 1 | ||||
| -rw-r--r-- | src/module.cc | 49 | ||||
| -rw-r--r-- | testdata/modulecache-tests/README.txt | 19 | ||||
| -rw-r--r-- | testdata/modulecache-tests/mainsubsub.scad | 2 | ||||
| -rw-r--r-- | testdata/modulecache-tests/mainusingerror.scad | 3 | ||||
| -rw-r--r-- | testdata/modulecache-tests/moduleoverload.scad | 2 | ||||
| -rw-r--r-- | testdata/modulecache-tests/mymodule-lib.scad | 2 | ||||
| -rw-r--r-- | testdata/modulecache-tests/subdir/sub.scad | 5 | ||||
| -rw-r--r-- | testdata/modulecache-tests/subdir/subsub.scad | 3 |
12 files changed, 124 insertions, 65 deletions
diff --git a/examples/example018.scad b/examples/example018.scad index 15ed745..8cd7fa5 100644 --- a/examples/example018.scad +++ b/examples/example018.scad @@ -2,7 +2,7 @@ module step(len, mod) { for (i = [0:$children-1]) - translate([ len*(i - ($children-1)/2), 0, 0 ]) child((i+mod) % $children); + translate([ len*(i - ($children-1)/2), 0, 0 ]) children((i+mod) % $children); } for (i = [1:4]) diff --git a/src/ModuleCache.cc b/src/ModuleCache.cc index 1b7a9e5..d79a026 100644 --- a/src/ModuleCache.cc +++ b/src/ModuleCache.cc @@ -22,51 +22,68 @@ ModuleCache *ModuleCache::inst = NULL; /*! - Reevaluate the given file and recompile if necessary. - Returns NULL on any error (e.g. compile error or file not found) + Reevaluate the given file and all it's dependencies and recompile anything + needing reevaluation. + The given filename must be absolute. - If the given filename is relative, it means that the module hasn't been - previously located. + Sets the module reference to the new modile, or NULL on any error (e.g. compile + error or file not found) + + Returns true if anything was compiled (module or dependencies) and false otherwise. */ -FileModule *ModuleCache::evaluate(const std::string &filename) +bool ModuleCache::evaluate(const std::string &filename, FileModule *&module) { - FileModule *lib_mod = (this->entries.find(filename) != this->entries.end()) ? - &(*this->entries[filename].module) : NULL; - + FileModule *lib_mod = NULL; + bool found = false; + if (this->entries.find(filename) != this->entries.end()) { + found = true; + lib_mod = this->entries[filename].module; + } + // 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; - // Create cache ID struct stat st; memset(&st, 0, sizeof(struct stat)); bool valid = (stat(filename.c_str(), &st) == 0); - // If file isn't there, just return and let the cache retain the old module + // If file isn't there, just return and let the cache retain the old module if (!valid) return NULL; - + + // If the file is present, we'll always cache some result std::string cache_id = str(boost::format("%x.%x") % st.st_mtime % st.st_size); - // Lookup in cache - if (lib_mod) { - if (this->entries[filename].cache_id == cache_id) { + cache_entry &entry = this->entries[filename]; + // Initialize entry, if new + if (!found) { + entry.module = NULL; + entry.cache_id = cache_id; + } + + bool shouldCompile = true; + if (found) { + // Files should only be recompiled if the cache ID changed + if (entry.cache_id == cache_id) { shouldCompile = false; - if (lib_mod->includesChanged()) { + // Recompile if includes changed + if (lib_mod && lib_mod->includesChanged()) { lib_mod = NULL; shouldCompile = true; } } } - else { - shouldCompile = valid; - } + +#ifdef DEBUG + // Causes too much debug output + //if (!shouldCompile) PRINTB("Using cached library: %s (%p)", filename % lib_mod); +#endif // If cache lookup failed (non-existing or old timestamp), compile module if (shouldCompile) { #ifdef DEBUG - if (this->entries.find(filename) != this->entries.end()) { + if (found) { PRINTB("Recompiling cached library: %s (%s)", filename % cache_id); } else { @@ -84,37 +101,28 @@ FileModule *ModuleCache::evaluate(const std::string &filename) textbuf << ifs.rdbuf(); } textbuf << "\n" << commandline_commands; - + print_messages_push(); - - FileModule *oldmodule = NULL; - cache_entry e = { NULL, cache_id }; - if (this->entries.find(filename) != this->entries.end()) { - oldmodule = this->entries[filename].module; - } - this->entries[filename] = e; + + FileModule *oldmodule = lib_mod; std::string pathname = boosty::stringy(fs::path(filename).parent_path()); lib_mod = dynamic_cast<FileModule*>(parse(textbuf.str().c_str(), pathname.c_str(), false)); PRINTB_NOCACHE(" compiled module: %p", lib_mod); - if (lib_mod) { - // We defer deletion so we can ensure that the new module won't - // have the same address as the old - delete oldmodule; - this->entries[filename].module = lib_mod; - } else { - this->entries.erase(filename); - } + // We defer deletion so we can ensure that the new module won't + // have the same address as the old + if (oldmodule) delete oldmodule; + entry.module = lib_mod; + entry.cache_id = cache_id; print_messages_pop(); } + + module = lib_mod; + bool depschanged = lib_mod ? lib_mod->handleDependencies() : false; - if (lib_mod) { - lib_mod->handleDependencies(); - } - - return lib_mod; + return shouldCompile || depschanged; } void ModuleCache::clear() @@ -124,7 +132,11 @@ void ModuleCache::clear() FileModule *ModuleCache::lookup(const std::string &filename) { - return (this->entries.find(filename) != this->entries.end()) ? - &(*this->entries[filename].module) : NULL; + return isCached(filename) ? this->entries[filename].module : NULL; +} + +bool ModuleCache::isCached(const std::string &filename) +{ + return this->entries.find(filename) != this->entries.end(); } diff --git a/src/ModuleCache.h b/src/ModuleCache.h index 7795ab7..0c15fbd 100644 --- a/src/ModuleCache.h +++ b/src/ModuleCache.h @@ -8,8 +8,9 @@ class ModuleCache { public: static ModuleCache *instance() { if (!inst) inst = new ModuleCache; return inst; } - class FileModule *evaluate(const std::string &filename); + bool evaluate(const std::string &filename, class FileModule *&module); class FileModule *lookup(const std::string &filename); + bool isCached(const std::string &filename); size_t size() { return this->entries.size(); } void clear(); diff --git a/src/control.cc b/src/control.cc index d5f664e..1b58da1 100644 --- a/src/control.cc +++ b/src/control.cc @@ -161,6 +161,7 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns if (type == CHILD) { + PRINT("DEPRECATED: child() will be removed in future releases. Use children() instead."); int n = 0; if (evalctx->numArgs() > 0) { double v; diff --git a/src/module.cc b/src/module.cc index 1f4059e..ccc475e 100644 --- a/src/module.cc +++ b/src/module.cc @@ -263,45 +263,58 @@ bool FileModule::handleDependencies() if (this->is_handling_dependencies) return false; this->is_handling_dependencies = true; - bool changed = false; + bool somethingchanged = false; + std::vector<std::pair<std::string,std::string> > updates; // If a lib in usedlibs was previously missing, we need to relocate it // by searching the applicable paths. We can identify a previously missing module // as it will have a relative path. - - // Iterating manually since we want to modify the container while iterating - FileModule::ModuleContainer::iterator iter = this->usedlibs.begin(); - while (iter != this->usedlibs.end()) { - FileModule::ModuleContainer::iterator curr = iter++; + BOOST_FOREACH(std::string filename, this->usedlibs) { bool wasmissing = false; + bool found = true; + // Get an absolute filename for the module - 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); + if (!fullpath.empty()) { + updates.push_back(std::make_pair(filename, boosty::stringy(fullpath))); + filename = boosty::stringy(fullpath); + } + else { + found = false; + } } - 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; + if (found) { + bool wascached = ModuleCache::instance()->isCached(filename); + FileModule *oldmodule = ModuleCache::instance()->lookup(filename); + FileModule *newmodule; + bool changed = ModuleCache::instance()->evaluate(filename, newmodule); + // Detect appearance but not removal of files, and keep old module + // on compile errors (FIXME: Is this correct behavior?) + if (changed) { #ifdef DEBUG - PRINTB_NOCACHE(" %s: %p -> %p", filename % oldmodule % newmodule); + PRINTB_NOCACHE(" %s: %p -> %p", filename % oldmodule % newmodule); #endif - } - if (!newmodule) { + } + somethingchanged |= changed; // Only print warning if we're not part of an automatic reload - if (!oldmodule && !wasmissing) { + if (!newmodule && !wascached && !wasmissing) { PRINTB_NOCACHE("WARNING: Failed to compile library '%s'.", filename); } } } + // Relative filenames which were located is reinserted as absolute filenames + typedef std::pair<std::string,std::string> stringpair; + BOOST_FOREACH(const stringpair &files, updates) { + this->usedlibs.erase(files.first); + this->usedlibs.insert(files.second); + } this->is_handling_dependencies = false; - return changed; + return somethingchanged; } AbstractNode *FileModule::instantiate(const Context *ctx, const ModuleInstantiation *inst, const EvalContext *evalctx) const diff --git a/testdata/modulecache-tests/README.txt b/testdata/modulecache-tests/README.txt index 214acc5..3a123e7 100644 --- a/testdata/modulecache-tests/README.txt +++ b/testdata/modulecache-tests/README.txt @@ -124,3 +124,22 @@ o rm cascade*.scad o Verify that no rerendering was triggered (the 4 objects are still there) o ./cascade2.sh o Verify that everything reloads at once without flickering + +Test 15: Correct handling of compile errors in auto-reloaded modules +-------- +o Turn on Automatic Reload and Compile +o Open mainusingerror.scad +o Verify that you get: + - Compiling library '.../error.scad'. + - Parser error in line 3: syntax error + - WARNING: Failed to compile library '.../error.scad'. + - Main file should keep compiling +o Verify that the above doesn't repeat + +Test 16: Dependency tracking of underlying dependencies +-------- +o Turn on Automatic Reload and Compile +o Open mainsubsub.scad +o Verify that you see a red cylinder +o edit subdit/subsub.scad: Change color +o Verify that color changes diff --git a/testdata/modulecache-tests/mainsubsub.scad b/testdata/modulecache-tests/mainsubsub.scad new file mode 100644 index 0000000..b87af98 --- /dev/null +++ b/testdata/modulecache-tests/mainsubsub.scad @@ -0,0 +1,2 @@ +use <subdir/sub.scad> +sub(); diff --git a/testdata/modulecache-tests/mainusingerror.scad b/testdata/modulecache-tests/mainusingerror.scad new file mode 100644 index 0000000..626f4aa --- /dev/null +++ b/testdata/modulecache-tests/mainusingerror.scad @@ -0,0 +1,3 @@ +//mainusingerror.scad +echo(version()); +use <error.scad> diff --git a/testdata/modulecache-tests/moduleoverload.scad b/testdata/modulecache-tests/moduleoverload.scad index 1928715..f8ebc6a 100644 --- a/testdata/modulecache-tests/moduleoverload.scad +++ b/testdata/modulecache-tests/moduleoverload.scad @@ -1,7 +1,7 @@ use <mymodule-lib.scad> module mymodule() { - sphere(); + sphere(5); } mymodule(); diff --git a/testdata/modulecache-tests/mymodule-lib.scad b/testdata/modulecache-tests/mymodule-lib.scad index 9d68581..c6f30f7 100644 --- a/testdata/modulecache-tests/mymodule-lib.scad +++ b/testdata/modulecache-tests/mymodule-lib.scad @@ -1,3 +1,3 @@ module mymodule() { - cylinder(center=true); + cylinder(r=5, center=true); } diff --git a/testdata/modulecache-tests/subdir/sub.scad b/testdata/modulecache-tests/subdir/sub.scad new file mode 100644 index 0000000..bf0fc44 --- /dev/null +++ b/testdata/modulecache-tests/subdir/sub.scad @@ -0,0 +1,5 @@ +use <subsub.scad> + +module sub() { + subsub(); +} diff --git a/testdata/modulecache-tests/subdir/subsub.scad b/testdata/modulecache-tests/subdir/subsub.scad new file mode 100644 index 0000000..c79d608 --- /dev/null +++ b/testdata/modulecache-tests/subdir/subsub.scad @@ -0,0 +1,3 @@ +module subsub() { + color("red") cylinder(r = 10, h = 5); +} |