From 3ba17db6300d7e0cfc4fa001b5a8eb91bf417ea3 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 12 Oct 2022 08:31:54 +0200 Subject: Switch to public/private variables model Now unqualified variables are project-private and can be typified. --- libbuild2/context.cxx | 39 +++++++++------------- libbuild2/context.hxx | 13 ++++---- libbuild2/dump.cxx | 4 +-- libbuild2/file.cxx | 10 +++--- libbuild2/function.test.cxx | 4 ++- libbuild2/install/init.cxx | 28 ++++++++++------ libbuild2/install/operation.cxx | 3 -- libbuild2/install/rule.cxx | 45 +++++++++++++------------- libbuild2/operation.cxx | 3 -- libbuild2/operation.hxx | 46 ++++++++++++++++++-------- libbuild2/parser.cxx | 12 +++++-- libbuild2/scope.hxx | 72 +++++++++++++++++++++++++++++------------ libbuild2/target.cxx | 19 +++++++---- libbuild2/test/init.cxx | 10 +++--- libbuild2/test/operation.cxx | 2 -- libbuild2/variable.cxx | 67 +++++++++++++++++++++++++++++++++++++- libbuild2/variable.hxx | 11 +++++-- libbuild2/variable.ixx | 15 ++++++++- 18 files changed, 275 insertions(+), 128 deletions(-) (limited to 'libbuild2') diff --git a/libbuild2/context.cxx b/libbuild2/context.cxx index 4cbdecb..f46a3eb 100644 --- a/libbuild2/context.cxx +++ b/libbuild2/context.cxx @@ -464,6 +464,18 @@ namespace build2 string n (t.value, c == '!' || c == '%' || c == '/' ? 1 : 0); + // Make sure it is qualified. + // + // We can support overridable public unqualified variables (which must + // all be pre-entered by the end of this constructor) but we will need + // to detect their names here in an ad hoc manner (we cannot enter them + // before this logic because of the "untyped override" requirement). + // + // Note: issue the same diagnostics as in variable_pool::update(). + // + if (n.find ('.') == string::npos) + fail << "variable " << n << " cannot be overridden"; + if (c == '!' && dir) fail << "scope-qualified global override of variable " << n; @@ -620,6 +632,10 @@ namespace build2 r.insert (perform_update_id, "build.file", file_rule::instance); r.insert (perform_clean_id, "build.file", file_rule::instance); } + + // End of initialization. + // + load_generation = 1; } context:: @@ -716,29 +732,6 @@ namespace build2 current_mode = inner_oif.mode; current_diag_noise = diag_noise; - auto find_ovar = [this] (const char* n) - { - const variable* v (var_pool.find (n)); // @@ TMP: pub/prv vars - - // The operation variable should have prerequisite or target visibility. - // - assert (v != nullptr && - (v->visibility == variable_visibility::prereq || - v->visibility == variable_visibility::target)); - - return v; - }; - - current_inner_ovar = - inner_oif.var_name != nullptr - ? find_ovar (inner_oif.var_name) - : nullptr; - - current_outer_ovar = - outer_oif != nullptr && outer_oif->var_name != nullptr - ? find_ovar (outer_oif->var_name) - : nullptr; - // Reset counters (serial execution). // dependency_count.store (0, memory_order_relaxed); diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx index 9cb4fb4..b426ea0 100644 --- a/libbuild2/context.hxx +++ b/libbuild2/context.hxx @@ -253,7 +253,7 @@ namespace build2 // change already existing nodes or invalidate any references to such (the // idea here is that one should be able to load additional buildfiles as // long as they don't interfere with the existing build state). The - // "islands" are identified by the load_generation number (0 for the + // "islands" are identified by the load_generation number (1 for the // initial/serial load). It is incremented in case of a phase switch and // can be stored in various "nodes" to verify modifications are only done // "within the islands". Another example of invalidation would be @@ -263,6 +263,10 @@ namespace build2 // for example, we may have queried a variable which in the new hierarchy // would "see" a new value from the newly inserted scope. // + // The special load_generation value 0 indicates initialization before + // anything has been loaded. Currently, it is changed to 1 at the end + // of the context constructor. + // run_phase phase = run_phase::load; size_t load_generation = 0; @@ -309,11 +313,6 @@ namespace build2 const operation_info* current_inner_oif; const operation_info* current_outer_oif; - // Current operation-specific variables. - // - const variable* current_inner_ovar; - const variable* current_outer_ovar; - action current_action () const { @@ -549,7 +548,7 @@ namespace build2 // operations. The initial idea was to extend this value to allow // specifying the operation (e.g., clean@false). However, later we // realized that we could reuse the "operation-specific variables" - // (update, clean, install, test; see context::current_ovar) with a more + // (update, clean, install, test; see project_operation_info) with a more // natural-looking and composable result. Plus, this allows for // operation-specific "modifiers", for example, "unmatch" and "update // during match" logic for update (see var_update for details) or diff --git a/libbuild2/dump.cxx b/libbuild2/dump.cxx index 14078da..b8ec74e 100644 --- a/libbuild2/dump.cxx +++ b/libbuild2/dump.cxx @@ -208,7 +208,7 @@ namespace build2 for (action a: r.actions) os << ' ' << re.meta_operations[a.meta_operation ()]->name << - '(' << re.operations[a.operation ()]->name << ')'; + '(' << re.operations[a.operation ()].info->name << ')'; os << endl; r.dump_text (os, ind); @@ -289,7 +289,7 @@ namespace build2 os << "rule_hint="; if (v.operation != default_id) - os << s.root_scope ()->root_extra->operations[v.operation]->name + os << s.root_scope ()->root_extra->operations[v.operation].info->name << '@'; os << v.hint; diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index a3962d7..cb57f70 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -582,7 +582,7 @@ namespace build2 fail << "variable out_root expected as first line in " << f << endf; } - // Note: not static due to being a friend of variable_pool. + // Note: not static due to being a friend of scope and variable_pool. // void setup_root_extra (scope& root, optional& altn) @@ -620,6 +620,8 @@ namespace build2 {}, /* environment */ ""} /* environment_checksum */); + root.var_pool_ = &root.root_extra->var_pool; + // Enter built-in meta-operation and operation names. Loading of // modules (via the src bootstrap; see below) can result in // additional meta/operations being added. @@ -628,9 +630,9 @@ namespace build2 root.insert_meta_operation (perform_id, mo_perform); root.insert_meta_operation (info_id, mo_info); - root.insert_operation (default_id, op_default); - root.insert_operation (update_id, op_update); - root.insert_operation (clean_id, op_clean); + root.insert_operation (default_id, op_default, nullptr); + root.insert_operation (update_id, op_update, ctx.var_update); + root.insert_operation (clean_id, op_clean, ctx.var_clean); } value& diff --git a/libbuild2/function.test.cxx b/libbuild2/function.test.cxx index f711d2f..7ce7ad3 100644 --- a/libbuild2/function.test.cxx +++ b/libbuild2/function.test.cxx @@ -124,7 +124,9 @@ namespace build2 try { - scope& s (ctx.global_scope.rw ()); + // Use temp scope for the private variable pool. + // + temp_scope s (ctx.global_scope.rw ()); parser p (ctx); p.parse_buildfile (cin, path_name ("buildfile"), &s, s); diff --git a/libbuild2/install/init.cxx b/libbuild2/install/init.cxx index ef9de05..35c2d13 100644 --- a/libbuild2/install/init.cxx +++ b/libbuild2/install/init.cxx @@ -250,6 +250,21 @@ namespace build2 context& ctx (rs.ctx); + // Enter module variables (note that init() below enters some more). + // + auto& vp (rs.var_pool ()); + + // The install variable is a path, not dir_path, since it can be used + // to both specify the target directory (to install with the same file + // name) or target file (to install with a different name). And the + // way we distinguish between the two is via the presence/absence of + // the trailing directory separator. + // + // Plus it can have the special true/false values when acting as a + // operation variable. + // + auto& ovar (vp.insert ("install", variable_visibility::target)); + // Register the install function family if this is the first instance of // the install modules. // @@ -258,9 +273,9 @@ namespace build2 // Register our operations. // - rs.insert_operation (install_id, op_install); - rs.insert_operation (uninstall_id, op_uninstall); - rs.insert_operation (update_for_install_id, op_update_for_install); + rs.insert_operation (install_id, op_install, &ovar); + rs.insert_operation (uninstall_id, op_uninstall, &ovar); + rs.insert_operation (update_for_install_id, op_update_for_install, &ovar); } static const path cmd ("install"); @@ -317,13 +332,6 @@ namespace build2 // Note that the set_dir() calls below enter some more. // { - // The install variable is a path, not dir_path, since it can be used - // to both specify the target directory (to install with the same file - // name) or target file (to install with a different name). And the - // way we distinguish between the two is via the presence/absence of - // the trailing directory separator. - // - vp.insert ("install", variable_visibility::target); vp.insert ("for_install", variable_visibility::prereq); vp.insert ("install.mode"); vp.insert ("install.subdirs"); diff --git a/libbuild2/install/operation.cxx b/libbuild2/install/operation.cxx index 8c59ac1..52e8c94 100644 --- a/libbuild2/install/operation.cxx +++ b/libbuild2/install/operation.cxx @@ -37,7 +37,6 @@ namespace build2 0, "install", "install", - "install", "installing", "installed", "has nothing to install", // We cannot "be installed". @@ -62,7 +61,6 @@ namespace build2 uninstall_id, 0, "uninstall", - "install", "uninstall", "uninstalling", "uninstalled", @@ -81,7 +79,6 @@ namespace build2 update_id, // Note: not update_for_install_id. install_id, op_update.name, - op_update.var_name, op_update.name_do, op_update.name_doing, op_update.name_did, diff --git a/libbuild2/install/rule.cxx b/libbuild2/install/rule.cxx index 6458a54..756d5a8 100644 --- a/libbuild2/install/rule.cxx +++ b/libbuild2/install/rule.cxx @@ -40,12 +40,17 @@ namespace build2 // Note that the below rules are called for both install and // update-for-install. // + // @@ TODO: we clearly need a module class. + // static inline const variable& - var_install (context& ctx) + var_install (const scope& rs) { - return ctx.current_outer_ovar != nullptr - ? *ctx.current_outer_ovar - : *ctx.current_inner_ovar; + context& ctx (rs.ctx); + + return *rs.root_extra->operations[ + (ctx.current_outer_oif != nullptr + ? ctx.current_outer_oif + : ctx.current_inner_oif)->id].ovar; } // alias_rule @@ -85,8 +90,6 @@ namespace build2 { tracer trace ("install::alias_rule::apply"); - context& ctx (t.ctx); - // Pass-through to our installable prerequisites. // // @@ Shouldn't we do match in parallel (here and below)? @@ -138,7 +141,7 @@ namespace build2 // // Note: not the same as lookup_install() above. // - auto l ((*pt)[var_install (ctx)]); + auto l ((*pt)[var_install (*p.scope.root_scope ())]); if (l && cast (l).string () == "false") { l5 ([&]{trace << "ignoring " << *pt << " (not installable)";}); @@ -223,8 +226,10 @@ namespace build2 // if (p.is_a ()) { + const scope& rs (*p.scope.root_scope ()); + if (p.vars.empty () || - cast_empty (p.vars[var_install (t.ctx)]).string () != "true") + cast_empty (p.vars[var_install (rs)]).string () != "true") return nullptr; } @@ -237,8 +242,6 @@ namespace build2 { tracer trace ("install::group_rule::apply"); - context& ctx (t.ctx); - // Resolve group members. // // Remember that we are called twice: first during update for install @@ -253,8 +256,10 @@ namespace build2 ? resolve_members (a, t) : t.group_members (a)); - if (gv.members != nullptr) + if (gv.members != nullptr && gv.count != 0) { + const scope& rs (t.root_scope ()); + auto& pts (t.prerequisite_targets[a]); for (size_t i (0); i != gv.count; ++i) { @@ -277,7 +282,7 @@ namespace build2 // // Note: not the same as lookup_install() above. // - auto l ((*mt)[var_install (ctx)]); + auto l ((*mt)[var_install (rs)]); if (l && cast (l).string () == "false") { l5 ([&]{trace << "ignoring " << *mt << " (not installable)";}); @@ -324,13 +329,15 @@ namespace build2 // if (p.is_a ()) { + const scope& rs (*p.scope.root_scope ()); + // Note that while include() checks for install=false, here we need to // check for explicit install=true. We could have re-used the lookup // performed by include(), but then we would have had to drag it // through and also diagnose any invalid values. // if (p.vars.empty () || - cast_empty (p.vars[var_install (t.ctx)]).string () != "true") + cast_empty (p.vars[var_install (rs)]).string () != "true") return nullptr; } @@ -350,8 +357,6 @@ namespace build2 { tracer trace ("install::file_rule::apply"); - context& ctx (t.ctx); - // Note that we are called both as the outer part during the update-for- // un/install pre-operation and as the inner part during the un/install // operation itself. @@ -419,7 +424,7 @@ namespace build2 // // Note: not the same as lookup_install() above. // - auto l ((*pt)[var_install (ctx)]); + auto l ((*pt)[var_install (*p.scope.root_scope ())]); if (l && cast (l).string () == "false") { l5 ([&]{trace << "ignoring " << *pt << " (not installable)";}); @@ -974,8 +979,6 @@ namespace build2 target_state file_rule:: perform_install (action a, const target& xt) const { - context& ctx (xt.ctx); - const file& t (xt.as ()); const path& tp (t.path ()); @@ -1073,7 +1076,7 @@ namespace build2 // if (!tp.empty ()) { - install_target (t, cast (t[var_install (ctx)]), 1); + install_target (t, cast (t[var_install (rs)]), 1); r |= target_state::changed; } @@ -1277,8 +1280,6 @@ namespace build2 target_state file_rule:: perform_uninstall (action a, const target& xt) const { - context& ctx (xt.ctx); - const file& t (xt.as ()); const path& tp (t.path ()); @@ -1342,7 +1343,7 @@ namespace build2 target_state r (target_state::unchanged); if (!tp.empty ()) - r |= uninstall_target (t, cast (t[var_install (ctx)]), 1); + r |= uninstall_target (t, cast (t[var_install (rs)]), 1); // Then installable ad hoc group members, if any. To be anally precise, // we would have to do it in reverse, but that's not easy (it's a diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index 412e07f..fc569a9 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -978,7 +978,6 @@ namespace build2 default_id, 0, "", - nullptr, "", "", "", @@ -1006,7 +1005,6 @@ namespace build2 0, "update", "update", - "update", "updating", "updated", "is up to date", @@ -1023,7 +1021,6 @@ namespace build2 0, "clean", "clean", - "clean", "cleaning", "cleaned", "is clean", diff --git a/libbuild2/operation.hxx b/libbuild2/operation.hxx index ce3cd79..4eb2658 100644 --- a/libbuild2/operation.hxx +++ b/libbuild2/operation.hxx @@ -196,7 +196,6 @@ namespace build2 const operation_id id; const operation_id outer_id; const char* name; - const char* var_name; // Operation variable or NULL if not used. // Name derivatives for diagnostics. Note that unlike meta-operations, // these can only be empty for the default operation (id 1), And @@ -308,35 +307,36 @@ namespace build2 using operation_table = butl::string_table; - // These are "sparse" in the sense that we may have "holes" that - // are represented as NULL pointers. Also, lookup out of bounds - // is treated as a hole. + // This is a "sparse" vector in the sense that we may have "holes" that are + // represented as default-initialized empty instances (for example, NULL if + // T is a pointer). Also, lookup out of bounds is treated as a hole. // template struct sparse_vector { - using base_type = small_vector; + using base_type = small_vector; using size_type = typename base_type::size_type; void - insert (size_type i, T& x) + insert (size_type i, T x) { size_type n (v_.size ()); if (i < n) - v_[i] = &x; + v_[i] = x; else { if (n != i) - v_.resize (i, nullptr); // Add holes. - v_.push_back (&x); + v_.resize (i, T ()); // Add holes. + + v_.push_back (move (x)); } } - T* + T operator[] (size_type i) const { - return i < v_.size () ? v_[i] : nullptr; + return i < v_.size () ? v_[i] : T (); } bool @@ -351,8 +351,28 @@ namespace build2 base_type v_; }; - using meta_operations = sparse_vector; - using operations = sparse_vector; + // For operations we keep both the pointer to its description as well + // as to its operation variable (see var_include) which may belong to + // the project-private variable pool. + // + struct project_operation_info + { + const operation_info* info = nullptr; + const variable* ovar = nullptr; // Operation variable. + + // Allow treating it as pointer to operation_info in most contexts. + // + operator const operation_info*() const {return info;} + bool operator== (nullptr_t) {return info == nullptr;} + bool operator!= (nullptr_t) {return info != nullptr;} + + project_operation_info (const operation_info* i = nullptr, // VC14 + const variable* v = nullptr) + : info (i), ovar (v) {} + }; + + using meta_operations = sparse_vector; + using operations = sparse_vector; } namespace butl diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index ec33d6e..301acf9 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -3462,9 +3462,12 @@ namespace build2 // In a somewhat hackish way we pass the variable in an undefined // lookup. // + // Note: consistent with parse_variable_name() wrt overridability. + // l = lookup (); l.var = &root_->var_pool ().insert ( - move (report_var), true /* overridable */); + move (report_var), + report_var.find ('.') != string::npos /* overridable */); } if (l.var != nullptr) @@ -4680,10 +4683,12 @@ namespace build2 { // Enter a variable name for assignment (as opposed to lookup). + // If the variable is qualified (and thus public), make it overridable. + // // Note that the overridability can still be restricted (e.g., by a module // that enters this variable or by a pattern). // - bool ovr (true); + bool ovr (on.find ('.') != string::npos); auto r (scope_->var_pool ().insert (move (on), nullptr, nullptr, &ovr)); if (!r.second) @@ -4951,6 +4956,9 @@ namespace build2 //@@ TODO: the same checks for vis and ovr (when we have the corresponding // attributes). + //@@ TODO: need to verify/diagnose ovr and visibility for project-private + // variables. + if (type || vis || ovr) var.owner->update (const_cast (var), type, diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx index 598d7e8..a02d865 100644 --- a/libbuild2/scope.hxx +++ b/libbuild2/scope.hxx @@ -510,6 +510,8 @@ namespace build2 // Project-private variable pool. // + // Note: see scope::var_pool_ and use scope::var_pool(). + // variable_pool var_pool; // Meta/operations supported by this project. @@ -551,16 +553,27 @@ namespace build2 unique_ptr root_extra; + // The last argument is the operation variable (see var_include) or NULL + // if not used. + // void - insert_operation (operation_id id, const operation_info& in) + insert_operation (operation_id id, + const operation_info& in, + const variable* ovar) { - root_extra->operations.insert (id, in); + // The operation variable should have prerequisite or target visibility. + // + assert (ovar == nullptr || + (ovar->visibility == variable_visibility::prereq || + ovar->visibility == variable_visibility::target)); + + root_extra->operations.insert (id, project_operation_info {&in, ovar}); } void insert_meta_operation (meta_operation_id id, const meta_operation_info& in) { - root_extra->meta_operations.insert (id, in); + root_extra->meta_operations.insert (id, &in); } bool @@ -593,19 +606,29 @@ namespace build2 return const_cast (*this); } - // @@ TODO: find root scope and return its var_pool falling back to - // ctx.var_pool if no root scope. + // Return the project-private variable pool (which is chained to the + // public pool) unless pub is true, in which case return the public pool. + // + // You would normally go for the public pool directly as an optimization + // (for example, in the module's init()) if you know all your variables + // are qualified and thus public. // variable_pool& - var_pool () + var_pool (bool pub = false) { - return ctx.var_pool.rw (*this); + return (pub ? ctx.var_pool : + var_pool_ != nullptr ? *var_pool_ : + root_ != nullptr ? *root_->var_pool_ : + ctx.var_pool).rw (*this); } const variable_pool& - var_pool () const + var_pool (bool pub = false) const { - return ctx.var_pool; + return (pub ? ctx.var_pool : + var_pool_ != nullptr ? *var_pool_ : + root_ != nullptr ? *root_->var_pool_ : + ctx.var_pool); } private: @@ -615,6 +638,7 @@ namespace build2 // These two from set strong_. // + friend void setup_root_extra (scope&, optional&); friend LIBBUILD2_SYMEXPORT void create_bootstrap_outer (scope&, bool); friend LIBBUILD2_SYMEXPORT scope& create_bootstrap_inner (scope&, const dir_path&); @@ -634,6 +658,8 @@ namespace build2 scope* root_; scope* strong_ = nullptr; // Only set on root scopes. // NULL means no strong amalgamtion. + + variable_pool* var_pool_ = nullptr; // For temp_scope override. }; inline bool @@ -701,24 +727,28 @@ namespace build2 // Temporary scope. The idea is to be able to create a temporary scope in // order not to change the variables in the current scope. Such a scope is - // not entered in to the scope map. As a result it can only be used as a - // temporary set of variables. In particular, defining targets directly in - // such a scope will surely end up badly. Defining any nested scopes will be - // as if defining such a scope in the parent (since path() returns parent's - // path). + // not entered in to the scope map and its parent is the global scope. As a + // result it can only be used as a temporary set of variables. In + // particular, defining targets directly in such a scope will surely end up + // badly. // class temp_scope: public scope { public: - temp_scope (scope& p) - : scope (p.ctx, false /* shared */) + temp_scope (scope& gs) + : scope (gs.ctx, false /* shared */), + var_pool_ (nullptr /* shared */, &gs.ctx.var_pool.rw (gs), nullptr) { - out_path_ = p.out_path_; - src_path_ = p.src_path_; - parent_ = &p; - root_ = p.root_; - // No need to copy strong_ since we are never root scope. + // Note that making this scope its own root is a bad idea. + // + root_ = nullptr; + parent_ = &gs; + out_path_ = gs.out_path_; + scope::var_pool_ = &var_pool_; } + + private: + variable_pool var_pool_; }; // Scope map. Protected by the phase mutex. diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index 14b6496..a466951 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -582,15 +582,22 @@ namespace build2 names storage; names_view ns; - const variable* current_ovar (nullptr); + const variable* ovar (nullptr); if (r != include_type::excluded) { - current_ovar = a.outer () - ? ctx.current_outer_ovar - : ctx.current_inner_ovar; + // Instead of going via potentially expensive target::base_scope(), use + // the prerequisite's scope; while it may not be the same as the + // targets's base scope, they must have the same root scope. + // + const scope& rs (*p.scope.root_scope ()); + + ovar = rs.root_extra->operations[ + (a.outer () + ? ctx.current_outer_oif + : ctx.current_inner_oif)->id].ovar; - if (current_ovar != nullptr && (l = p.vars[*current_ovar])) + if (ovar != nullptr && (l = p.vars[*ovar])) { // Maybe we should optimize this for the common cases (bool, path, // name)? But then again we don't expect many such overrides. Plus @@ -635,7 +642,7 @@ namespace build2 // Note: we have to delay this until the meta-operation callback above // had a chance to override it. // - fail << "unrecognized " << *current_ovar << " variable value " + fail << "unrecognized " << *ovar << " variable value " << "'" << ns << "' specified for prerequisite " << p; } } diff --git a/libbuild2/test/init.cxx b/libbuild2/test/init.cxx index c80d3f0..f62ddcc 100644 --- a/libbuild2/test/init.cxx +++ b/libbuild2/test/init.cxx @@ -30,11 +30,6 @@ namespace build2 l5 ([&]{trace << "for " << rs;}); - // Register our operations. - // - rs.insert_operation (test_id, op_test); - rs.insert_operation (update_for_test_id, op_update_for_test); - // Enter module variables. Do it during boot in case they get assigned // in bootstrap.build. // @@ -128,6 +123,11 @@ namespace build2 v = *rs.ctx.build_host; } + // Register our operations. + // + rs.insert_operation (test_id, op_test, &d.var_test); + rs.insert_operation (update_for_test_id, op_update_for_test, &d.var_test); + extra.set_module (new module (move (d))); } diff --git a/libbuild2/test/operation.cxx b/libbuild2/test/operation.cxx index b7ec357..841abb5 100644 --- a/libbuild2/test/operation.cxx +++ b/libbuild2/test/operation.cxx @@ -65,7 +65,6 @@ namespace build2 0, "test", "test", - "test", "testing", "tested", "has nothing to test", // We cannot "be tested". @@ -83,7 +82,6 @@ namespace build2 update_id, // Note: not update_for_test_id. test_id, op_update.name, - op_update.var_name, op_update.name_do, op_update.name_doing, op_update.name_did, diff --git a/libbuild2/variable.cxx b/libbuild2/variable.cxx index 54bdcf6..8b5bc24 100644 --- a/libbuild2/variable.cxx +++ b/libbuild2/variable.cxx @@ -1578,6 +1578,17 @@ namespace build2 const variable_visibility* v, const bool* o) const { + assert (var.owner == this); + + if (outer_ != nullptr) + { + // Project-private variable. Assert visibility/overridability, the same + // as in insert(). + // + assert ((o == nullptr || !*o) && + (v == nullptr || *v >= variable_visibility::project)); + } + // Check overridability (all overrides, if any, should already have // been entered; see context ctor for details). // @@ -1710,6 +1721,52 @@ namespace build2 const bool* o, bool pat) { + if (outer_ != nullptr) + { + // Project-private pool. + // + if (n.find ('.') != string::npos) // Qualified. + return outer_->insert (move (n), t, v, o, pat); + + // Unqualified. + // + // The pool chaining semantics for insertion: first check the outer pool + // then, if not found, insert in own pool. + // + if (const variable* var = outer_->find (n)) + { + // Verify type/visibility/overridability. + // + // Should we assert or fail? Currently the buildfile parser goes + // through update() to set these so let's do assert for now. We also + // require equality (these are a handful of special variables). + // + assert ((t == nullptr || t == var->type) && + (v == nullptr || *v == var->visibility) && + (o == nullptr || *o || var->overrides == nullptr)); + + return pair (const_cast (*var), false); + } + + // Project-private variable. Assert visibility/overridability and fall + // through. Again, we expect the buildfile parser to verify and diagnose + // these. + // + // Note: similar code in update(). + // + assert ((o == nullptr || !*o) && + (v == nullptr || *v >= variable_visibility::project)); + } + else if (shared_) + { + // Public pool. + // + // Make sure all the unqualified variables are pre-entered during + // initialization. + // + assert (shared_->load_generation == 0 || n.find ('.') != string::npos); + } + assert (!shared_ || shared_->phase == run_phase::load); // Apply pattern. @@ -1781,7 +1838,15 @@ namespace build2 const variable& variable_pool:: insert_alias (const variable& var, string n) { - assert (var.aliases != nullptr && var.overrides == nullptr); + if (outer_ != nullptr) + { + assert (n.find ('.') != string::npos); // Qualified. + return outer_->insert_alias (var, move (n)); + } + + assert (var.owner == this && + var.aliases != nullptr && + var.overrides == nullptr); variable& a (insert (move (n), var.type, diff --git a/libbuild2/variable.hxx b/libbuild2/variable.hxx index bfe3b87..9206cbb 100644 --- a/libbuild2/variable.hxx +++ b/libbuild2/variable.hxx @@ -1270,7 +1270,7 @@ namespace build2 // // Note also that a pattern and later insertions may restrict (but not // relax) visibility and overridability. - + // const variable& insert (string name) { @@ -1357,6 +1357,8 @@ namespace build2 // don't allow this (manually handle multiple names by merging their // values instead). // + // Note: currently only public variables can be aliased. + // const variable& insert_alias (const variable& var, string name); @@ -1380,6 +1382,8 @@ namespace build2 // true null -- public variable pool in context // true not null -- project-private pool in scope::root_extra // with outer pointing to context::var_pool + // false not null -- temporary scope-private pool in temp_scope + // with outer pointing to context::var_pool // false null -- script-private pool in script::environment // // Notice that the script-private pool doesn't rely on outer and does @@ -1388,6 +1392,7 @@ namespace build2 // private: friend class context; + friend class temp_scope; friend void setup_root_extra (scope&, optional&); // Shared pool (public or project-private). The shared argument is @@ -1440,6 +1445,8 @@ namespace build2 const bool* overridable, bool pattern = true); + // Note: the variable must belong to this pool. + // void update (variable&, const value_type*, @@ -1457,7 +1464,7 @@ namespace build2 // gets hairy very quickly (there is no std::hash for C-strings). So // let's rely on small object-optimized std::string for now. // - string n (var.name); + string n (var.name); // @@ PERF (maybe keep reuse buffer at least?) auto r (map_.insert (map::value_type (&n, move (var)))); if (r.second) diff --git a/libbuild2/variable.ixx b/libbuild2/variable.ixx index a84c012..7cafd89 100644 --- a/libbuild2/variable.ixx +++ b/libbuild2/variable.ixx @@ -916,8 +916,21 @@ namespace build2 inline const variable* variable_pool:: find (const string& n) const { + // The pool chaining semantics for lookup: first check own pool then, if + // not found, check the outer pool. + // auto i (map_.find (&n)); - return i != map_.end () ? &i->second : nullptr; + if (i != map_.end ()) + return &i->second; + + if (outer_ != nullptr) + { + i = outer_->map_.find (&n); + if (i != outer_->map_.end ()) + return &i->second; + } + + return nullptr; } // variable_map -- cgit v1.1