Clean up default target extension derivation logic

5 files changed, 67 insertions, 13 deletions

diff --git a/build2/bin/target.cxx b/build2/bin/target.cxx
index 9d96ca4..136de9c 100644
--- a/build2/bin/target.cxx
+++ b/build2/bin/target.cxx
@@ -27,7 +27,7 @@ namespace build2
       "obja",
       &file::static_type,
       &obja_factory,
-      nullptr,
+      &target_extension_assert,
       &search_target, // Note: not _file(); don't look for an existing file.
       false
     };
@@ -49,7 +49,7 @@ namespace build2
       "objso",
       &file::static_type,
       &objso_factory,
-      nullptr,
+      &target_extension_assert,
       &search_target, // Note: not _file(); don't look for an existing file.
       false
     };
@@ -80,12 +80,24 @@ namespace build2
       false
     };
 
+    // @@ What extension should we be using when searching for an existing
+    //    exe{}? Say we have a dependency on some pre-existing tool, maybe
+    //    some source code generator. Should we use 'build' extension? But
+    //    what if we find such an executable for something that we need to
+    //    build for 'host'?
+    //
+    //    What if we use extension variables and scoping. We could set the
+    //    root scope exe{*} extension to 'build' and then, say, cxx module
+    //    (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<exe>,
-      nullptr,
+      &target_extension_fix<exe_ext>,
       &search_file,
       false
     };
diff --git a/build2/target b/build2/target
index 3d794c8..c6cc46e 100644
--- a/build2/target
+++ b/build2/target
@@ -1068,6 +1068,16 @@ namespace build2
   const std::string&
   target_extension_var (const target_key&, scope&);
 
+  // Issue diagnostics and fail if called.
+  //
+  const std::string&
+  target_extension_fail (const target_key&, scope&);
+
+  // Assert if called.
+  //
+  const std::string&
+  target_extension_assert (const target_key&, scope&);
+
   // The default behavior, that is, look for an existing target in the
   // prerequisite's directory scope.
   //
diff --git a/build2/target-type b/build2/target-type
index 5647156..60a0e35 100644
--- a/build2/target-type
+++ b/build2/target-type
@@ -20,10 +20,15 @@ namespace build2
 
   // Target type.
   //
-  // Note that we assume there is always a single instance of this class
-  // for any target type. As a result, we can use address comparison to
-  // determine if two target types are the same.
+  // Note that we assume there is always a single instance of this class for
+  // any target type. As a result, we can use address comparison to determine
+  // if two target types are the same.
   //
+  // 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.
   //
   struct target_type
   {
diff --git a/build2/target.cxx b/build2/target.cxx
index 43c6a41..89e16b5 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -308,7 +308,7 @@ namespace build2
       if (de != nullptr)
         ext = &extension_pool.find (de);
       //
-      // Otherwis see if the target type has function that will
+      // Otherwise see if the target type has function that will
       // give us the default extension.
       //
       else if (auto f = type ().extension)
@@ -393,6 +393,36 @@ namespace build2
     return t;
   }
 
+  const string&
+  target_extension_fail (const target_key& tk, scope& s)
+  {
+    {
+      diag_record dr;
+      dr << error << "no default extension to derive file name for ";
+
+      // This is a bit hacky: we may be dealing with a target (see
+      // file::derive_path()) or prerequisite (see search_existing_file()). So
+      // we are going to check if dir is absolute. If it is, then we assume
+      // this is a target, otherwise -- prerequisite.
+      //
+      if (tk.dir->absolute ())
+        dr << "target " << tk;
+      else
+        dr << "prerequisite " << prerequisite_key {nullptr, tk, &s};
+    }
+
+    throw failed ();
+  }
+
+  // Assert if called.
+  //
+  const string&
+  target_extension_assert (const target_key&, scope&)
+  {
+    assert (false); // Attempt to obtain the default extension.
+    throw failed ();
+  }
+
   // type info
   //
 
@@ -525,7 +555,7 @@ namespace build2
     "man",
     &doc::static_type,
     &man_factory,
-    nullptr,        // Should be specified explicitly.
+    &target_extension_assert, // Should be specified explicitly (see factory).
     &search_file,
     false
   };
diff --git a/build2/target.txx b/build2/target.txx
index 6fef367..fbaa8ef 100644
--- a/build2/target.txx
+++ b/build2/target.txx
@@ -34,16 +34,13 @@ namespace build2
     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 ";
 
-      // This is a bit hacky: we may be dealing with a target (see
-      // file::derive_path()) or prerequsite (see search_existing_file()).
-      // So we are going to check if dir is absolute. If it is, then
-      // we assume this is a target, otherwise -- prerequsite.
-      //
       if (tk.dir->absolute ())
         dr << "target " << tk;
       else