From 2169f0e960c6e2b94518c03e6eb0406908b96e65 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 27 Jan 2020 09:07:09 +0200 Subject: Improve module loading API --- build2/cli/init.cxx | 2 +- libbuild2/bash/init.cxx | 3 +-- libbuild2/bin/init.cxx | 38 ++++++++++--------------------- libbuild2/c/init.cxx | 14 +++++------- libbuild2/cc/compile-rule.cxx | 2 +- libbuild2/cc/init.cxx | 51 +++++++++++++----------------------------- libbuild2/config/operation.cxx | 4 ++-- libbuild2/config/utility.cxx | 4 ++-- libbuild2/cxx/init.cxx | 14 +++++------- libbuild2/dist/operation.cxx | 4 ++-- libbuild2/file.cxx | 4 ++-- libbuild2/in/init.cxx | 3 +-- libbuild2/module.cxx | 40 +++++++++++++++++++++++++++++++-- libbuild2/module.hxx | 43 ++++++++++++++++++++++++----------- libbuild2/parser.cxx | 2 +- libbuild2/version/init.cxx | 8 +++---- libbuild2/version/rule.cxx | 4 ++-- 17 files changed, 126 insertions(+), 114 deletions(-) diff --git a/build2/cli/init.cxx b/build2/cli/init.cxx index 67aad29..89da97a 100644 --- a/build2/cli/init.cxx +++ b/build2/cli/init.cxx @@ -320,7 +320,7 @@ namespace build2 // if (!cast_false (bs["cli.config.loaded"])) { - if (!load_module (rs, bs, "cli.config", l, optional, hints)) + if (!init_module (rs, bs, "cli.config", l, optional, hints)) return false; } else if (!cast_false (bs["cli.config.configured"])) diff --git a/libbuild2/bash/init.cxx b/libbuild2/bash/init.cxx index 65d9d0f..dc1f8f6 100644 --- a/libbuild2/bash/init.cxx +++ b/libbuild2/bash/init.cxx @@ -37,8 +37,7 @@ namespace build2 // Load in.base (in.* variables, in{} target type). // - if (!cast_false (rs["in.base.loaded"])) - load_module (rs, rs, "in.base", l); + load_module (rs, rs, "in.base", l); bool install_loaded (cast_false (rs["install.loaded"])); diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx index 1e5ccc5..344d54e 100644 --- a/libbuild2/bin/init.cxx +++ b/libbuild2/bin/init.cxx @@ -161,8 +161,7 @@ namespace build2 // Load bin.vars. // - if (!cast_false (rs["bin.vars.loaded"])) - load_module (rs, rs, "bin.vars", loc); + load_module (rs, rs, "bin.vars", loc); // Configure. // @@ -423,8 +422,7 @@ namespace build2 // Load bin.config. // - if (!cast_false (rs["bin.config.loaded"])) - load_module (rs, rs, "bin.config", loc, false, hints); + load_module (rs, rs, "bin.config", loc, hints); // Cache some config values we will be needing below. // @@ -551,7 +549,7 @@ namespace build2 r.insert (perform_uninstall_id, "bin.lib", gr); } - if (const test::module* m = rs.lookup_module ("test")) + if (const test::module* m = rs.find_module ("test")) { r.insert (perform_test_id, "bin.lib", m->group_rule ()); } @@ -574,8 +572,7 @@ namespace build2 // Make sure bin.config is loaded. // - if (!cast_false (rs["bin.config.loaded"])) - load_module (rs, bs, "bin.config", loc, false, hints); + load_module (rs, bs, "bin.config", loc, hints); // Enter configuration variables. // @@ -723,11 +720,8 @@ namespace build2 // Make sure the bin core and ar.config are loaded. // - if (!cast_false (bs["bin.loaded"])) - load_module (rs, bs, "bin", loc, false, hints); - - if (!cast_false (bs["bin.ar.config.loaded"])) - load_module (rs, bs, "bin.ar.config", loc, false, hints); + load_module (rs, bs, "bin", loc, hints); + load_module (rs, bs, "bin.ar.config", loc, hints); return true; } @@ -746,8 +740,7 @@ namespace build2 // Make sure bin.config is loaded. // - if (!cast_false (rs["bin.config.loaded"])) - load_module (rs, rs, "bin.config", loc, false, hints); + load_module (rs, rs, "bin.config", loc, hints); // Enter configuration variables. // @@ -820,11 +813,8 @@ namespace build2 // Make sure the bin core and ld.config are loaded. // - if (!cast_false (bs["bin.loaded"])) - load_module (rs, bs, "bin", loc, false, hints); - - if (!cast_false (bs["bin.ld.config.loaded"])) - load_module (rs, bs, "bin.ld.config", loc, false, hints); + load_module (rs, bs, "bin", loc, hints); + load_module (rs, bs, "bin.ld.config", loc, hints); const string& lid (cast (rs["bin.ld.id"])); @@ -856,8 +846,7 @@ namespace build2 // Make sure bin.config is loaded. // - if (!cast_false (bs["bin.config.loaded"])) - load_module (rs, bs, "bin.config", loc, false, hints); + load_module (rs, bs, "bin.config", loc, hints); // Enter configuration variables. // @@ -930,11 +919,8 @@ namespace build2 // Make sure the bin core and rc.config are loaded. // - if (!cast_false (bs["bin.loaded"])) - load_module (rs, bs, "bin", loc, false, hints); - - if (!cast_false (bs["bin.rc.config.loaded"])) - load_module (rs, bs, "bin.rc.config", loc, false, hints); + load_module (rs, bs, "bin", loc, hints); + load_module (rs, bs, "bin.rc.config", loc, hints); return true; } diff --git a/libbuild2/c/init.cxx b/libbuild2/c/init.cxx index 423e6af..4efba69 100644 --- a/libbuild2/c/init.cxx +++ b/libbuild2/c/init.cxx @@ -146,8 +146,7 @@ namespace build2 // Load cc.core.vars so that we can cache all the cc.* variables. // - if (!cast_false (rs["cc.core.vars.loaded"])) - load_module (rs, rs, "cc.core.vars", loc); + load_module (rs, rs, "cc.core.vars", loc); // Enter all the variables and initialize the module data. // @@ -285,10 +284,8 @@ namespace build2 // Load c.guess. // - if (!cast_false (rs["c.guess.loaded"])) - load_module (rs, rs, "c.guess", loc, false, hints); + auto& cm (load_module (rs, rs, "c.guess", loc, hints)); - config_module& cm (*rs.lookup_module ("c.guess")); cm.init (rs, loc, hints); return true; } @@ -325,10 +322,11 @@ namespace build2 // Load c.config. // - if (!cast_false (rs["c.config.loaded"])) - load_module (rs, rs, "c.config", loc, false, hints); + // @@ TODO: move guess to config and use return value? + // + load_module (rs, rs, "c.config", loc, hints); - config_module& cm (*rs.lookup_module ("c.guess")); + config_module& cm (*rs.find_module ("c.guess")); cc::data d { cm, diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 328b65e..de35ad5 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -5201,7 +5201,7 @@ namespace build2 // #ifndef NDEBUG assert (ps->root ()); - const module* m (ps->lookup_module (x)); + const module* m (ps->find_module (x)); assert (m != nullptr && m->modules); #endif diff --git a/libbuild2/cc/init.cxx b/libbuild2/cc/init.cxx index d310c6b..a495519 100644 --- a/libbuild2/cc/init.cxx +++ b/libbuild2/cc/init.cxx @@ -74,8 +74,7 @@ namespace build2 // Load bin.vars (we need its config.bin.target/pattern for hints). // - if (!cast_false (rs["bin.vars.loaded"])) - load_module (rs, rs, "bin.vars", loc); + load_module (rs, rs, "bin.vars", loc); // Enter variables. Note: some overridable, some not. // @@ -172,8 +171,7 @@ namespace build2 // Load cc.core.vars. // - if (!cast_false (rs["cc.core.vars.loaded"])) - load_module (rs, rs, "cc.core.vars", loc); + load_module (rs, rs, "cc.core.vars", loc); // config.cc.{id,hinter} // @@ -246,8 +244,7 @@ namespace build2 // Load cc.core.guess. // - if (!cast_false (rs["cc.core.guess.loaded"])) - load_module (rs, rs, "cc.core.guess", loc); + load_module (rs, rs, "cc.core.guess", loc); // Configure. // @@ -303,7 +300,7 @@ namespace build2 h.assign ("config.bin.pattern") = cast (l); } - load_module (rs, rs, "bin.config", loc, false, h); + init_module (rs, rs, "bin.config", loc, false /* optional */, h); } // Verify bin's target matches ours (we do it even if we loaded it @@ -330,20 +327,13 @@ namespace build2 // const string& tsys (cast (rs["cc.target.system"])); - if (!cast_false (rs["bin.ar.config.loaded"])) - load_module (rs, rs, "bin.ar.config", loc); + load_module (rs, rs, "bin.ar.config", loc); if (tsys == "win32-msvc") - { - if (!cast_false (rs["bin.ld.config.loaded"])) - load_module (rs, rs, "bin.ld.config", loc); - } + load_module (rs, rs, "bin.ld.config", loc); if (tsys == "mingw32") - { - if (!cast_false (rs["bin.rc.config.loaded"])) - load_module (rs, rs, "bin.rc.config", loc); - } + load_module (rs, rs, "bin.rc.config", loc); return true; } @@ -366,36 +356,27 @@ namespace build2 // Load cc.core.config. // - if (!cast_false (rs["cc.core.config.loaded"])) - load_module (rs, rs, "cc.core.config", loc, false, hints); + load_module (rs, rs, "cc.core.config", loc, hints); // Load the bin module. // - if (!cast_false (rs["bin.loaded"])) - load_module (rs, rs, "bin", loc); + load_module (rs, rs, "bin", loc); // Load the bin.ar module. // - if (!cast_false (rs["bin.ar.loaded"])) - load_module (rs, rs, "bin.ar", loc); + load_module (rs, rs, "bin.ar", loc); // For this target we link things directly with link.exe so load the // bin.ld module. // if (tsys == "win32-msvc") - { - if (!cast_false (rs["bin.ld.loaded"])) - load_module (rs, rs, "bin.ld", loc); - } + load_module (rs, rs, "bin.ld", loc); // If our target is MinGW, then we will need the resource compiler // (windres) in order to embed manifests into executables. // if (tsys == "mingw32") - { - if (!cast_false (rs["bin.rc.loaded"])) - load_module (rs, rs, "bin.rc", loc); - } + load_module (rs, rs, "bin.rc", loc); return true; } @@ -436,13 +417,13 @@ namespace build2 // if (lc && lp && rs["config.c"]) { - load_module (rs, rs, c, loc, false, hints); - load_module (rs, rs, cxx, loc, false, hints); + init_module (rs, rs, c, loc, false /* optional */, hints); + init_module (rs, rs, cxx, loc, false /* optional */, hints); } else { - if (lp) load_module (rs, rs, cxx, loc, false, hints); - if (lc) load_module (rs, rs, c, loc, false, hints); + if (lp) init_module (rs, rs, cxx, loc, false, hints); + if (lc) init_module (rs, rs, c, loc, false, hints); } return true; diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx index 9f241e8..81b99be 100644 --- a/libbuild2/config/operation.cxx +++ b/libbuild2/config/operation.cxx @@ -172,7 +172,7 @@ namespace build2 // saved) and this function can be called from a buildfile (probably // only during serial execution but still). // - module* mod (rs.lookup_module (module::name)); + module* mod (rs.find_module (module::name)); if (mod == nullptr) fail (on) << "no configuration information available during this " @@ -336,7 +336,7 @@ namespace build2 { // Find the config module (might not be there). // - if (auto* m = r->lookup_module (module::name)) + if (auto* m = r->find_module (module::name)) { // Find the corresponding saved module. // diff --git a/libbuild2/config/utility.cxx b/libbuild2/config/utility.cxx index 291bf0c..34220f8 100644 --- a/libbuild2/config/utility.cxx +++ b/libbuild2/config/utility.cxx @@ -132,14 +132,14 @@ namespace build2 void save_variable (scope& rs, const variable& var, uint64_t flags) { - if (module* m = rs.lookup_module (module::name)) + if (module* m = rs.find_module (module::name)) m->save_variable (var, flags); } void save_module (scope& rs, const char* name, int prio) { - if (module* m = rs.lookup_module (module::name)) + if (module* m = rs.find_module (module::name)) m->save_module (name, prio); } diff --git a/libbuild2/cxx/init.cxx b/libbuild2/cxx/init.cxx index 8f404ea..d2601c4 100644 --- a/libbuild2/cxx/init.cxx +++ b/libbuild2/cxx/init.cxx @@ -369,8 +369,7 @@ namespace build2 // Load cc.core.vars so that we can cache all the cc.* variables. // - if (!cast_false (rs["cc.core.vars.loaded"])) - load_module (rs, rs, "cc.core.vars", loc); + load_module (rs, rs, "cc.core.vars", loc); // Enter all the variables and initialize the module data. // @@ -532,10 +531,8 @@ namespace build2 // Load cxx.guess. // - if (!cast_false (rs["cxx.guess.loaded"])) - load_module (rs, rs, "cxx.guess", loc, false, hints); + auto& cm (load_module (rs, rs, "cxx.guess", loc, hints)); - config_module& cm (*rs.lookup_module ("cxx.guess")); cm.init (rs, loc, hints); return true; } @@ -580,10 +577,11 @@ namespace build2 // Load cxx.config. // - if (!cast_false (rs["cxx.config.loaded"])) - load_module (rs, rs, "cxx.config", loc, false, hints); + // @@ TODO: move guess to config and use return value? + // + load_module (rs, rs, "cxx.config", loc, hints); - config_module& cm (*rs.lookup_module ("cxx.guess")); + config_module& cm (*rs.find_module ("cxx.guess")); auto& vp (rs.ctx.var_pool.rw (rs)); diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx index 12ae464..90be2c0 100644 --- a/libbuild2/dist/operation.cxx +++ b/libbuild2/dist/operation.cxx @@ -346,7 +346,7 @@ namespace build2 // Copy over all the files. Apply post-processing callbacks. // - module& mod (*rs->lookup_module (module::name)); + module& mod (*rs->find_module (module::name)); prog = prog && show_progress (1 /* max_verb */); size_t prog_percent (0); @@ -380,7 +380,7 @@ namespace build2 { srs = &ctx.scopes.find (out_root / pd); - if (auto* m = srs->lookup_module (module::name)) + if (auto* m = srs->find_module (module::name)) cbs = &m->callbacks_; else fail << "dist module not loaded in subproject " << pd; diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 8d59d7c..be85745 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -1245,7 +1245,7 @@ namespace build2 module_state& s (p.second); if (s.boot && s.first) - load_module (root, root, p.first, s.loc); + init_module (root, root, p.first, s.loc); } for (auto& p: root.root_extra->modules) @@ -1253,7 +1253,7 @@ namespace build2 module_state& s (p.second); if (s.boot && !s.first) - load_module (root, root, p.first, s.loc); + init_module (root, root, p.first, s.loc); } // Load hooks and root.build. diff --git a/libbuild2/in/init.cxx b/libbuild2/in/init.cxx index ece1bcf..95bdb0a 100644 --- a/libbuild2/in/init.cxx +++ b/libbuild2/in/init.cxx @@ -84,8 +84,7 @@ namespace build2 // Load in.base. // - if (!cast_false (rs["in.base.loaded"])) - load_module (rs, rs, "in.base", loc); + load_module (rs, rs, "in.base", loc); // Register rules. // diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx index eec770b..9bc79ab 100644 --- a/libbuild2/module.cxx +++ b/libbuild2/module.cxx @@ -526,7 +526,7 @@ namespace build2 rs.assign (rs.ctx.var_pool.rw (rs).insert (mod + ".booted")) = true; } - bool + module_state* init_module (scope& rs, scope& bs, const string& mod, @@ -614,6 +614,42 @@ namespace build2 cv = c; } - return l && c; + return l && c ? &i->second : nullptr; + } + + // @@ TODO: This is a bit of a fuzzy mess: + // + // - The .loaded variable check: it's not clear if init_module() + // already has this semantics? + // + // - Why do we use variable instead of the module map entry? Probably + // because of optional. Also entry present if only booted. Need to be + // careful here. Also root vs base! + // + // Note that it would have been nice to keep these inline but we need the + // definition of scope for the variable lookup. + // + bool + load_module (scope& rs, + scope& bs, + const string& name, + const location& loc, + bool opt, + const variable_map& hints) + { + return cast_false (bs[name + ".loaded"]) || + init_module (rs, bs, name, loc, opt, hints); + } + + unique_ptr& + load_module (scope& rs, + scope& bs, + const string& name, + const location& loc, + const variable_map& hints) + { + return cast_false (bs[name + ".loaded"]) + ? rs.root_extra->modules.find (name)->second.module + : init_module (rs, bs, name, loc, false /* optional */, hints)->module; } } diff --git a/libbuild2/module.hxx b/libbuild2/module.hxx index 34c7245..729cc2e 100644 --- a/libbuild2/module.hxx +++ b/libbuild2/module.hxx @@ -96,7 +96,7 @@ namespace build2 { template T* - lookup (const string& name) const + find_module (const string& name) const { auto i (find (name)); return i != end () @@ -111,9 +111,10 @@ namespace build2 boot_module (scope& root, const string& name, const location&); // Init the specified module loading its library if necessary. Used by the - // parser but also by some modules to init prerequisite modules. Return true - // if the module was both successfully loaded and configured (false can only - // be returned if optional is true). + // parser but also by some modules to init prerequisite modules. Return a + // pointer to the corresponding module state if the module was both + // successfully loaded and configured and NULL otherwise (which can only + // happen if optional is true). Note that the return can be used as a bool. // // The config_hints variable map can be used to pass configuration hints // from one module to another. For example, the cxx modude may pass the @@ -121,7 +122,7 @@ namespace build2 // module (which may not always be able to extract the same information from // its tools). // - LIBBUILD2_SYMEXPORT bool + LIBBUILD2_SYMEXPORT module_state* init_module (scope& root, scope& base, const string& name, @@ -129,21 +130,37 @@ namespace build2 bool optional = false, const variable_map& config_hints = empty_variable_map); - // An alias to use from other modules (we could also distinguish between - // boot and init). + // A wrapper over init_module() to use from other modules that incorporates + // the .loaded variable check (use init_module() directly to sidestep + // this check). // - // @@ TODO: maybe incorporate the .loaded variable check we have all over - // (it's not clear if init_module() already has this semantics)? + bool + load_module (scope& root, + scope& base, + const string& name, + const location&, + bool optional, + const variable_map& config_hints = empty_variable_map); + + // As above but always load and return a reference to the module instance + // pointer (so it can be moved). // - inline bool + unique_ptr& load_module (scope& root, scope& base, const string& name, - const location& loc, - bool optional = false, + const location&, + const variable_map& config_hints = empty_variable_map); + + template + inline T& + load_module (scope& root, + scope& base, + const string& name, + const location& l, const variable_map& config_hints = empty_variable_map) { - return init_module (root, base, name, loc, optional, config_hints); + return static_cast (*load_module (root, base, name, l, config_hints)); } // Loaded modules (as in libraries). diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 59eeafd..2631845 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -1905,7 +1905,7 @@ namespace build2 if (boot_) boot_module (*root_, n, l); else - load_module (*root_, *scope_, n, l, optional); + init_module (*root_, *scope_, n, l, optional); } } diff --git a/libbuild2/version/init.cxx b/libbuild2/version/init.cxx index 9a56756..3ab3621 100644 --- a/libbuild2/version/init.cxx +++ b/libbuild2/version/init.cxx @@ -303,8 +303,7 @@ namespace build2 // Load in.base (in.* variables, in{} target type). // - if (!cast_false (rs["in.base.loaded"])) - load_module (rs, rs, "in.base", l); + load_module (rs, rs, "in.base", l); module& m (static_cast (*mod)); const standard_version& v (m.version); @@ -312,12 +311,11 @@ namespace build2 // If the dist module is used, set its dist.package and register the // post-processing callback. // - if (auto* dm = rs.lookup_module (dist::module::name)) + if (auto* dm = rs.find_module (dist::module::name)) { // Make sure dist is init'ed, not just boot'ed. // - if (!cast_false (rs["dist.loaded"])) - load_module (rs, rs, "dist", l); + load_module (rs, rs, "dist", l); m.dist_uncommitted = cast_false (rs["config.dist.uncommitted"]); diff --git a/libbuild2/version/rule.cxx b/libbuild2/version/rule.cxx index fe999b3..463b6e0 100644 --- a/libbuild2/version/rule.cxx +++ b/libbuild2/version/rule.cxx @@ -80,7 +80,7 @@ namespace build2 // If we match, lookup and cache the module for the update operation. // if (r && a == perform_update_id) - t.data (rs.lookup_module (module::name)); + t.data (rs.find_module (module::name)); return r; } @@ -319,7 +319,7 @@ namespace build2 const path& p (t.path ()); const scope& rs (t.root_scope ()); - const module& m (*rs.lookup_module (module::name)); + const module& m (*rs.find_module (module::name)); if (!m.rewritten) return auto_rmfile (p, false /* active */); -- cgit v1.1