From e1f472e471533330db05a42d5bcd4e99b211da0c Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 21 May 2021 15:11:04 +0200 Subject: Clean default target type extension logic --- libbuild2/in/target.cxx | 4 +--- libbuild2/scope.cxx | 38 ++++++++++++++++++++++++++++++++++++++ libbuild2/target-type.hxx | 21 +++++++++++++++++---- libbuild2/target.cxx | 36 +++++++++++++++++++++--------------- libbuild2/target.hxx | 6 ++++++ 5 files changed, 83 insertions(+), 22 deletions(-) (limited to 'libbuild2') diff --git a/libbuild2/in/target.cxx b/libbuild2/in/target.cxx index 0affc04..d9bc8a7 100644 --- a/libbuild2/in/target.cxx +++ b/libbuild2/in/target.cxx @@ -43,14 +43,12 @@ namespace build2 fail (l) << "pattern in in{} prerequisite" << endf; } - extern const char in_ext_def[] = ""; // No extension by default. - const target_type in::static_type { "in", &file::static_type, &target_factory, - &target_extension_fix, + &target_extension_none, nullptr, /* default_extension */ // Taken care of by search. &in_pattern, &target_print_1_ext_verb, // Same as file. diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx index e16e773..3b612f8 100644 --- a/libbuild2/scope.cxx +++ b/libbuild2/scope.cxx @@ -887,6 +887,7 @@ namespace build2 dt->base = &base; dt->factory = &derived_tt_factory; +#if 0 // @@ We should probably inherit the fixed extension unless overriden with // another fixed? But then any derivation from file{} will have to specify // (or override) the fixed extension? But what is the use of deriving from @@ -918,6 +919,43 @@ namespace build2 dt->fixed_extension != nullptr ? &target_print_0_ext_verb // Fixed extension, no use printing. : nullptr; // Normal. +#endif + + // An attempt to clarify the above mess: + // + // 1. If we have a "really fixed" extension (like man1{}) then we keep + // it (including pattern and print functions). + // + // 2. Otherwise, we make it target_extension_var. + // + // Note that this still mis-fires for the following scenarios: + // + // file{} -- What if the user does not set the default extension expecting + // similar semantics as file{} or man{} itself. Maybe explicit + // via attribute (i.e., inherit from base)? + // + // @@ Get the fallback extension from base target_extension_var + // somehow (we know the base target type so could just call it)? + // + if (ext) + { + if (dt->fixed_extension == nullptr || + dt->fixed_extension == &target_extension_none || + dt->fixed_extension == &target_extension_must) + { + dt->fixed_extension = nullptr; + dt->default_extension = &target_extension_var; + dt->pattern = &target_pattern_var; + dt->print = nullptr; + } + } + else + { + dt->fixed_extension = nullptr; + dt->default_extension = nullptr; + dt->pattern = nullptr; + dt->print = nullptr; + } return root_extra->target_types.insert (name, move (dt)); } diff --git a/libbuild2/target-type.hxx b/libbuild2/target-type.hxx index 913432e..5798766 100644 --- a/libbuild2/target-type.hxx +++ b/libbuild2/target-type.hxx @@ -22,10 +22,23 @@ namespace build2 // type does not use extensions. Note that this is relied upon when deciding // whether to print the extension. // - // The fixed extension function should return the fixed extension (which can - // point to the key's ext member; note that for performance reasons we - // currently only verify the explicitly specified extension on target - // insersion -- see target_key comparison for details). + // If the fixed extension function is specified, then it means that this + // target type has a fixed extension (including the no-extension case) and + // this function should return such a fixed extension (which, if overriding + // by the user is allowed, can point to the key's ext member; note that for + // performance reasons we currently only verify the explicitly specified + // extension on target insersion -- see target_key comparison for details). + // It is called eraly, during the target insertion, in contrast to the + // default extension function described below (you would specify one or the + // other). + // + // Note that the fixed no-extension case that allows overriding by the user + // is used to implement the "if extension is not specified by the user then + // there is no extension" semantics of the file{} and similar target types. + // For such cases the target_extension_none() function should be used (we + // compare to it's address to detect target types with such semantics). + // Similarly, for cases where the user must specify the extension explicitly + // (e.g., man{}), use target_extension_must(). // // The root scope argument to the fixed extension function may be NULL which // means the root scope is not known. A target type that relies on this must diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index a5061e3..6ba12b9 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -979,6 +979,23 @@ namespace build2 return search_existing_file (t.ctx, pk); } + extern const char target_extension_none_[] = ""; + + const char* + target_extension_none (const target_key& k, const scope* s) + { + return target_extension_fix (k, s); + } + + const char* + target_extension_must (const target_key& tk, const scope*) + { + if (!tk.ext) + fail << tk.type->name << " target " << tk << " must include extension"; + + return tk.ext->c_str (); + } + void target_print_0_ext_verb (ostream& os, const target_key& k) { @@ -1037,14 +1054,12 @@ namespace build2 false }; - extern const char file_ext_def[] = ""; - const target_type file::static_type { "file", &path_target::static_type, &target_factory, - &target_extension_fix, + &target_extension_none, nullptr, /* default_extension */ nullptr, /* pattern */ &target_print_1_ext_verb, // Print extension even at verbosity level 0. @@ -1467,7 +1482,7 @@ namespace build2 "doc", &file::static_type, &target_factory, - &target_extension_fix, // Same as file (no extension). + &target_extension_none, // Same as file (no extension). nullptr, /* default_extension */ nullptr, /* pattern */ // Same as file. &target_print_1_ext_verb, // Same as file. @@ -1480,7 +1495,7 @@ namespace build2 "legal", &doc::static_type, &target_factory, - &target_extension_fix, // Same as file (no extension). + &target_extension_none, // Same as file (no extension). nullptr, /* default_extension */ nullptr, /* pattern */ // Same as file. &target_print_1_ext_verb, // Same as file. @@ -1488,21 +1503,12 @@ namespace build2 false }; - static const char* - man_extension (const target_key& tk, const scope*) - { - if (!tk.ext) - fail << "man target " << tk << " must include extension (man section)"; - - return tk.ext->c_str (); - } - const target_type man::static_type { "man", &doc::static_type, &target_factory, - &man_extension, // Should be specified explicitly. + &target_extension_must, // Should be specified explicitly. nullptr, /* default_extension */ nullptr, &target_print_1_ext_verb, // Print extension even at verbosity level 0. diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 49d4563..10abf16 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -1940,6 +1940,12 @@ namespace build2 string&, optional&, const location&, bool); + const char* + target_extension_none (const target_key&, const scope*); + + const char* + target_extension_must (const target_key&, const scope*); + // Get the extension from the `extension` variable or use the default if // none set. If the default is NULL, then return NULL. // -- cgit v1.1