From fc27ec48c9d63879e4b0f049060e943233cb540d Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 17 Oct 2016 15:43:47 +0200 Subject: Cleanup match_result mess --- build2/algorithm.cxx | 8 +++--- build2/algorithm.ixx | 3 +- build2/bin/rule | 8 +++--- build2/bin/rule.cxx | 30 ++++++++++++-------- build2/cc/compile | 4 +-- build2/cc/compile.cxx | 20 ++++++++++--- build2/cc/link | 4 +-- build2/cc/link.cxx | 10 +++---- build2/cli/rule | 4 +-- build2/cli/rule.cxx | 14 +++++----- build2/dist/rule | 4 +-- build2/dist/rule.cxx | 6 ++-- build2/install/rule | 12 ++++---- build2/install/rule.cxx | 48 +++++++++++++++++++++----------- build2/rule | 74 ++++++++++++------------------------------------- build2/rule.cxx | 20 ++++++------- build2/target | 6 ++++ build2/test/rule | 4 +-- build2/test/rule.cxx | 4 +-- 19 files changed, 142 insertions(+), 141 deletions(-) diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx index 2390b04..2a5b2d2 100644 --- a/build2/algorithm.cxx +++ b/build2/algorithm.cxx @@ -55,7 +55,7 @@ namespace build2 pair match_impl (action a, target& t, bool apply, const rule* skip) { - pair r; + pair r (nullptr, false); // By default, clear the resolved targets list before calling // match(). The rule is free to modify this list in match() @@ -143,7 +143,7 @@ namespace build2 if (&ru == skip) continue; - match_result m; + match_result m (false); { auto g ( make_exception_guard ( @@ -212,7 +212,7 @@ namespace build2 // @@ We could also allow the rule to change the recipe // action in apply(). Could be useful with delegates. // - t.recipe (ra, ru.apply (ra, t, m)); + t.recipe (ra, ru.apply (ra, t)); } else { @@ -259,7 +259,7 @@ namespace build2 // phase. // const match_result& mr (rp.second); - g.recipe (mr.recipe_action, rp.first->apply (mr.recipe_action, g, mr)); + g.recipe (mr.recipe_action, rp.first->apply (mr.recipe_action, g)); } // Note that we use execute_direct() rather than execute() here to diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx index ee4e338..b2c4941 100644 --- a/build2/algorithm.ixx +++ b/build2/algorithm.ixx @@ -86,8 +86,7 @@ namespace build2 { auto rp (match_impl (a, t, false, &r)); const match_result& mr (rp.second); - return make_pair (rp.first->apply (mr.recipe_action, t, mr), - mr.recipe_action); + return make_pair (rp.first->apply (mr.recipe_action, t), mr.recipe_action); } group_view diff --git a/build2/bin/rule b/build2/bin/rule index 9bf1379..4072e4e 100644 --- a/build2/bin/rule +++ b/build2/bin/rule @@ -18,20 +18,20 @@ namespace build2 { public: virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; }; class lib_rule: public rule { public: virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; static target_state perform (action, target&); diff --git a/build2/bin/rule.cxx b/build2/bin/rule.cxx index 7710a53..3f16d71 100644 --- a/build2/bin/rule.cxx +++ b/build2/bin/rule.cxx @@ -25,17 +25,26 @@ namespace build2 fail << diag_doing (a, t) << " target group" << info << "explicitly select obje{}, obja{}, or objs{} member"; - return nullptr; + return false; } recipe obj_rule:: - apply (action, target&, const match_result&) const {return empty_recipe;} + apply (action, target&) const {return empty_recipe;} // lib // // The whole logic is pretty much as if we had our two group members as // our prerequisites. // + + struct match_data + { + const string& type; + }; + + static_assert (sizeof (match_data) <= target::data_size, + "insufficient space"); + match_result lib_rule:: match (action act, target& xt, const string&) const { @@ -79,7 +88,9 @@ namespace build2 match_only (act, *t.s); } - match_result mr (t, &type); + t.data (match_data {type}); // Save in the target's auxilary storage. + + match_result mr (true); // If there is an outer operation, indicate that we match // unconditionally so that we don't override ourselves. @@ -91,11 +102,12 @@ namespace build2 } recipe lib_rule:: - apply (action act, target& xt, const match_result& mr) const + apply (action act, target& xt) const { lib& t (static_cast (xt)); - const string& type (*static_cast (mr.cpvalue)); + const match_data& md (t.data ()); + const string& type (md.type); bool a (type == "static" || type == "both"); bool s (type == "shared" || type == "both"); @@ -116,12 +128,8 @@ namespace build2 { lib& t (static_cast (xt)); - //@@ Not cool we have to do this again. Looks like we need - // some kind of a cache vs resolved pointer, like in - // prerequisite vs prerequisite_target. - // - // - const string& type (cast (t["bin.lib"])); + const match_data& md (t.data ()); + const string& type (md.type); bool a (type == "static" || type == "both"); bool s (type == "shared" || type == "both"); diff --git a/build2/cc/compile b/build2/cc/compile index 2be2e86..584da7c 100644 --- a/build2/cc/compile +++ b/build2/cc/compile @@ -27,10 +27,10 @@ namespace build2 compile (data&&); virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; target_state perform_update (action, target&) const; diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx index 6587d8c..e180798 100644 --- a/build2/cc/compile.cxx +++ b/build2/cc/compile.cxx @@ -35,6 +35,14 @@ namespace build2 { } + struct match_data + { + prerequisite_member src; + }; + + static_assert (sizeof (match_data) <= target::data_size, + "insufficient space"); + match_result compile:: match (action a, target& t, const string&) const { @@ -53,11 +61,14 @@ namespace build2 for (prerequisite_member p: reverse_group_prerequisite_members (a, t)) { if (p.is_a (x_src)) - return p; + { + t.data (match_data {p}); // Save in the target's auxilary storage. + return true; + } } l4 ([&]{trace << "no " << x_lang << " source file for target " << t;}); - return nullptr; + return false; } // Append or hash library options from a pair of *.export.* variables @@ -180,11 +191,12 @@ namespace build2 } recipe compile:: - apply (action a, target& xt, const match_result& mr) const + apply (action a, target& xt) const { tracer trace (x, "compile::apply"); file& t (static_cast (xt)); + const match_data& md (t.data ()); scope& bs (t.base_scope ()); scope& rs (*bs.root_scope ()); @@ -297,7 +309,7 @@ namespace build2 // t.prerequisite_targets since we used standard search() and match() // above. // - file& src (mr.as_target ()); + file& src (*md.src.search ().is_a ()); // Make sure the output directory exists. // diff --git a/build2/cc/link b/build2/cc/link index cd8c10e..ea30320 100644 --- a/build2/cc/link +++ b/build2/cc/link @@ -25,10 +25,10 @@ namespace build2 link (data&&); virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; target_state perform_update (action, target&) const; diff --git a/build2/cc/link.cxx b/build2/cc/link.cxx index 5ab9d91..7722c4f 100644 --- a/build2/cc/link.cxx +++ b/build2/cc/link.cxx @@ -109,11 +109,11 @@ namespace build2 // it will most definitely need to be compiled but we can't do that. // else if (p.is_a ()) - return nullptr; + return false; } if (!(seen_x || seen_c || seen_obj || seen_lib)) - return nullptr; + return false; // We will only chain a C source if there is also an X source or we were // explicitly told to. @@ -121,7 +121,7 @@ namespace build2 if (seen_c && !seen_x && hint < x) { l4 ([&]{trace << "C prerequisite without " << x_lang << " or hint";}); - return nullptr; + return false; } // Set the library type. @@ -173,7 +173,7 @@ namespace build2 } } - return &t; + return true; } auto link:: @@ -316,7 +316,7 @@ namespace build2 } recipe link:: - apply (action a, target& xt, const match_result&) const + apply (action a, target& xt) const { tracer trace (x, "link::apply"); diff --git a/build2/cli/rule b/build2/cli/rule index 4f8ebc2..41ff8bf 100644 --- a/build2/cli/rule +++ b/build2/cli/rule @@ -18,10 +18,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; static target_state perform_update (action, target&); diff --git a/build2/cli/rule.cxx b/build2/cli/rule.cxx index cec8a60..bcc3a9f 100644 --- a/build2/cli/rule.cxx +++ b/build2/cli/rule.cxx @@ -56,7 +56,7 @@ namespace build2 // See if we have a .cli source file. // - match_result r; + bool r (false); for (prerequisite_member p: group_prerequisite_members (a, t)) { if (p.is_a ()) @@ -67,10 +67,10 @@ namespace build2 { l4 ([&]{trace << ".cli file stem '" << p.name () << "' " << "doesn't match target " << t;}); - return r; + return false; } - r = p; + r = true; break; } } @@ -113,7 +113,7 @@ namespace build2 // it is some other group, then we are definitely not a match. // if (t.group != nullptr) - return t.group->is_a (); + return t.group->is_a () != nullptr; // Check if there is a corresponding cli.cxx{} group. // @@ -164,12 +164,12 @@ namespace build2 } assert (t.group == g); - return g; + return g != nullptr; } } recipe compile:: - apply (action a, target& xt, const match_result& mr) const + apply (action a, target& xt) const { if (cli_cxx* pt = xt.is_a ()) { @@ -199,7 +199,7 @@ namespace build2 } else { - cli_cxx& g (*static_cast (mr.target)); + cli_cxx& g (*static_cast (xt.group)); build2::match (a, g); return group_recipe; // Execute the group's recipe. } diff --git a/build2/dist/rule b/build2/dist/rule index f428ff2..0c40cf0 100644 --- a/build2/dist/rule +++ b/build2/dist/rule @@ -20,10 +20,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string&) const; + match (action, target&, const string&) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; }; } } diff --git a/build2/dist/rule.cxx b/build2/dist/rule.cxx index e2bad9a..f6b50f7 100644 --- a/build2/dist/rule.cxx +++ b/build2/dist/rule.cxx @@ -16,13 +16,13 @@ namespace build2 namespace dist { match_result rule:: - match (action, target& t, const string&) const + match (action, target&, const string&) const { - return t; // We always match. + return true; // We always match. } recipe rule:: - apply (action a, target& t, const match_result&) const + apply (action a, target& t) const { const dir_path& out_root (t.root_scope ().out_path ()); diff --git a/build2/install/rule b/build2/install/rule index 4ad91bb..94ce65b 100644 --- a/build2/install/rule +++ b/build2/install/rule @@ -20,10 +20,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string&) const; + match (action, target&, const string&) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; }; struct install_dir; @@ -32,7 +32,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string&) const; + match (action, target&, const string&) const override; + + virtual recipe + apply (action, target&) const override; // Return NULL if this prerequisite should be ignored and pointer to its // target otherwise. The default implementation ignores prerequsites that @@ -41,9 +44,6 @@ namespace build2 virtual target* filter (action, target&, prerequisite_member) const; - virtual recipe - apply (action, target&, const match_result&) const; - // Extra installation hooks. // using install_dir = install::install_dir; diff --git a/build2/install/rule.cxx b/build2/install/rule.cxx index 68000af..c97dbb0 100644 --- a/build2/install/rule.cxx +++ b/build2/install/rule.cxx @@ -39,13 +39,13 @@ namespace build2 // alias_rule // match_result alias_rule:: - match (action, target& t, const string&) const + match (action, target&, const string&) const { - return t; + return true; } recipe alias_rule:: - apply (action a, target& t, const match_result&) const + apply (action a, target& t) const { tracer trace ("install::alias_rule::apply"); @@ -82,26 +82,39 @@ namespace build2 // file_rule // + struct match_data + { + bool install; + }; + + static_assert (sizeof (match_data) <= target::data_size, + "insufficient space"); + match_result file_rule:: match (action a, target& t, const string&) const { // First determine if this target should be installed (called // "installable" for short). // - if (lookup_install (t, "install") == nullptr) - // If this is the update pre-operation, signal that we don't match so - // that some other rule can take care of it. - // - return a.operation () == update_id ? nullptr : match_result (t, false); + match_data md {lookup_install (t, "install") != nullptr}; + match_result mr (true); - match_result mr (t, true); - - // If this is the update pre-operation, change the recipe action - // to (update, 0) (i.e., "unconditional update"). - // if (a.operation () == update_id) - mr.recipe_action = action (a.meta_operation (), update_id); + { + // If this is the update pre-operation and the target is installable, + // change the recipe action to (update, 0) (i.e., "unconditional + // update") so that we don't get matched for its prerequisites. + // + if (md.install) + mr.recipe_action = action (a.meta_operation (), update_id); + else + // Otherwise, signal that we don't match so that some other rule can + // take care of it. + // + return false; + } + t.data (md); // Save the data in the target's auxilary storage. return mr; } @@ -113,9 +126,12 @@ namespace build2 } recipe file_rule:: - apply (action a, target& t, const match_result& mr) const + apply (action a, target& t) const { - if (!mr.bvalue) // Not installable. + match_data md (move (t.data ())); + t.clear_data (); // In case delegated-to rule also uses aux storage. + + if (!md.install) // Not installable. return noop_recipe; // Ok, if we are here, then this means: diff --git a/build2/rule b/build2/rule index 6d4a580..f104c4f 100644 --- a/build2/rule +++ b/build2/rule @@ -13,60 +13,20 @@ namespace build2 { - // @@ This is an ugly mess that is overdue for redesign. Perhaps - // something similar to variable value storage? - // class match_result { public: - typedef build2::target target_type; - typedef build2::prerequisite prerequisite_type; - - // Can contain neither (both are NULL), one of, or both. If both - // are NULL, then it is a "no match" indicator. - // - // Note that if the "payload" is stored in *value instead of - // prerequisite, then target must not be NULL. - // - union - { - prerequisite_type* prerequisite; - - bool bvalue; - void* pvalue; - const void* cpvalue; - }; - - target_type* target; - + bool result; action recipe_action = action (); // Used as recipe's action if set. - match_result (nullptr_t v = nullptr): prerequisite (v), target (v) {} - match_result (prerequisite_type& p): prerequisite (&p), target (nullptr) {} - match_result (prerequisite_type* p): prerequisite (p), target (nullptr) {} - match_result (target_type& t): prerequisite (nullptr), target (&t) {} - match_result (target_type* t): prerequisite (nullptr), target (t) {} - match_result (const prerequisite_member& pm) - : prerequisite (&pm.prerequisite.get ()), target (pm.target) {} - - match_result (target_type& t, bool v): bvalue (v), target (&t) {} - match_result (target_type& t, void* v): pvalue (v), target (&t) {} - match_result (target_type& t, const void* v): cpvalue (v), target (&t) {} - match_result (target_type& t, nullptr_t v): pvalue (v), target (&t) {} - explicit - operator bool () const - { - return target != nullptr || prerequisite != nullptr; - } - - template - T& - as_target () const - { - return static_cast ( - target != nullptr ? *target : *prerequisite->target); - } + operator bool () const {return result;} + + // Note that the from-bool constructor is intentionally implicit so that + // we can return true/false from match(). + // + match_result (bool r): result (r) {} + match_result (bool r, action a): result (r), recipe_action (a) {} }; class rule @@ -76,7 +36,7 @@ namespace build2 match (action, target&, const string& hint) const = 0; virtual recipe - apply (action, target&, const match_result&) const = 0; + apply (action, target&) const = 0; }; // Fallback rule that on update verifies that the file exists and is @@ -86,10 +46,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; static target_state perform_update (action, target&); @@ -101,10 +61,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; static alias_rule instance; }; @@ -113,10 +73,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string& hint) const; + match (action, target&, const string& hint) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; static target_state perform_update (action, target&); @@ -133,10 +93,10 @@ namespace build2 { public: virtual match_result - match (action, target& t, const string&) const {return t;} + match (action, target&, const string&) const override {return true;} virtual recipe - apply (action, target&, const match_result&) const {return noop_recipe;} + apply (action, target&) const override {return noop_recipe;} static fallback_rule instance; }; diff --git a/build2/rule.cxx b/build2/rule.cxx index 58340b5..f9805db 100644 --- a/build2/rule.cxx +++ b/build2/rule.cxx @@ -63,18 +63,18 @@ namespace build2 } if (ts != timestamp_unknown && ts != timestamp_nonexistent) - return t; + return true; l4 ([&]{trace << "no existing file for target " << t;}); - return nullptr; + return false; } default: - return t; + return true; } } recipe file_rule:: - apply (action a, target& t, const match_result&) const + apply (action a, target& t) const { // Update triggers the update of this target's prerequisites // so it would seem natural that we should also trigger their @@ -143,13 +143,13 @@ namespace build2 // alias_rule // match_result alias_rule:: - match (action, target& t, const string&) const + match (action, target&, const string&) const { - return t; + return true; } recipe alias_rule:: - apply (action a, target& t, const match_result&) const + apply (action a, target& t) const { // Inject dependency on our directory (note: not parent) so that it is // automatically created on update and removed on clean. @@ -165,13 +165,13 @@ namespace build2 // fsdir_rule // match_result fsdir_rule:: - match (action, target& t, const string&) const + match (action, target&, const string&) const { - return t; + return true; } recipe fsdir_rule:: - apply (action a, target& t, const match_result&) const + apply (action a, target& t) const { // Inject dependency on the parent directory. Note that we don't do it for // clean since we shouldn't (and can't possibly, since it's our parent) be diff --git a/build2/target b/build2/target index 8c2b2da..f824689 100644 --- a/build2/target +++ b/build2/target @@ -766,6 +766,12 @@ namespace build2 as_prerequisite (tracer&) const; }; + // It is often stored as the target's auxiliary data so make sure there is + // no destructor overhead. + // + static_assert (std::is_trivially_destructible::value, + "prerequisite_member is not trivially destructible"); + inline ostream& operator<< (ostream& os, const prerequisite_member& pm) { diff --git a/build2/test/rule b/build2/test/rule index e6cdc37..b915ea7 100644 --- a/build2/test/rule +++ b/build2/test/rule @@ -19,10 +19,10 @@ namespace build2 { public: virtual match_result - match (action, target&, const string&) const; + match (action, target&, const string&) const override; virtual recipe - apply (action, target&, const match_result&) const; + apply (action, target&) const override; static target_state perform_script (action, target&); diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx index 975d19f..8621422 100644 --- a/build2/test/rule.cxx +++ b/build2/test/rule.cxx @@ -106,7 +106,7 @@ namespace build2 } } - match_result mr (t); + match_result mr (true); // Theoretically if this target is testable and this is the update // pre-operation, then all we need to do is say we are not a match and @@ -146,7 +146,7 @@ namespace build2 } recipe rule:: - apply (action a, target& t, const match_result&) const + apply (action a, target& t) const { tracer trace ("test::rule::apply"); -- cgit v1.1