Prefer default extension supplied by rule over one supplied by target type

3 files changed, 25 insertions, 20 deletions

diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index 4f11cbd..12bf2fa 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -2010,7 +2010,7 @@ namespace build2
 
                 // GCC until version 8 was not capable of writing the
                 // dependency info to stdout. We also need to sense the
-                // diagnostics on the -E runs (which we do by redirective
+                // diagnostics on the -E runs (which we do by redirecting
                 // stderr to stdout).
                 //
                 if (cid == compiler_id::gcc)
diff --git a/build2/target-type.hxx b/build2/target-type.hxx
index 5886fdc..ebe60db 100644
--- a/build2/target-type.hxx
+++ b/build2/target-type.hxx
@@ -34,10 +34,12 @@ namespace build2
   // returns NULL, then it means the default extension for this target could
   // not be derived.
   //
-  // The extension is used in two places: search_existing_file() (called for a
-  // prerequisite with the last argument true) and in target::derive_path()
-  // (called for a target with the last argument false); see their
-  // implementations for details.
+  // The default extension is used in two (key; there are others) places:
+  // search_existing_file() (called for a prerequisite with the last argument
+  // true) and in target::derive_extension() (called for a target with the
+  // last argument false); see their respective implementations for details.
+  // Note that the default extension supplied to derive_extension() (e.g., by
+  // a rule) takes precedence over the one returned by default_extension.
   //
   // If the pattern function is not NULL, then it is used to amend a pattern
   // or match (reverse is false) and then, if the amendment call returned
diff --git a/build2/target.cxx b/build2/target.cxx
index 9c11156..200f562 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -471,6 +471,10 @@ namespace build2
   {
     // See also search_existing_file() if updating anything here.
 
+    // Should be no default extension if searching.
+    //
+    assert (!search || de == nullptr);
+
     // The target should use extensions and they should not be fixed.
     //
     assert (de == nullptr || type ().default_extension != nullptr);
@@ -484,24 +488,24 @@ namespace build2
     {
       optional<string> e;
 
-      // If the target type has the default 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.
+      // Prefer the default extension specified (presumably) by the rule over
+      // the one returned by the default extension function. Here we assume
+      // the rule knows what it is doing (see the exe{} target type for a use
+      // case).
       //
-      if (auto f = type ().default_extension)
-        e = f (key (), base_scope (), search);
-
-      if (!e)
+      if (de != nullptr)
+        e = de;
+      else
       {
-        if (de != nullptr)
-          e = de;
-        else
+        if (auto f = type ().default_extension)
+          e = f (key (), base_scope (), search);
+
+        if (!e)
         {
           if (search)
             return nullptr;
 
-          fail << "no default extension for target " << *this;
+          fail << "no default extension for target " << *this << endf;
         }
       }
 
@@ -850,14 +854,13 @@ namespace build2
     // then we expect the rule to supply the target machine extension. But if
     // it doesn't, then assume no extension (e.g., a script).
     //
-    return string (search
-                   ?
+    return string (!search ? "" :
 #ifdef _WIN32
                    "exe"
 #else
                    ""
 #endif
-                   : "");
+    );
   }
 
 #ifdef _WIN32