From b259a318223370881d5244cc38ff8a7be58e2a3e Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Sun, 3 Dec 2023 13:47:42 +0200 Subject: Reimplement search_existing() functions via target_type::search This allows us to automatically get the target type-specific behavior with regards to the out_only semantics (added in the previous commit) instead of passing it explicitly from each call site. --- libbuild2/algorithm.cxx | 22 ++++++++++------- libbuild2/algorithm.hxx | 10 +++----- libbuild2/algorithm.ixx | 2 +- libbuild2/bin/init.cxx | 4 +-- libbuild2/bin/target.cxx | 6 ++--- libbuild2/build/script/parser.cxx | 2 ++ libbuild2/cc/guess.cxx | 2 +- libbuild2/cc/link-rule.cxx | 2 +- libbuild2/dist/rule.cxx | 4 +-- libbuild2/dyndep.cxx | 2 +- libbuild2/functions-name.cxx | 3 +++ libbuild2/in/target.cxx | 10 ++++---- libbuild2/target-type.hxx | 9 ++++++- libbuild2/target.cxx | 52 ++++++++++++++++++++++----------------- libbuild2/target.hxx | 17 +++++++++---- libbuild2/test/common.cxx | 6 ++--- libbuild2/test/rule.cxx | 2 +- libbuild2/test/script/script.cxx | 2 +- 18 files changed, 91 insertions(+), 66 deletions(-) (limited to 'libbuild2') diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 71bf134..8b7aa28 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -54,19 +54,21 @@ namespace build2 const target& search (const target& t, const prerequisite_key& pk) { - assert (t.ctx.phase == run_phase::match); + context& ctx (t.ctx); + + assert (ctx.phase == run_phase::match); // If this is a project-qualified prerequisite, then this is import's // business (phase 2). // if (pk.proj) - return import2 (t.ctx, pk); + return import2 (ctx, pk); - if (const target* pt = pk.tk.type->search (t, pk)) + if (const target* pt = pk.tk.type->search (ctx, &t, pk)) return *pt; if (pk.tk.out->empty ()) - return create_new_target (t.ctx, pk); + return create_new_target (ctx, pk); // If this is triggered, then you are probably not passing scope to // search() (which leads to search_existing_file() being skipped). @@ -77,13 +79,15 @@ namespace build2 pair search_locked (const target& t, const prerequisite_key& pk) { - assert (t.ctx.phase == run_phase::match && !pk.proj); + context& ctx (t.ctx); + + assert (ctx.phase == run_phase::match && !pk.proj); - if (const target* pt = pk.tk.type->search (t, pk)) + if (const target* pt = pk.tk.type->search (ctx, &t, pk)) return {const_cast (*pt), ulock ()}; if (pk.tk.out->empty ()) - return create_new_target_locked (t.ctx, pk); + return create_new_target_locked (ctx, pk); // If this is triggered, then you are probably not passing scope to // search() (which leads to search_existing_file() being skipped). @@ -96,7 +100,7 @@ namespace build2 { return pk.proj ? import_existing (ctx, pk) - : search_existing_target (ctx, pk, false /*out_only*/); // @@ TODO + : pk.tk.type->search (ctx, nullptr /* existing */, pk); } const target& @@ -181,7 +185,7 @@ namespace build2 return q ? import_existing (s.ctx, pk) - : search_existing_target (s.ctx, pk, false /*out_only*/); // @@ TODO + : tt->search (s.ctx, nullptr /* existing */, pk); } const target* diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx index a0f8cd6..456862d 100644 --- a/libbuild2/algorithm.hxx +++ b/libbuild2/algorithm.hxx @@ -79,7 +79,7 @@ namespace build2 search_locked (const target&, const target_type&, const prerequisite_key&); const target* - search_exsiting (context&, const target_type&, const prerequisite_key&); + search_existing (context&, const target_type&, const prerequisite_key&); const target& search_new (context&, const target_type&, const prerequisite_key&); @@ -166,13 +166,11 @@ namespace build2 LIBBUILD2_SYMEXPORT const target& search (const target&, name&&, const scope&, const target_type* = nullptr); - // Note: returns NULL for unknown target types. Note that unlike the above - // version, these ones can be called during the load and execute phases. + // Note: returns NULL for unknown target types. Note also that unlike the + // above version, these can be called during the load and execute phases. // LIBBUILD2_SYMEXPORT const target* - search_existing (const name&, - const scope&, - const dir_path& out = dir_path ()); + search_existing (const name&, const scope&, const dir_path& out = dir_path ()); LIBBUILD2_SYMEXPORT const target* search_existing (const names&, const scope&); diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx index c7b4b7e..0d931dc 100644 --- a/libbuild2/algorithm.ixx +++ b/libbuild2/algorithm.ixx @@ -46,7 +46,7 @@ namespace build2 } inline const target* - search_exsiting (context& ctx, + search_existing (context& ctx, const target_type& tt, const prerequisite_key& k) { diff --git a/libbuild2/bin/init.cxx b/libbuild2/bin/init.cxx index 27e0b77..1b3039f 100644 --- a/libbuild2/bin/init.cxx +++ b/libbuild2/bin/init.cxx @@ -559,7 +559,7 @@ namespace build2 nullptr, /* default_extension */ &target_pattern_fix, &target_print_0_ext_verb, // Fixed extension, no use printing. - &file_search, + &target_search, // Note: don't look for an existing file. target_type::flag::none})); if (install_loaded) @@ -955,7 +955,7 @@ namespace build2 nullptr, /* default_extension */ &target_pattern_fix, &target_print_0_ext_verb, // Fixed extension, no use printing. - &file_search, + &target_search, // Note: don't look for an existing file. target_type::flag::none})); if (cast_false (rs["install.loaded"])) diff --git a/libbuild2/bin/target.cxx b/libbuild2/bin/target.cxx index 38572ef..7e4875a 100644 --- a/libbuild2/bin/target.cxx +++ b/libbuild2/bin/target.cxx @@ -374,7 +374,7 @@ namespace build2 &target_extension_var, &target_pattern_var, nullptr, - &file_search, + &target_search, // Note: not _file(); don't look for an existing file. target_type::flag::none }; @@ -387,7 +387,7 @@ namespace build2 &target_extension_var, &target_pattern_var, nullptr, - &file_search, + &target_search, // Note: not _file(); don't look for an existing file. target_type::flag::none }; @@ -452,7 +452,7 @@ namespace build2 &target_extension_var, &target_pattern_var, nullptr, - &file_search, + &target_search, // Note: not _file(); don't look for an existing file. target_type::flag::none }; diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index bb9ff66..2298780 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -1147,6 +1147,8 @@ namespace build2 { if (!qs) { + // This could be a script from src so search like a prerequisite. + // if (const target* t = search_existing ( ns[0], *scope_, ns[0].pair ? ns[1].dir : empty_dir_path)) { diff --git a/libbuild2/cc/guess.cxx b/libbuild2/cc/guess.cxx index b38db38..48240d4 100644 --- a/libbuild2/cc/guess.cxx +++ b/libbuild2/cc/guess.cxx @@ -2708,7 +2708,7 @@ namespace build2 const char* cpu (msvc_cpu (tt.cpu)); // Come up with the system library search paths. Ideally we would want - // to extract this from Clang and -print-search-paths would have been + // to extract this from Clang and -print-search-dirs would have been // the natural way for Clang to report it. But no luck. // lib_dirs = msvc_lib (mi, x_mo, cpu); diff --git a/libbuild2/cc/link-rule.cxx b/libbuild2/cc/link-rule.cxx index e3e89b6..499f867 100644 --- a/libbuild2/cc/link-rule.cxx +++ b/libbuild2/cc/link-rule.cxx @@ -96,7 +96,7 @@ namespace build2 return false; } - if (const target* t = search_existing (n, bs, dir_path () /* out */)) + if (const target* t = search_existing (n, bs)) { // The same logic as in process_libraries(). // diff --git a/libbuild2/dist/rule.cxx b/libbuild2/dist/rule.cxx index 320d17a..c63f7f3 100644 --- a/libbuild2/dist/rule.cxx +++ b/libbuild2/dist/rule.cxx @@ -87,7 +87,7 @@ namespace build2 // Note: see also similar code in match_postponed() below. // const prerequisite_key& k (p.key ()); - pt = k.tk.type->search (t, k); + pt = k.tk.type->search (t.ctx, &t, k); if (pt == nullptr) { @@ -134,7 +134,7 @@ namespace build2 const prerequisite& p (pp.prereq); const prerequisite_key& k (p.key ()); - const target* pt (k.tk.type->search (t, k)); + const target* pt (k.tk.type->search (t.ctx, &t, k)); if (pt == nullptr) { diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index c6294bb..68260fb 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -639,7 +639,7 @@ namespace build2 #else prerequisite_key pk {nullopt, {tts[0], &d, &out, &n, move (e)}, s}; - r = pk.tk.type->search (t, pk); + r = pk.tk.type->search (ctx, &t, pk); if (r == nullptr && pk.tk.out->empty ()) r = &create_new_target (ctx, pk); diff --git a/libbuild2/functions-name.cxx b/libbuild2/functions-name.cxx index 7dddc3a..c11b4b5 100644 --- a/libbuild2/functions-name.cxx +++ b/libbuild2/functions-name.cxx @@ -45,6 +45,9 @@ namespace build2 const target& to_target (const scope& s, name&& n, name&& o) { + // Note: help the user out and search in both out and src like a + // prerequisite. + // if (const target* r = search_existing (n, s, o.dir)) return *r; diff --git a/libbuild2/in/target.cxx b/libbuild2/in/target.cxx index 54130ff..d664e3a 100644 --- a/libbuild2/in/target.cxx +++ b/libbuild2/in/target.cxx @@ -10,7 +10,7 @@ namespace build2 namespace in { static const target* - in_search (const target& xt, const prerequisite_key& cpk) + in_search (context& ctx, const target* xt, const prerequisite_key& cpk) { // If we have no extension then derive it from our target. Then delegate // to file_search(). @@ -18,7 +18,7 @@ namespace build2 prerequisite_key pk (cpk); optional& e (pk.tk.ext); - if (!e) + if (!e && xt != nullptr) { // Why is the extension, say, .h.in and not .in (with .h being in the // name)? While this is mostly academic (in this case things will work @@ -28,16 +28,16 @@ namespace build2 // // See also the low verbosity tidying up code in the rule. // - if (const file* t = xt.is_a ()) + if (const file* t = xt->is_a ()) { const string& te (t->derive_extension ()); e = te + (te.empty () ? "" : ".") + "in"; } else - fail << "prerequisite " << pk << " for a non-file target " << xt; + fail << "prerequisite " << pk << " for a non-file target " << *xt; } - return file_search (xt, pk); + return file_search (ctx, xt, pk); } static bool diff --git a/libbuild2/target-type.hxx b/libbuild2/target-type.hxx index a0fc5a2..c0a5571 100644 --- a/libbuild2/target-type.hxx +++ b/libbuild2/target-type.hxx @@ -93,7 +93,14 @@ namespace build2 // bool (*print) (ostream&, const target_key&, bool name_only); - const target* (*search) (const target&, const prerequisite_key&); + // Target type-specific prerequisite to target search. + // + // If passed target is NULL, then only search for an existing target (and + // which can be performed during execute, not only match). + // + const target* (*search) (context&, + const target*, + const prerequisite_key&); // Target type flags. // diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index 1d21c07..5a352a8 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -1161,31 +1161,27 @@ namespace build2 // const target* - target_search (const target& t, const prerequisite_key& pk) + target_search (context& ctx, const target*, const prerequisite_key& pk) { // The default behavior is to look for an existing target in the // prerequisite's directory scope. // - // Note that it would be reasonable to assume that such a target can only - // be found in the out tree (targets that can be in the src tree should - // use file_search()). But omitting the src search will make it hard to - // keep search() (which calls this) and search_existing() (which does not) - // consistent. - // - return search_existing_target (t.ctx, pk, false /* out_only */); + return search_existing_target (ctx, pk, true /* out_only */); } const target* - file_search (const target& t, const prerequisite_key& pk) + file_search (context& ctx, const target* t, const prerequisite_key& pk) { // First see if there is an existing target in the out or src tree. // - if (const target* e = search_existing_target (t.ctx, pk, false /*out_only*/)) + if (const target* e = search_existing_target (ctx, + pk, + false /* out_only */)) return e; // Then look for an existing file in the src tree. // - return search_existing_file (t.ctx, pk); + return t != nullptr ? search_existing_file (ctx, pk) : nullptr; } extern const char target_extension_none_[] = ""; @@ -1332,10 +1328,12 @@ namespace build2 // alias // static const target* - alias_search (const target& t, const prerequisite_key& pk) + alias_search (context& ctx, const target* t, const prerequisite_key& pk) { // For an alias we don't want to silently create a target since it will do - // nothing and it most likely not what the user intended. + // nothing and it most likely not what the user intended (but omit this + // check when searching for an existing target since presumably a new one + // won't be created in this case). // // But, allowing implied aliases seems harmless since all the alias does // is pull its prerequisites. And they are handy to use as metadata @@ -1343,9 +1341,10 @@ namespace build2 // // Doesn't feel like an alias in the src tree makes much sense. // - const target* e (search_existing_target (t.ctx, pk, true /* out_only */)); + const target* e (search_existing_target (ctx, pk, true /* out_only */)); - if (e == nullptr || !(operator>= (e->decl, target_decl::implied))) + if ((e == nullptr || + !(operator>= (e->decl, target_decl::implied))) && t != nullptr) fail << "no explicit target for " << pk; return e; @@ -1452,7 +1451,7 @@ namespace build2 } static const target* - dir_search (const target& t, const prerequisite_key& pk) + dir_search (context& ctx, const target* t, const prerequisite_key& pk) { tracer trace ("dir_search"); @@ -1461,11 +1460,18 @@ namespace build2 // // Likewise, dir{} in the src tree doesn't make much sense. // - const target* e (search_existing_target (t.ctx, pk, true /* out_only */)); + const target* e (search_existing_target (ctx, pk, true /* out_only */)); if (e != nullptr && e->decl == target_decl::real) return e; + // The search for an existing target can also be done during execute so + // none of the below code applied. Note: return implied instead of NULL + // (to be consistent with search_new(), for example). + // + if (t == nullptr) + return e; + // If not found (or is implied), then try to load the corresponding // buildfile (which would normally define this target). Failed that, see // if we can assume an implied buildfile which would be equivalent to: @@ -1499,18 +1505,18 @@ namespace build2 // bool retest (false); - assert (t.ctx.phase == run_phase::match); + assert (ctx.phase == run_phase::match); { // Switch the phase to load. // - phase_switch ps (t.ctx, run_phase::load); + phase_switch ps (ctx, run_phase::load); // This is subtle: while we were fussing around another thread may have // loaded the buildfile. So re-test now that we are in an exclusive // phase. // if (e == nullptr) - e = search_existing_target (t.ctx, pk, true); + e = search_existing_target (ctx, pk, true); if (e != nullptr && e->decl == target_decl::real) retest = true; @@ -1548,14 +1554,14 @@ namespace build2 } } - assert (t.ctx.phase == run_phase::match); + assert (ctx.phase == run_phase::match); // If we loaded/implied the buildfile, examine the target again. // if (retest) { if (e == nullptr) - e = search_existing_target (t.ctx, pk, true); + e = search_existing_target (ctx, pk, true); if (e != nullptr && e->decl == target_decl::real) return e; @@ -1680,7 +1686,7 @@ namespace build2 nullptr, #endif nullptr, - &file_search, + &file_search, // Note: can also be a script in src. target_type::flag::none }; diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 5283ab8..e3152ac 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -2619,7 +2619,7 @@ namespace build2 string&, optional&, const location&, bool); - // Target print functions. + // Target print functions (target_type::print). // // Target type uses the extension but it is fixed and there is no use @@ -2634,17 +2634,24 @@ namespace build2 LIBBUILD2_SYMEXPORT bool target_print_1_ext_verb (ostream&, const target_key&, bool); + // Target search functions (target_type::search). + // + // The default behavior, that is, look for an existing target in the // prerequisite's directory scope. // + // Note that this implementation assumes a target can only be found in the + // out tree (targets that can be in the src tree would normally use + // file_search() below). + // LIBBUILD2_SYMEXPORT const target* - target_search (const target&, const prerequisite_key&); + target_search (context&, const target*, const prerequisite_key&); - // First look for an existing target as above. If not found, then look - // for an existing file in the target-type-specific list of paths. + // First look for an existing target both in out and src. If not found, then + // look for an existing file in src. // LIBBUILD2_SYMEXPORT const target* - file_search (const target&, const prerequisite_key&); + file_search (context&, const target*, const prerequisite_key&); } #include diff --git a/libbuild2/test/common.cxx b/libbuild2/test/common.cxx index 7fdb347..89f3dd6 100644 --- a/libbuild2/test/common.cxx +++ b/libbuild2/test/common.cxx @@ -150,8 +150,7 @@ namespace build2 t.name == n->value && // Name matches. tt.name == n->type && // Target type matches. d == n->dir && // Directory matches. - (search_existing (*n, *root_) == &t || - search_existing (*n, *root_, d) == &t); + search_existing (*n, *root_) == &t; if (r) break; @@ -198,8 +197,7 @@ namespace build2 t.name == n->value && tt.name == n->type && d == n->dir && - (search_existing (*n, *root_) == &t || - search_existing (*n, *root_, d) == &t); + search_existing (*n, *root_) == &t; if (!r) continue; // Not our target. diff --git a/libbuild2/test/rule.cxx b/libbuild2/test/rule.cxx index 81bf50a..28eb35b 100644 --- a/libbuild2/test/rule.cxx +++ b/libbuild2/test/rule.cxx @@ -1161,7 +1161,7 @@ namespace build2 fail << "invalid test executable override: '" << *n << "'"; else { - // Must be a target name. + // Must be a target name. Could be from src (e.g., a script). // // @@ OUT: what if this is a @-qualified pair of names? // diff --git a/libbuild2/test/script/script.cxx b/libbuild2/test/script/script.cxx index 05dc7b0..f7827f6 100644 --- a/libbuild2/test/script/script.cxx +++ b/libbuild2/test/script/script.cxx @@ -268,7 +268,7 @@ namespace build2 v = path (n->dir); else { - // Must be a target name. + // Must be a target name. Could be from src (e.g., a script). // // @@ OUT: what if this is a @-qualified pair of names? // -- cgit v1.1