From 1d6e68fda762535fa8508f94ca254a79f293edb2 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 24 Jul 2015 16:39:55 +0200 Subject: Add support for generated test input/output --- build/algorithm | 18 ++++++ build/algorithm.cxx | 40 +++++++------ build/algorithm.ixx | 27 ++++++--- build/b.cxx | 9 ++- build/bin/rule.cxx | 16 +----- build/cli/rule.cxx | 6 +- build/context | 3 +- build/context.cxx | 3 +- build/cxx/compile.cxx | 5 +- build/diagnostics.cxx | 57 ++++++++++++------- build/rule | 17 +++++- build/rule.cxx | 15 ++--- build/target | 22 +++++-- build/target.cxx | 4 +- build/test/rule.cxx | 155 ++++++++++++++++++++++++++++++++++++++------------ 15 files changed, 273 insertions(+), 124 deletions(-) (limited to 'build') diff --git a/build/algorithm b/build/algorithm index d01614c..3b09d9c 100644 --- a/build/algorithm +++ b/build/algorithm @@ -6,6 +6,7 @@ #define BUILD_ALGORITHM #include +#include // pair #include #include @@ -68,6 +69,14 @@ namespace build void match (action, target&); + // Match a "delegate rule" from withing another rules' apply() + // function. Return recipe and recipe action (if any). Note + // that unlike match(), this call doesn't increment the + // dependents count. See also execute_delegate(). + // + std::pair + match_delegate (action, target&); + // The standard prerequisite search and match implementations. They call // search_and_match_*() versions below passing non-empty directory for // the clean operation. @@ -116,6 +125,15 @@ namespace build target_state execute (action, target&); + // Execute the recipe obtained with match_delegate(). Note that + // the target's state is neither checked nor updated by this + // function. In other words, the appropriate usage is to call + // this function from another recipe and to factor the obtained + // state into the one returned. + // + target_state + execute_delegate (const recipe&, action, target&); + // A special version of the above that should be used for "direct" // and "now" execution, that is, side-stepping the normal target- // prerequisite relationship (so no dependents count is decremented) diff --git a/build/algorithm.cxx b/build/algorithm.cxx index 8893045..615d4b7 100644 --- a/build/algorithm.cxx +++ b/build/algorithm.cxx @@ -55,10 +55,10 @@ namespace build return search (*tt, move (n.dir), move (n.value), e, &s); } - match_result_impl + pair match_impl (action a, target& t, bool apply) { - match_result_impl r; + pair r; // Clear the resolved targets list before calling match(). The rule // is free to, say, resize() this list in match() (provided that it @@ -148,6 +148,9 @@ namespace build if (!(m = ru.match (ra, t, hint))) continue; + + if (!m.recipe_action.valid ()) + m.recipe_action = ra; // Default, if not set. } // Do the ambiguity test. @@ -187,6 +190,8 @@ namespace build if (!ambig) { + ra = m.recipe_action; // Use custom, if set. + if (apply) { auto g ( @@ -198,13 +203,15 @@ namespace build }, ra, t, n)); + // @@ 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)); } else { - r.ra = ra; - r.ru = &ru; - r.mr = m; + r.first = &ru; + r.second = move (m); } return r; @@ -235,7 +242,7 @@ namespace build // if (!g.recipe (a)) { - auto mir (match_impl (a, g, false)); + auto rp (match_impl (a, g, false)); r = g.group_members (a); if (r.members != nullptr) @@ -244,7 +251,8 @@ namespace build // That didn't help, so apply the rule and go to the building // phase. // - g.recipe (mir.ra, mir.ru->apply (mir.ra, g, mir.mr)); + const match_result& mr (rp.second); + g.recipe (mr.recipe_action, rp.first->apply (mr.recipe_action, g, mr)); } // Note that we use execute_direct() rather than execute() here to @@ -375,10 +383,7 @@ namespace build if (pt == nullptr) // Skipped. continue; - target_state ts (execute (a, *pt)); - if (ts == target_state::postponed || - (ts == target_state::changed && r == target_state::unchanged)) - r = ts; + r |= execute (a, *pt); } return r; @@ -394,10 +399,7 @@ namespace build if (pt == nullptr) // Skipped. continue; - target_state ts (execute (a, *pt)); - if (ts == target_state::postponed || - (ts == target_state::changed && r == target_state::unchanged)) - r = ts; + r |= execute (a, *pt); } return r; @@ -481,7 +483,9 @@ namespace build // file& ft (dynamic_cast (t)); - bool r (rmfile (ft.path (), ft)); + target_state r (rmfile (ft.path (), ft) + ? target_state::changed + : target_state::unchanged); // Update timestamp in case there are operations after us that // could use the information. @@ -490,8 +494,8 @@ namespace build // Clean prerequisites. // - target_state ts (reverse_execute_prerequisites (a, t)); + r |= reverse_execute_prerequisites (a, t); - return r && ts != target_state::postponed ? target_state::changed : ts; + return r; } } diff --git a/build/algorithm.ixx b/build/algorithm.ixx index 7849713..26761bf 100644 --- a/build/algorithm.ixx +++ b/build/algorithm.ixx @@ -2,6 +2,8 @@ // copyright : Copyright (c) 2014-2015 Code Synthesis Ltd // license : MIT; see accompanying LICENSE file +#include // pair + #include #include #include @@ -48,15 +50,7 @@ namespace build return static_cast (search (T::static_type, dir, name, ext, scope)); } - - struct match_result_impl - { - action ra; // Action to set recipe, not to pass to apply(). - const rule* ru; - match_result mr; - }; - - match_result_impl + std::pair match_impl (action, target&, bool apply); inline void @@ -68,6 +62,15 @@ namespace build t.dependents++; } + inline std::pair + match_delegate (action a, target& t) + { + auto rp (match_impl (a, t, false)); + const match_result& mr (rp.second); + return std::make_pair (rp.first->apply (mr.recipe_action, t, mr), + mr.recipe_action); + } + group_view resolve_group_members_impl (action, target&); @@ -153,6 +156,12 @@ namespace build } inline target_state + execute_delegate (const recipe& r, action a, target& t) + { + return r (a, t); + } + + inline target_state execute_direct (action a, target& t) { switch (target_state ts = t.state ()) diff --git a/build/b.cxx b/build/b.cxx index 3a6d71f..21e72cf 100644 --- a/build/b.cxx +++ b/build/b.cxx @@ -739,7 +739,8 @@ main (int argc, char* argv[]) if (mif->operation_pre != nullptr) mif->operation_pre (pre_oid); // Cannot be translated. - current_oif = pre_oif; + current_inner_oif = pre_oif; + current_outer_oif = oif; current_mode = pre_oif->mode; action a (mid, pre_oid, oid); @@ -754,7 +755,8 @@ main (int argc, char* argv[]) << ", id " << static_cast (pre_oid);}); } - current_oif = oif; + current_inner_oif = oif; + current_outer_oif = nullptr; current_mode = oif->mode; action a (mid, oid, 0); @@ -770,7 +772,8 @@ main (int argc, char* argv[]) if (mif->operation_pre != nullptr) mif->operation_pre (post_oid); // Cannot be translated. - current_oif = post_oif; + current_inner_oif = post_oif; + current_outer_oif = oif; current_mode = post_oif->mode; action a (mid, post_oid, oid); diff --git a/build/bin/rule.cxx b/build/bin/rule.cxx index f02029b..0a7f2ee 100644 --- a/build/bin/rule.cxx +++ b/build/bin/rule.cxx @@ -114,23 +114,13 @@ namespace build if (current_mode == execution_mode::last) swap (m1, m2); - target_state r (target_state::unchanged), ts; + target_state r (target_state::unchanged); if (m1 != nullptr) - { - ts = execute (a, *m1); - if (ts == target_state::postponed || - (ts == target_state::changed && r == target_state::unchanged)) - r = ts; - } + r |= execute (a, *m1); if (m2 != nullptr) - { - ts = execute (a, *m2); - if (ts == target_state::postponed || - (ts == target_state::changed && r == target_state::unchanged)) - r = ts; - } + r |= execute (a, *m2); return r; } diff --git a/build/cli/rule.cxx b/build/cli/rule.cxx index dff13f0..d1cb616 100644 --- a/build/cli/rule.cxx +++ b/build/cli/rule.cxx @@ -290,11 +290,13 @@ namespace build t.mtime (timestamp_nonexistent); + target_state ts (r ? target_state::changed : target_state::unchanged); + // Clean prerequisites. // - target_state ts (reverse_execute_prerequisites (a, t)); + ts |= reverse_execute_prerequisites (a, t); - return r && ts != target_state::postponed ? target_state::changed : ts; + return ts; } } } diff --git a/build/context b/build/context index ad9c091..c8200b7 100644 --- a/build/context +++ b/build/context @@ -28,7 +28,8 @@ namespace build // Current action (meta/operation). // extern const meta_operation_info* current_mif; - extern const operation_info* current_oif; + extern const operation_info* current_inner_oif; + extern const operation_info* current_outer_oif; extern execution_mode current_mode; diff --git a/build/context.cxx b/build/context.cxx index 9d9c322..f0e853d 100644 --- a/build/context.cxx +++ b/build/context.cxx @@ -25,7 +25,8 @@ namespace build string_pool project_name_pool; const meta_operation_info* current_mif; - const operation_info* current_oif; + const operation_info* current_inner_oif; + const operation_info* current_outer_oif; execution_mode current_mode; void diff --git a/build/cxx/compile.cxx b/build/cxx/compile.cxx index a12650f..4095598 100644 --- a/build/cxx/compile.cxx +++ b/build/cxx/compile.cxx @@ -642,10 +642,7 @@ namespace build // them is fallback file_rule. So we are going to do a little // fast-path optimization by detecting this common case. // - recipe_function* const* recipe ( - pt.recipe (a).target ()); - - if (recipe == nullptr || *recipe != &file_rule::perform_update) + if (!file_rule::uptodate (a, pt)) { // We only want to restart if our call to execute() actually // caused an update. In particular, the target could already diff --git a/build/diagnostics.cxx b/build/diagnostics.cxx index a254860..2f9525b 100644 --- a/build/diagnostics.cxx +++ b/build/diagnostics.cxx @@ -84,24 +84,28 @@ namespace build string diag_do (const action&, const target& t) { - const meta_operation_info& mi (*current_mif); - const operation_info& oi (*current_oif); + const meta_operation_info& m (*current_mif); + const operation_info& io (*current_inner_oif); + const operation_info* oo (current_outer_oif); ostringstream os; // perform(update(x)) -> "update x" // configure(update(x)) -> "configure updating x" // - if (mi.name_do.empty ()) - os << oi.name_do << ' '; + if (m.name_do.empty ()) + os << io.name_do << ' '; else { - os << mi.name_do << ' '; + os << m.name_do << ' '; - if (!oi.name_doing.empty ()) - os << oi.name_doing << ' '; + if (!io.name_doing.empty ()) + os << io.name_doing << ' '; } + if (oo != nullptr) + os << "(for " << oo->name << ") "; + os << t; return os.str (); } @@ -109,19 +113,23 @@ namespace build string diag_doing (const action&, const target& t) { - const meta_operation_info& mi (*current_mif); - const operation_info& oi (*current_oif); + const meta_operation_info& m (*current_mif); + const operation_info& io (*current_inner_oif); + const operation_info* oo (current_outer_oif); ostringstream os; // perform(update(x)) -> "updating x" // configure(update(x)) -> "configuring updating x" // - if (!mi.name_doing.empty ()) - os << mi.name_doing << ' '; + if (!m.name_doing.empty ()) + os << m.name_doing << ' '; + + if (!io.name_doing.empty ()) + os << io.name_doing << ' '; - if (!oi.name_doing.empty ()) - os << oi.name_doing << ' '; + if (oo != nullptr) + os << "(for " << oo->name << ") "; os << t; return os.str (); @@ -130,27 +138,34 @@ namespace build string diag_done (const action&, const target& t) { - const meta_operation_info& mi (*current_mif); - const operation_info& oi (*current_oif); + const meta_operation_info& m (*current_mif); + const operation_info& io (*current_inner_oif); + const operation_info* oo (current_outer_oif); ostringstream os; // perform(update(x)) -> "x is up to date" // configure(update(x)) -> "updating x is configured" // - if (mi.name_done.empty ()) + if (m.name_done.empty ()) { os << t; - if (!oi.name_done.empty ()) - os << " " << oi.name_done; + if (!io.name_done.empty ()) + os << " " << io.name_done; + + if (oo != nullptr) + os << "(for " << oo->name << ") "; } else { - if (!oi.name_doing.empty ()) - os << oi.name_doing << ' '; + if (!io.name_doing.empty ()) + os << io.name_doing << ' '; + + if (oo != nullptr) + os << "(for " << oo->name << ") "; - os << t << " " << mi.name_done; + os << t << " " << m.name_done; } return os.str (); diff --git a/build/rule b/build/rule index ea0b5ae..a144a9b 100644 --- a/build/rule +++ b/build/rule @@ -34,8 +34,9 @@ namespace build target_type* target; - match_result (target_type& t, bool v): value (v), target (&t) {} + action recipe_action = action (); // Used as recipe's action if set. + match_result (target_type& t, bool v): value (v), target (&t) {} match_result (std::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) {} @@ -76,6 +77,20 @@ namespace build static target_state perform_update (action, target&); + // Sometimes it is useful, normally as an optimization, to check + // if the file target is up to date. In particular, if the rule + // that matched a target is this fallback rule and the target + // has no prerequisites, then it means it is up to date. + // + static bool + uptodate (action a, target& t) + { + recipe_function* const* r (t.recipe (a).target ()); + return r != nullptr && *r == &perform_update && + t.prerequisites.empty () && + (t.group == nullptr || t.group->prerequisites.empty ()); + } + static file_rule instance; }; diff --git a/build/rule.cxx b/build/rule.cxx index f9b9a3c..82ce993 100644 --- a/build/rule.cxx +++ b/build/rule.cxx @@ -213,7 +213,7 @@ namespace build fail << "unable to create directory " << d << ": " << e.what (); } - ts = target_state::changed; + ts |= target_state::changed; } return ts; @@ -229,22 +229,19 @@ namespace build target_state ts (target_state::unchanged); if (t.has_prerequisites ()) - { ts = reverse_execute_prerequisites (a, t); - if (ts == target_state::postponed) - return ts; - } - // If we couldn't remove the directory, return postponed meaning // that the operation could not be performed at this time. // switch (rs) { - case rmdir_status::success: return target_state::changed; - case rmdir_status::not_empty: return target_state::postponed; - default: return ts; + case rmdir_status::success: ts |= target_state::changed; + case rmdir_status::not_empty: ts |= target_state::postponed; + default: break; } + + return ts; } fsdir_rule fsdir_rule::instance; diff --git a/build/target b/build/target index f5a26b4..6ee65f4 100644 --- a/build/target +++ b/build/target @@ -10,6 +10,7 @@ #include #include // unique_ptr #include // size_t +#include // uint8_t #include // reference_wrapper #include #include @@ -38,19 +39,32 @@ namespace build // Target state. // - enum class target_state + enum class target_state: std::uint8_t { - group, // Target's state is the group's state. + // The order of the enumerators is arranged so that their + // inegral values indicate whether one "overrides" the other + // in the merge operator (see below). + // unknown, - postponed, unchanged, changed, - failed + postponed, + failed, + group // Target's state is the group's state. }; std::ostream& operator<< (std::ostream&, target_state); + inline target_state& + operator |= (target_state& l, target_state r) + { + if (static_cast (r) > static_cast (l)) + l = r; + + return l; + } + // Recipe. // // The returned target state should be changed, unchanged, or diff --git a/build/target.cxx b/build/target.cxx index f6fc5c8..df7e2aa 100644 --- a/build/target.cxx +++ b/build/target.cxx @@ -32,12 +32,12 @@ namespace build // target_state // static const char* target_state_[] = { - "group", "unknown", "postponed", "unchanged", "changed", "failed"}; + "unknown", "unchanged", "changed", "postponed", "failed", "group"}; ostream& operator<< (ostream& os, target_state ts) { - return os << target_state_[static_cast (ts)]; + return os << target_state_[static_cast (ts)]; } // recipe diff --git a/build/test/rule.cxx b/build/test/rule.cxx index b8d7428..da17d61 100644 --- a/build/test/rule.cxx +++ b/build/test/rule.cxx @@ -50,7 +50,7 @@ namespace build // is a test. // if (var.name == "test.input" || - var.name == "test.ouput" || + var.name == "test.output" || var.name == "test.roundtrip" || var.name == "test.options" || var.name == "test.arguments") @@ -68,19 +68,34 @@ namespace build v.rebind (t.base_scope ()[string("test.") + t.type ().name]); r = v && v.as (); - } - if (!r) - return match_result (t, false); // "Not a test" result. + // If this is the update pre-operation, then all we really need to + // do is say we are not a match and the standard matching machinery + // will (hopefully) find the rule to update this target. + // + // There is one thing that compilates this simple approach: test + // input/output. While normally they will be existing (in src_base) + // files, they could also be auto-generated. In fact, they could + // only be needed for testing, which means the normall update won't + // even know about them (nor clean, for that matter; this is why we + // need cleantest). + // + // To make generated input/output work we will have to cause their + // update ourselves. I other words, we may have to do some actual + // work for (update, test), and not simply "guide" (update, 0) as + // to which targets need updating. For how exactly we are going to + // do it, see apply() below. + // + match_result mr (t, r); - // If this is the update pre-operation, make someone else do - // the job. + // If this is the update pre-operation, change the recipe action + // to (update, 0) (i.e., "unconditional update"). // - if (a.operation () != test_id) - return nullptr; + if (r && a.operation () == update_id) + mr.recipe_action = action (a.meta_operation (), update_id); - return match_result (t, true); + return mr; } recipe rule:: @@ -91,13 +106,24 @@ namespace build if (!mr.value) // Not a test. return noop_recipe; - // Don't do anything for other meta-operations. + // In case of test, we don't do anything for other meta-operations. // - if (a != action (perform_id, test_id)) + if (a.operation () == test_id && a.meta_operation () != perform_id) return noop_recipe; - // See if we have test.{input,output,roundtrip}. First check the - // target-specific vars since they override any scope ones. + // Ok, if we are here, then this means: + // + // 1. This target is a test. + // 2. The action is either + // a. (perform, test, 0) or + // b. (*, update, 0) + // + // In both cases, the next step is to see if we have test.{input, + // output,roundtrip}. + // + + // First check the target-specific vars since they override any + // scope ones. // auto iv (t.vars["test.input"]); auto ov (t.vars["test.output"]); @@ -147,8 +173,8 @@ namespace build } } - const name* i; - const name* o; + const name* in; + const name* on; // Reduce the roundtrip case to input/output. // @@ -158,35 +184,92 @@ namespace build fail << "both test.roundtrip and test.input/output specified " << "for target " << t; - i = o = rv.as (); + in = on = rv.as (); } else { - i = iv ? iv.as () : nullptr; - o = ov ? ov.as () : nullptr; + in = iv ? iv.as () : nullptr; + on = ov ? ov.as () : nullptr; } - // Resolve them to targets (normally just files) and cache in - // our prerequsite targets lists where they can be found by - // perform_test(). If we have either or both, then the first - // entry is input and the second -- output (either can be NULL). + // Resolve them to targets, which normally would be existing files + // but could also be targets that need updating. // - auto& pts (t.prerequisite_targets); + target* it (in != nullptr ? &search (*in, bs) : nullptr); + target* ot (on != nullptr ? in == on ? it : &search (*on, bs) : nullptr); - if (i != nullptr || o != nullptr) - pts.resize (2, nullptr); + if (a.operation () == update_id) + { + // First see if input/output are existing, up-to-date files. This + // is a common case optimization. + // + if (it != nullptr) + { + build::match (a, *it); - //@@ We should match() them, but for update, not test. - //@@ If not doing this for some reason, need to then verify - // path was assigned (i.e., matched an existing file). - // - if (i != nullptr) - pts[0] = &search (*i, bs); + if (file_rule::uptodate (a, *it)) + it = nullptr; + } + + if (ot != nullptr && in == on) + { + build::match (a, *ot); + + if (file_rule::uptodate (a, *ot)) + ot = nullptr; + } + else + ot = it; + + + // Find the "real" update rule, that is, the rule that would + // have been found if we signalled that we do not match from + // match() above. + // + recipe d (match_delegate (a, t).first); + + // If we have no input/output that needs updating, then simply + // redirect to it + // + if (it == nullptr && ot == nullptr) + return d; + + // Ok, time to handle the worst case scenario: we need to + // cause update of input/output targets and also delegate + // to the real update. + // + return [it, ot, dr = move (d)] (action a, target& t) -> target_state + { + // Do the general update first. + // + target_state r (execute_delegate (dr, a, t)); - if (o != nullptr) - pts[1] = i == o ? pts[0] : &search (*o, bs); + if (it != nullptr) + r |= execute (a, *it); - return &perform_test; + if (ot != nullptr) + r |= execute (a, *ot); + + return r; + }; + } + else + { + // Cache the targets in our prerequsite targets lists where they + // can be found by perform_test(). If we have either or both, + // then the first entry is input and the second -- output (either + // can be NULL). + // + if (it != nullptr || ot != nullptr) + { + auto& pts (t.prerequisite_targets); + pts.resize (2, nullptr); + pts[0] = it; + pts[1] = ot; + } + + return &perform_test; + } } static void @@ -310,7 +393,7 @@ namespace build if (pts.size () != 0 && pts[0] != nullptr) { file& it (static_cast (*pts[0])); - assert (!it.path ().empty ()); // Should have been assigned. + assert (!it.path ().empty ()); // Should have been assigned by update. args.push_back (it.path ().string ().c_str ()); } // Maybe arguments then? @@ -325,7 +408,7 @@ namespace build if (pts.size () != 0 && pts[1] != nullptr) { file& ot (static_cast (*pts[1])); - assert (!ot.path ().empty ()); // Should have been assigned. + assert (!ot.path ().empty ()); // Should have been assigned by update. args.push_back ("diff"); args.push_back ("-u"); -- cgit v1.1