From a5949f9e974171144dba84771a9f305f9e1dcc3f Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 27 Jan 2016 13:59:51 +0200 Subject: Rework default extension derivation, again --- build2/bin/target.cxx | 15 +++++++-------- build2/cxx/link.cxx | 25 +++++++++++-------------- build2/search.cxx | 20 +++++++++----------- build2/target | 38 ++++++++++++++++++++++---------------- build2/target-type | 11 ++++++++--- build2/target.cxx | 46 ++++++++++++++++++++++++++++------------------ build2/target.txx | 29 +++++------------------------ 7 files changed, 90 insertions(+), 94 deletions(-) diff --git a/build2/bin/target.cxx b/build2/bin/target.cxx index de51fc7..ed36f80 100644 --- a/build2/bin/target.cxx +++ b/build2/bin/target.cxx @@ -10,6 +10,8 @@ namespace build2 { namespace bin { + constexpr const char ext_var[] = "extension"; + static target* obja_factory (const target_type&, dir_path d, string n, const string* e) { @@ -27,7 +29,7 @@ namespace build2 "obja", &file::static_type, &obja_factory, - &target_extension_assert, + &target_extension_var, nullptr, &search_target, // Note: not _file(); don't look for an existing file. false @@ -50,7 +52,7 @@ namespace build2 "objso", &file::static_type, &objso_factory, - &target_extension_assert, + &target_extension_var, nullptr, &search_target, // Note: not _file(); don't look for an existing file. false @@ -94,13 +96,12 @@ namespace build2 // (or any module that knows how to build exe{}) changes it to the // 'host'. Maybe that's not a bad idea? // - constexpr const char exe_ext[] = ""; const target_type exe::static_type { "exe", &file::static_type, &target_factory, - &target_extension_fix, + &target_extension_var, nullptr, &search_file, false @@ -132,13 +133,12 @@ namespace build2 // come from a variable that is set not by bin but by the module // whose rule matched the target (e.g., cxx::link). // - constexpr const char a_ext[] = "a"; const target_type liba::static_type { "liba", &file::static_type, &liba_factory, - &target_extension_fix, + &target_extension_var, nullptr, &search_file, false @@ -158,13 +158,12 @@ namespace build2 return so; } - constexpr const char so_ext[] = "so"; const target_type libso::static_type { "libso", &file::static_type, &libso_factory, - &target_extension_fix, + &target_extension_var, nullptr, &search_file, false diff --git a/build2/cxx/link.cxx b/build2/cxx/link.cxx index 7d17553..2f9161c 100644 --- a/build2/cxx/link.cxx +++ b/build2/cxx/link.cxx @@ -238,15 +238,13 @@ namespace build2 if (l || p.is_a ()) { - an = path ("lib" + p.name); - - // Note that p.scope should be the same as the target's for - // which we are looking for this library. The idea here is - // that we have to use the same "extension configuration" as - // the target's. + // We are trying to find a library in the search paths extracted from + // the compiler. It would only be natural if we use the library + // prefix/extension that correspond to this compiler's target. // + an = path ("lib" + p.name); ae = ext == nullptr - ? &liba::static_type.extension (p.key ().tk, p.scope) + ? &extension_pool.find ("a") : ext; if (!ae->empty ()) @@ -265,7 +263,7 @@ namespace build2 { sn = path ("lib" + p.name); se = ext == nullptr - ? &libso::static_type.extension (p.key ().tk, p.scope) + ? &extension_pool.find ("so") : ext; if (!se->empty ()) @@ -487,22 +485,21 @@ namespace build2 // if (t.path ().empty ()) { - auto l (t["extension"]); - const char* e (l ? as (*l).c_str () : nullptr); - switch (lt) { case type::e: { - t.derive_path (e != nullptr ? e : ""); + t.derive_path (""); break; } case type::a: case type::so: { auto l (t["bin.libprefix"]); - t.derive_path (e != nullptr ? e : (lt == type::a ? "a" : "so"), - l ? as (*l).c_str () : "lib"); + const char* e (lt == type::a ? "a" : "so"); + const char* p (l ? as (*l).c_str () : "lib"); + + t.derive_path (e, p); break; } } diff --git a/build2/search.cxx b/build2/search.cxx index 2ab3d1f..3df8877 100644 --- a/build2/search.cxx +++ b/build2/search.cxx @@ -71,20 +71,18 @@ namespace build2 { if (auto f = ctk.type->extension) { - ext = &f (ctk, *cpk.scope); // Already from the pool. + ext = f (ctk, *cpk.scope); // Already from the pool. } - else + + if (ext == nullptr) { // What should we do here, fail or say we didn't find anything? - // Current think is that if the target type didn't provide the - // default extension, then it doesn't want us to search for an - // existing file (of course, if the user specified the extension - // explicitly, we will still do so). But let me know what you - // think. + // Current think is that if the target type couldn't find the default + // extension, then we simply shouldn't search for any existing files + // (of course, if the user specified the extension explicitly, we will + // still do so). // - //fail << "no default extension for prerequisite " << cpk; - level4 ([&]{trace << "no existing file found for prerequisite " - << cpk;}); + level4 ([&]{trace << "no existing file for prerequisite " << cpk;}); return nullptr; } } @@ -134,7 +132,7 @@ namespace build2 return &t; } - level4 ([&]{trace << "no existing file found for prerequisite " << cpk;}); + level4 ([&]{trace << "no existing file for prerequisite " << cpk;}); return nullptr; } diff --git a/build2/target b/build2/target index e6ebc42..b35efdf 100644 --- a/build2/target +++ b/build2/target @@ -879,18 +879,19 @@ namespace build2 void path (path_type p) {assert (path_.empty ()); path_ = std::move (p);} - // Derive a path from target's dir, name, and, if specified, ext. - // If ext is not specified, then use default_ext and also update - // the target's extension (this becomes important if later we need - // to reliably determine whether this file has an extension; think - // hxx{foo.bar.} and hxx.ext is empty). + // Derive a path from target's dir, name, and, if set, ext. If ext is not + // set, try to derive it using the target type extension function and + // fallback to default_ext, if specified. In both cases also update the + // target's extension (this becomes important if later we need to reliably + // determine whether this file has an extension; think hxx{foo.bar.} and + // hxx{*}:extension is empty). // - // If name_prefix is not NULL, add it before the name part and after - // the directory. Similarly, if name_suffix is not NULL, add it after - // the name part and before the extension. + // If name_prefix is not NULL, add it before the name part and after the + // directory. Similarly, if name_suffix is not NULL, add it after the name + // part and before the extension. // - // Finally, if the path was already assigned to this target, then - // this function verifies that the two are the same. + // Finally, if the path was already assigned to this target, then this + // function verifies that the two are the same. // void derive_path (const char* default_ext = nullptr, @@ -1058,24 +1059,29 @@ namespace build2 // Return fixed target extension. // template - const std::string& + const std::string* target_extension_fix (const target_key&, scope&); - // Get the extension from the variable or use the default if none set. - // Issue diagnostics and fail if the default is NULL. + // Get the extension from the variable or use the default if none set. If + // the default is NULL, then return NULL. // template - const std::string& + const std::string* target_extension_var (const target_key&, scope&); + // Always return NULL extension. + // + const std::string* + target_extension_null (const target_key&, scope&); + // Issue diagnostics and fail if called. // - const std::string& + const std::string* target_extension_fail (const target_key&, scope&); // Assert if called. // - const std::string& + const std::string* target_extension_assert (const target_key&, scope&); // Target print functions. diff --git a/build2/target-type b/build2/target-type index a831670..2659e3d 100644 --- a/build2/target-type +++ b/build2/target-type @@ -28,15 +28,20 @@ namespace build2 // If the extension derivation function is NULL, then it means this target // type does not use extensions. Note that this is relied upon when deciding // whether to print the extension; if the target does use extensions but the - // defaults should not or cannot (logically) be obtained, then use - // target_extension_{fail,assert}(), respectively. + // defaults could not (and its ok), could not (and its not ok), or should not + // (logically) be obtained, then use target_extension_{null,fail,assert}(), + // respectively. If the extension function returns NULL, then that means the + // default extension for this target could not be derived. + // + // The extension function is used in two places: search_existing_file() and + // in target::derive_path(); see their implementations for details. // struct target_type { const char* name; const target_type* base; target* (*factory) (const target_type&, dir_path, string, const string*); - const string& (*extension) (const target_key&, scope&); + const string* (*extension) (const target_key&, scope&); void (*print) (ostream&, const target_key&); target* (*search) (const prerequisite_key&); bool see_through; // A group with the default "see through" semantics. diff --git a/build2/target.cxx b/build2/target.cxx index 3fd099d..50188ac 100644 --- a/build2/target.cxx +++ b/build2/target.cxx @@ -322,24 +322,28 @@ namespace build2 if (ns != nullptr) n += ns; - // Update the extension. - // - // See also search_existing_file() if updating anything here. + // Update the extension. See also search_existing_file() if updating + // anything here. // + assert (de == nullptr || type ().extension != nullptr); + if (ext == nullptr) { - // If provided by the caller, then use that. - // - if (de != nullptr) - ext = &extension_pool.find (de); + // If the target type has the extension function then try that first. + // The reason for preferring it over what's been provided by the caller + // is that this function will often use the 'extension' variable which + // the user can use to override extensions. // - // Otherwise see if the target type has function that will - // give us the default extension. - // - else if (auto f = type ().extension) - ext = &f (key (), base_scope ()); // Already from the pool. - else - fail << "no default extension for target " << *this; + if (auto f = type ().extension) + ext = f (key (), base_scope ()); // Already from the pool. + + if (ext == nullptr) + { + if (de != nullptr) + ext = &extension_pool.find (de); + else + fail << "no default extension for target " << *this; + } } // Add the extension. @@ -418,7 +422,13 @@ namespace build2 return t; } - const string& + const string* + target_extension_null (const target_key&, scope&) + { + return nullptr; + } + + const string* target_extension_fail (const target_key& tk, scope& s) { { @@ -439,7 +449,7 @@ namespace build2 throw failed (); } - const string& + const string* target_extension_assert (const target_key&, scope&) { assert (false); // Attempt to obtain the default extension. @@ -556,13 +566,13 @@ namespace build2 false }; - static const std::string& + static const std::string* buildfile_target_extension (const target_key& tk, scope&) { // If the name is special 'buildfile', then there is no extension, // otherwise it is .build. // - return extension_pool.find (*tk.name == "buildfile" ? "" : "build"); + return &extension_pool.find (*tk.name == "buildfile" ? "" : "build"); } const target_type buildfile::static_type diff --git a/build2/target.txx b/build2/target.txx index fbaa8ef..4999f61 100644 --- a/build2/target.txx +++ b/build2/target.txx @@ -10,14 +10,14 @@ namespace build2 { template - const string& + const string* target_extension_fix (const target_key&, scope&) { - return extension_pool.find (ext); + return &extension_pool.find (ext); } template - const string& + const string* target_extension_var (const target_key& tk, scope& s) { // Include target type/pattern-specific variables. @@ -27,29 +27,10 @@ namespace build2 // Help the user here and strip leading '.' from the extension. // const string& e (as (*l)); - return extension_pool.find ( + return &extension_pool.find ( !e.empty () && e.front () == '.' ? string (e, 1) : e); } - if (def != nullptr) - return extension_pool.find (def); - - // Similar code to target_extension_fail(). - // - { - diag_record dr; - dr << error << "no default extension in variable '" << var << "'" - << info << "required to derive file name for "; - - if (tk.dir->absolute ()) - dr << "target " << tk; - else - dr << "prerequisite " << prerequisite_key {nullptr, tk, &s}; - - dr << info << "perhaps you forgot to add " - << tk.type->name << "{*}: " << var << " = ..."; - } - - throw failed (); + return def != nullptr ? &extension_pool.find (def) : nullptr; } } -- cgit v1.1