From e94354ceef93f45b0a95f35eee62750876ec936b Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 19 Mar 2021 15:38:34 +0200 Subject: Redo entering of src directories into scope_map --- build2/b.cxx | 8 +-- libbuild2/algorithm.cxx | 4 +- libbuild2/bash/rule.cxx | 2 +- libbuild2/cc/compile-rule.cxx | 30 +++++++---- libbuild2/config/operation.cxx | 8 +-- libbuild2/context.cxx | 6 +-- libbuild2/dist/operation.cxx | 8 +-- libbuild2/dump.cxx | 10 ++-- libbuild2/file.cxx | 26 ++++----- libbuild2/operation.cxx | 6 ++- libbuild2/scope.cxx | 118 ++++++++++++++++++++++++----------------- libbuild2/scope.hxx | 95 +++++++++++++++++++-------------- libbuild2/target.cxx | 2 +- 13 files changed, 187 insertions(+), 136 deletions(-) diff --git a/build2/b.cxx b/build2/b.cxx index 93c02b4..6424b59 100644 --- a/build2/b.cxx +++ b/build2/b.cxx @@ -1167,7 +1167,7 @@ main (int argc, char* argv[]) // use to the bootstrap files (other than src-root.build, which, // BTW, doesn't need to exist if src_root == out_root). // - scope& rs (*create_root (*ctx, out_root, src_root)->second.scope); + scope& rs (*create_root (*ctx, out_root, src_root)->second.front ()); bool bstrapped (bootstrapped (rs)); @@ -1579,7 +1579,7 @@ main (int argc, char* argv[]) // scope& s ( o.dir - ? *sm.insert ((out_base / *o.dir).normalize ())->second.scope + ? *sm.insert_out ((out_base / *o.dir).normalize ())->second.front () : *rs.weak_scope ()); auto p (s.vars.insert (o.ovr)); @@ -1600,7 +1600,7 @@ main (int argc, char* argv[]) scope& s ( o.dir - ? *sm.insert ((out_base / *o.dir).normalize ())->second.scope + ? *sm.insert_out ((out_base / *o.dir).normalize ())->second.front () : rs); auto p (s.vars.insert (o.ovr)); @@ -1646,7 +1646,7 @@ main (int argc, char* argv[]) // building before we know how to for all the targets in this // operation batch. // - const scope& bs (ctx->scopes.find (ts.out_base)); + const scope& bs (ctx->scopes.find_out (ts.out_base)); // Find the target type and extract the extension. // diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 39df018..3f83ddd 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -1063,7 +1063,7 @@ namespace build2 // const dir_path& d (parent && t.name.empty () ? t.dir.directory () : t.dir); - const scope& bs (t.ctx.scopes.find (d)); + const scope& bs (t.ctx.scopes.find_out (d)); const scope* rs (bs.root_scope ()); // If root scope is NULL, then this can mean that we are out of any @@ -1142,7 +1142,7 @@ namespace build2 if (op_t != nullptr) { - op_s = &t.ctx.scopes.find (t.dir); + op_s = &t.ctx.scopes.find_out (t.dir); // Always out. if (op_s->out_path () == t.dir && !op_s->operation_callbacks.empty ()) { diff --git a/libbuild2/bash/rule.cxx b/libbuild2/bash/rule.cxx index 8b47fb0..d378227 100644 --- a/libbuild2/bash/rule.cxx +++ b/libbuild2/bash/rule.cxx @@ -278,7 +278,7 @@ namespace build2 continue; } - if (const scope* rs = t.ctx.scopes.find (b->dir).root_scope ()) + if (const scope* rs = b->base_scope ().root_scope ()) { const dir_path& d (pp.sub (rs->src_path ()) ? rs->src_path () diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx index 9ae76a9..cb11728 100644 --- a/libbuild2/cc/compile-rule.cxx +++ b/libbuild2/cc/compile-rule.cxx @@ -2839,13 +2839,22 @@ namespace build2 // small_vector tts; - const scope& bs (t.ctx.scopes.find (d)); - if (const scope* rs = bs.root_scope ()) + // Note that the path can be in out or src directory and the latter + // can be associated with multiple scopes. So strictly speaking we + // need to pick one that is "associated" with us. But that is still a + // TODO (see scope_map::find() for details) and so for now we just + // pick the first one (it's highly unlikely the source file extension + // mapping will differ based on the configuration). + // { - tts = map_extension (bs, n, e); + const scope& bs (**t.ctx.scopes.find (d).first); + if (const scope* rs = bs.root_scope ()) + { + tts = map_extension (bs, n, e); - if (!bs.out_eq_src () && d.sub (bs.src_path ())) - out = out_src (d, *rs); + if (!bs.out_eq_src () && d.sub (bs.src_path ())) + out = out_src (d, *rs); + } } // If it is outside any project, or the project doesn't have such an @@ -3504,12 +3513,13 @@ namespace build2 // See if this path is inside a project with an out-of- // tree build and is in the out directory tree. // - const scope& bs (ctx.scopes.find (d)); + const scope& bs (ctx.scopes.find_out (d)); if (bs.root_scope () != nullptr) { - const dir_path& bp (bs.out_path ()); - if (bp != bs.src_path ()) + if (!bs.out_eq_src ()) { + const dir_path& bp (bs.out_path ()); + bool e; if ((e = (d == bp)) || d.sub (bp)) { @@ -5977,7 +5987,7 @@ namespace build2 module_build_modules_dir /= x); - const scope* ps (&ctx.scopes.find (pd)); + const scope* ps (&ctx.scopes.find_out (pd)); if (ps->out_path () != pd) { @@ -5988,7 +5998,7 @@ namespace build2 // Re-test again now that we are in exclusive phase (another thread // could have already created and loaded the subproject). // - ps = &ctx.scopes.find (pd); + ps = &ctx.scopes.find_out (pd); if (ps->out_path () != pd) { diff --git a/libbuild2/config/operation.cxx b/libbuild2/config/operation.cxx index 7490bc3..d30de4d 100644 --- a/libbuild2/config/operation.cxx +++ b/libbuild2/config/operation.cxx @@ -609,7 +609,7 @@ namespace build2 { const dir_path& pd (p.second); dir_path out_nroot (out_root / pd); - const scope& nrs (ctx.scopes.find (out_nroot)); + const scope& nrs (ctx.scopes.find_out (out_nroot)); // @@ Strictly speaking we need to check whether the config module // was loaded for this subproject. @@ -649,7 +649,7 @@ namespace build2 for (auto p: *ps) { dir_path out_nroot (out_root / p.second); - const scope& nrs (ctx.scopes.find (out_nroot)); + const scope& nrs (ctx.scopes.find_out (out_nroot)); assert (nrs.out_path () == out_nroot); configure_forward (nrs, projects); @@ -877,7 +877,7 @@ namespace build2 { const dir_path& pd (p.second); dir_path out_nroot (out_root / pd); - const scope& nrs (ctx.scopes.find (out_nroot)); + const scope& nrs (ctx.scopes.find_out (out_nroot)); assert (nrs.out_path () == out_nroot); // See disfigure_load(). r = disfigure_project (a, nrs, projects) || r; @@ -985,7 +985,7 @@ namespace build2 for (auto p: *ps) { dir_path out_nroot (out_root / p.second); - const scope& nrs (ctx.scopes.find (out_nroot)); + const scope& nrs (ctx.scopes.find_out (out_nroot)); assert (nrs.out_path () == out_nroot); r = disfigure_forward (nrs, projects) || r; diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx index e8f671d..09ab15d 100644 --- a/libbuild2/context.cxx +++ b/libbuild2/context.cxx @@ -34,8 +34,8 @@ namespace build2 static inline scope& create_global_scope (scope_map& m) { - auto i (m.insert (dir_path ())); - scope& r (*i->second.scope); + auto i (m.insert_out (dir_path ())); + scope& r (*i->second.front ()); r.out_path_ = &i->first; return r; }; @@ -490,7 +490,7 @@ namespace build2 // if (c == '!' || (dir && dir->absolute ())) { - scope& s (c == '!' ? gs : *sm.insert (*dir)->second.scope); + scope& s (c == '!' ? gs : *sm.insert_out (*dir)->second.front ()); auto p (s.vars.insert (*o)); assert (p.second); // Variable name is unique. diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx index ac76a62..f04a809 100644 --- a/libbuild2/dist/operation.cxx +++ b/libbuild2/dist/operation.cxx @@ -309,7 +309,7 @@ namespace build2 { const dir_path& pd (p.second); dir_path out_nroot (out_root / pd); - const scope& nrs (ctx.scopes.find (out_nroot)); + const scope& nrs (ctx.scopes.find_out (out_nroot)); if (nrs.out_path () != out_nroot) // This subproject not loaded. continue; @@ -470,7 +470,7 @@ namespace build2 const dir_path& pd (p.second); if (dl.sub (pd)) { - srs = &ctx.scopes.find (out_root / pd); + srs = &ctx.scopes.find_out (out_root / pd); if (auto* m = srs->find_module (module::name)) cbs = &m->callbacks_; @@ -1008,7 +1008,9 @@ namespace build2 if (rs.out_path () != out_base || rs.src_path () != src_base) fail (l) << "dist meta-operation target must be project root directory"; - setup_base (rs.ctx.scopes.rw (rs).insert (out_base), out_base, src_base); + setup_base (rs.ctx.scopes.rw (rs).insert_out (out_base), + out_base, + src_base); // Also initialize the dist.* variables (needed in dist_project()). // diff --git a/libbuild2/dump.cxx b/libbuild2/dump.cxx index 7cd95dd..bc44b24 100644 --- a/libbuild2/dump.cxx +++ b/libbuild2/dump.cxx @@ -438,7 +438,7 @@ namespace build2 scope_map::const_iterator& i, bool rel) { - const scope& p (*i->second.scope); + const scope& p (*i->second.front ()); const dir_path& d (i->first); ++i; @@ -493,7 +493,9 @@ namespace build2 // scope). // for (auto e (p.ctx.scopes.end ()); - i != e && i->second.out && i->second.scope->parent_scope () == &p; ) + (i != e && + i->second.front () != nullptr && + i->second.front ()->parent_scope () == &p); ) { if (vb) { @@ -543,7 +545,7 @@ namespace build2 dump (const context& c, optional a) { auto i (c.scopes.begin ()); - assert (i->second.scope == &c.global_scope); + assert (i->second.front () == &c.global_scope); // We don't lock diag_stream here as dump() is supposed to be called from // the main thread prior/after to any other threads being spawned. @@ -559,7 +561,7 @@ namespace build2 { const scope_map& m (s.ctx.scopes); auto i (m.find_exact (s.out_path ())); - assert (i != m.end () && i->second.scope == &s); + assert (i != m.end () && i->second.front () == &s); string ind (cind); ostream& os (*diag_stream); diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 87e21a6..4f24dd9 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -398,8 +398,8 @@ namespace build2 const dir_path& out_root, const dir_path& src_root) { - auto i (ctx.scopes.rw ().insert (out_root, true /* root */)); - scope& rs (*i->second.scope); + auto i (ctx.scopes.rw ().insert_out (out_root, true /* root */)); + scope& rs (*i->second.front ()); // Set out_path. Note that src_path is set in setup_root() below. // @@ -460,7 +460,7 @@ namespace build2 { if (*s.out_path_ != d) { - auto i (ctx.scopes.rw (s).insert (s, d)); + auto i (ctx.scopes.rw (s).insert_src (s, d)); s.src_path_ = &i->first; } else @@ -477,7 +477,7 @@ namespace build2 const dir_path& out_base, const dir_path& src_base) { - scope& s (*i->second.scope); + scope& s (*i->second.front ()); context& ctx (s.ctx); // Set src/out_base variables. @@ -507,7 +507,7 @@ namespace build2 { if (out_base != src_base) { - auto i (ctx.scopes.rw (s).insert (s, src_base)); + auto i (ctx.scopes.rw (s).insert_src (s, src_base)); s.src_path_ = &i->first; } else @@ -525,8 +525,8 @@ namespace build2 // First, enter the scope into the map and see if it is in any project. If // it is not, then there is nothing else to do. // - auto i (root.ctx.scopes.rw (root).insert (out_base)); - scope& base (*i->second.scope); + auto i (root.ctx.scopes.rw (root).insert_out (out_base)); + scope& base (*i->second.front ()); scope* rs (nullptr); @@ -739,7 +739,7 @@ namespace build2 // in which case we will have src_root and maybe even the name. // const dir_path* src_root (nullptr); - const scope& s (ctx.scopes.find (out_root)); + const scope& s (ctx.scopes.find_out (out_root)); if (s.root_scope () == &s && s.out_path () == out_root) { @@ -1332,7 +1332,7 @@ namespace build2 // probably be tried first since that src_root was explicitly configured // by the user. After that, #2 followed by #1 seems reasonable. // - scope& rs (*create_root (ctx, out_root, dir_path ())->second.scope); + scope& rs (*create_root (ctx, out_root, dir_path ())->second.front ()); bool bstrapped (bootstrapped (rs)); @@ -1399,7 +1399,7 @@ namespace build2 // The same logic to src_root as in create_bootstrap_outer(). // - scope& rs (*create_root (ctx, out_root, dir_path ())->second.scope); + scope& rs (*create_root (ctx, out_root, dir_path ())->second.front ()); optional altn; if (!bootstrapped (rs)) @@ -1634,7 +1634,7 @@ namespace build2 assert (!forwarded || out_root != src_root); auto i (create_root (ctx, out_root, src_root)); - scope& rs (*i->second.scope); + scope& rs (*i->second.front ()); if (!bootstrapped (rs)) { @@ -2343,7 +2343,7 @@ namespace build2 { bool top (proot == nullptr); - root = create_root (ctx, out_root, src_root)->second.scope; + root = create_root (ctx, out_root, src_root)->second.front (); bool bstrapped (bootstrapped (*root)); @@ -2532,7 +2532,7 @@ namespace build2 // dir_path out_base (out_src (src_base, *root)); - auto i (ctx.scopes.rw (*root).insert (out_base)); + auto i (ctx.scopes.rw (*root).insert_out (out_base)); scope& base (setup_base (i, move (out_base), move (src_base))); source_once (*root, base, *bf); diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index 12aba0c..826cfff 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -81,7 +81,7 @@ namespace build2 // Create the base scope. Note that its existence doesn't mean it was // already setup as a base scope; it can be the same as root. // - auto i (root.ctx.scopes.rw (root).insert (out_base)); + auto i (root.ctx.scopes.rw (root).insert_out (out_base)); scope& base (setup_base (i, out_base, src_base)); // Load the buildfile unless it is implied. @@ -505,7 +505,9 @@ namespace build2 if (rs.out_path () != out_base || rs.src_path () != src_base) fail (l) << "meta-operation info target must be project root directory"; - setup_base (rs.ctx.scopes.rw (rs).insert (out_base), out_base, src_base); + setup_base (rs.ctx.scopes.rw (rs).insert_out (out_base), + out_base, + src_base); } void diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx index 29e0c55..c5eb17e 100644 --- a/libbuild2/scope.cxx +++ b/libbuild2/scope.cxx @@ -942,23 +942,20 @@ namespace build2 // auto scope_map:: - insert (const dir_path& k, bool root) -> iterator + insert_out (const dir_path& k, bool root) -> iterator { - auto er (map_.emplace (k, true /* out */)); + auto er (map_.emplace (k, scopes ())); if (er.second) + er.first->second.push_back (nullptr); + + if (er.first->second.front () == nullptr) { - er.first->second.scope = new scope (ctx, true /* global */); - } - else if (!er.first->second.out) - { - // This can potentially be triggered if we use the same directory as one - // project's out and another's src. - // - fail << "directory " << k << " is used as both src and out scope"; + er.first->second.front () = new scope (ctx, true /* global */); + er.second = true; } - scope& s (*er.first->second.scope); + scope& s (*er.first->second.front ()); // If this is a new scope, update the parent chain. // @@ -976,27 +973,28 @@ namespace build2 auto r (map_.find_sub (k)); for (++r.first; r.first != r.second; ++r.first) { - scope& c (*r.first->second.scope); - - // The first scope of which we are a parent is the least (shortest) - // one which means there is no other scope between it and our - // parent. - // - if (p == nullptr) - p = c.parent_; - - if (root && c.root_ == p->root_) // No intermediate root. - c.root_ = &s; - - if (p == c.parent_) // No intermediate parent. - c.parent_ = &s; + if (scope* c = r.first->second.front ()) + { + // The first scope of which we are a parent is the least + // (shortest) one which means there is no other scope between it + // and our parent. + // + if (p == nullptr) + p = c->parent_; + + if (root && c->root_ == p->root_) // No intermediate root. + c->root_ = &s; + + if (p == c->parent_) // No intermediate parent. + c->parent_ = &s; + } } // We couldn't get the parent from one of its old children so we have // to find it ourselves. // if (p == nullptr) - p = &find (k.directory ()); + p = &find_out (k.directory ()); } s.parent_ = p; @@ -1009,10 +1007,11 @@ namespace build2 auto r (map_.find_sub (k)); for (++r.first; r.first != r.second; ++r.first) { - scope& c (*r.first->second.scope); - - if (c.root_ == s.root_) // No intermediate root. - c.root_ = &s; + if (scope* c = r.first->second.front ()) + { + if (c->root_ == s.root_) // No intermediate root. + c->root_ = &s; + } } s.root_ = &s; @@ -1022,35 +1021,56 @@ namespace build2 } auto scope_map:: - insert (scope& s, const dir_path& k) -> iterator + insert_src (scope& s, const dir_path& k) -> iterator { - auto er (map_.emplace (k, false /* out */)); + auto er (map_.emplace (k, scopes ())); if (er.second) - { - er.first->second.scope = &s; - } - else if (!er.first->second.out) - { - assert (er.first->second.scope == &s); - } - else - { - // This can be triggered, for example, by specifying a variable override - // with src instead of out directory. - // - fail << "directory " << k << " is used as both src and out scope"; - } + er.first->second.push_back (nullptr); // Owning out path entry. + + // It doesn't feel like this function can possibly be called multiple + // times for the same scope and path so we skip the duplicate check. + // + er.first->second.push_back (&s); return er.first; } scope& scope_map:: - find (const dir_path& k) + find_out (const dir_path& k) { assert (k.normalized (false)); // Allow non-canonical dir separators. + + // This one is tricky: if we found an entry that doesn't contain the + // out path scope, then we need to consider outer scopes. + // + auto i (map_.find_sup_if (k, + [] (const pair& v) + { + return v.second.front () != nullptr; + })); + + assert (i != map_.end ()); // Should have at least global scope. + return *i->second.front (); + } + + auto scope_map:: + find (const dir_path& k) const -> pair + { + assert (k.normalized (false)); auto i (map_.find_sup (k)); - assert (i != map_.end ()); // Should have global scope. - return *i->second.scope; + assert (i != map_.end ()); + + auto b (i->second.begin ()); + auto e (i->second.end ()); + + // Skip NULL first element. + // + if (*b == nullptr) + ++b; + + assert (b != e); + return make_pair (b, e); } } diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx index 11fdfe4..b8ee15a 100644 --- a/libbuild2/scope.hxx +++ b/libbuild2/scope.hxx @@ -634,32 +634,27 @@ namespace build2 // While it contains both out and src paths, the latter is not available // during bootstrap (see setup_root() and setup_base() for details). // + // Note also that the same src path can be naturally associated with + // multiple out paths/scopes (and one of them may be the same as src). + // class scope_map { public: - struct scope_ptr + // The first element, if not NULL, is for the "owning" out path. The rest + // of the elements are for the src path shallow references. + // + struct scopes: small_vector { - using scope_type = build2::scope; - - scope_type* scope; - bool out; - - scope_ptr (bool o): scope (nullptr), out (o) {} - ~scope_ptr () {if (out) delete scope;} - - scope_ptr (scope_ptr&& x) // For GCC 4.9 - : scope (x.scope), out (x.out) - { - x.scope = nullptr; - } - - scope_ptr& operator= (scope_ptr&&) = delete; + scopes () = default; + ~scopes () {if (!empty ()) delete front ();} - scope_ptr (const scope_ptr&) = delete; - scope_ptr& operator= (const scope_ptr&) = delete; + scopes (scopes&&) = default; // For GCC 4.9 + scopes (const scopes&) = delete; + scopes& operator= (scopes&&) = delete; + scopes& operator= (const scopes&) = delete; }; - using map_type = dir_path_map; + using map_type = dir_path_map; using iterator = map_type::iterator; using const_iterator = map_type::const_iterator; @@ -670,36 +665,56 @@ namespace build2 // global scope with empty key. // LIBBUILD2_SYMEXPORT iterator - insert (const dir_path& our_path, bool root = false); + insert_out (const dir_path& our_path, bool root = false); // Insert a shallow reference to the scope for its src path. // LIBBUILD2_SYMEXPORT iterator - insert (scope&, const dir_path& src_path); + insert_src (scope&, const dir_path& src_path); - // Find the most qualified scope that encompasses this path. + // Find the most qualified scope that encompasses this out path. // const scope& - find (const dir_path& d) const + find_out (const dir_path& d) const { - return const_cast (this)->find (d); + return const_cast (this)->find_out (d); } - const scope& - find (const path& p) const - { - // Natural thing to do here would be to call find (p.directory ()). - // However, there could be a situation where the passed path is a - // directory (i.e., the calling code does not know what it is dealing - // with), so let's use the whole path. - // - // In fact, ideally, we should have used path_map instead of - // dir_path_map to be able to search for both paths without any casting - // (and copies). But currently we have too much stuff pointing to the - // key. - // - return find (path_cast (p)); - } + // Find all the scopes that encompass this path (out or src). + // + // Note that the returned range will never be empty (there is always the + // global scope). + // + // If the path is in src, then we may end up with multiple scopes. For + // example, if two configurations of the same project are being built in a + // single invocation. How can we pick the scope that is "ours", for some + // definition of "ours"? + // + // The current think is that a project can be "associated" with other + // projects: its sub-projects and imported projects (it doesn't feel like + // its super-projects should be in this set, but maybe). And "ours" could + // mean belonging to one of the associated projects. This feels correct + // since a project shouldn't really be reaching into unrelated projects. + // And a project can only import one instance of any given project. + // + // We could implement this by keeping track (in scope::root_extra) of all + // the imported projects. The potential problem is performance: we would + // need to traverse the imported projects set recursively (potentially + // re-traversing the same projects multiple times). + // + // An alternative idea is to tag associated scopes with some marker so + // that all the scopes that "know" about each other have the same tag, + // essentially partitioning the scope set into connected subsets. One + // issue here (other than the complexity of implementing something like + // this) is that there could potentially be multiple source scopes with + // the same tag (e.g., two projects that don't know anything about each + // other could each import a different configuration of some common + // project and in turn be both imported by yet another project thus all + // acquiring the same tag). BTW, this could also be related to that + // "island append" restriction we have on loading additional buildfile. + // + LIBBUILD2_SYMEXPORT pair + find (const dir_path&) const; const_iterator begin () const {return map_.begin ();} const_iterator end () const {return map_.end ();} @@ -725,7 +740,7 @@ namespace build2 scope_map (context& c): ctx (c) {} LIBBUILD2_SYMEXPORT scope& - find (const dir_path&); + find_out (const dir_path&); private: context& ctx; diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index 6647f75..2ac5f9a 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -132,7 +132,7 @@ namespace build2 // If this target is from the src tree, use its out directory to find // the scope. // - return ctx.scopes.find (out_dir ()); + return ctx.scopes.find_out (out_dir ()); } const scope& target:: -- cgit v1.1