Resolve function overload via the argument reversal to untyped

8 files changed, 99 insertions, 227 deletions

diff --git a/build2/function.cxx b/build2/function.cxx
index 4367185..0b432d3 100644
--- a/build2/function.cxx
+++ b/build2/function.cxx
@@ -118,22 +118,21 @@ namespace build2
 
     // Overload resolution.
     //
-    // Ours is pretty simple: if all the arguments match exactly, then we have
-    // a perfect match. Otherwise, if any argument matches via the derived-to-
-    // base conversion, then we have an imperfect match. More than one perfect
-    // or imperfect match is ambiguous (i.e., we don't try to rank imperfect
-    // matches).
+    // Ours is pretty simple: we sort all the overloads into three ranks:
     //
-    size_t count (args.size ());
-    auto ip (map_.equal_range (name));
-
-    // First look for a perfect match, then for imperfect. We do it this way
-    // to make sure we always stay small in the successful case.
+    // 0 -- all the arguments match exactly (perfect match)
+    // 1 -- one or more arguments match via the derived-to-base conversion
+    // 2 -- one or more arguments match via the reversal to untyped
     //
-    small_vector<const function_overload*, 1> r;
+    // More than one match of the same rank is ambiguous.
+    //
+    auto ip (map_.equal_range (name));
 
-    for (bool perf (true);; perf = false)
+    size_t rank (~0);
+    small_vector<const function_overload*, 2> ovls;
     {
+      size_t count (args.size ());
+
       for (auto it (ip.first); it != ip.second; ++it)
       {
         const function_overload& f (it->second);
@@ -145,6 +144,7 @@ namespace build2
 
         // Argument types match.
         //
+        size_t r (0);
         {
           size_t i (0), n (min (count, f.arg_types.size ()));
           for (; i != n; ++i)
@@ -158,29 +158,50 @@ namespace build2
             if (at == ft) // Types match perfectly.
               continue;
 
-            if (!perf && at != nullptr && ft != nullptr)
+            if (at != nullptr && ft != nullptr)
             {
               while ((at = at->base_type) != nullptr && at != ft) ;
 
               if (at != nullptr) // Types match via derived-to-base.
+              {
+                if (r < 1)
+                  r = 1;
                 continue;
+              }
             }
 
-            break;
+            if (ft == nullptr) // Types match via reversal to untyped.
+            {
+              if (r < 2)
+                r = 2;
+              continue;
+            }
+
+            break; // No match.
           }
 
           if (i != n)
-            continue;
+            continue; // No match.
         }
 
-        r.push_back (&f); // Continue looking to detect ambiguities.
-      }
+        // Better or just as good a match?
+        //
+        if (r <= rank)
+        {
+          if (r < rank) // Better.
+          {
+            rank = r;
+            ovls.clear ();
+          }
+
+          ovls.push_back (&f);
+        }
 
-      if (!r.empty () || !perf)
-        break;
+        // Continue looking to detect ambiguities.
+      }
     }
 
-    switch (r.size ())
+    switch (ovls.size ())
     {
     case 1:
       {
@@ -197,7 +218,26 @@ namespace build2
               }
             }));
 
-        auto f (r.back ());
+        auto f (ovls.back ());
+
+        // If one or more arguments match via the reversal to untyped (rank 2),
+        // then we need to go over the overload's arguments one more time an
+        // untypify() those that we need to reverse.
+        //
+        if (rank == 2)
+        {
+          size_t n (args.size ());
+          assert (n <= f->arg_types.size ());
+
+          for (size_t i (0); i != n; ++i)
+          {
+            if (f->arg_types[i]             &&
+                *f->arg_types[i] == nullptr &&
+                args[i].type != nullptr)
+              untypify (args[i]);
+          }
+        }
+
         return make_pair (f->impl (base, move (args), *f), true);
       }
     case 0:
@@ -246,7 +286,7 @@ namespace build2
         diag_record dr;
         dr << fail (loc) << "ambiguous call to "; print_call (dr.os);
 
-        for (auto f: r)
+        for (auto f: ovls)
           dr << info << "candidate: " << *f;
 
         dr << endf;
diff --git a/build2/function.hxx b/build2/function.hxx
index 62666cc..139f1fa 100644
--- a/build2/function.hxx
+++ b/build2/function.hxx
@@ -45,9 +45,6 @@ namespace build2
   // expected to issue diagnostics and throw failed. Note that the arguments
   // are conceptually "moved" and can be reused by the implementation.
   //
-  // @@ Maybe it makes sense to implicitly convert types like string to names
-  //    -- providing all the overload combinations really gets tedious.
-  //
   // A function can also optionally receive the current scope by having the
   // first argument of the const scope* type. It may be NULL is the function
   // is called out of any scope (e.g., command line).
@@ -72,6 +69,12 @@ namespace build2
   // For more examples/ideas, study the existing function families (reside
   // in the functions-*.cxx files).
   //
+  // Note that normally there will be a function overload that has all the
+  // parameters untyped with an implementation that falls back to one of the
+  // overloads that have all the parameters typed, possibly inferring the type
+  // from the argument value "syntax" (e.g., presence of a trailing slash for
+  // a directory path).
+  //
   struct function_overload;
 
   using function_impl = value (const scope*,
diff --git a/build2/functions-filesystem.cxx b/build2/functions-filesystem.cxx
index e7578b2..5236824 100644
--- a/build2/functions-filesystem.cxx
+++ b/build2/functions-filesystem.cxx
@@ -187,64 +187,17 @@ namespace build2
       return path_match (pattern, name);
     };
 
-    f["path_match"] = [](string pattern, names name)
-    {
-      return path_match (pattern, convert<string> (move (name)));
-    };
-
-    f["path_match"] = [](names pattern, string name)
-    {
-      return path_match (convert<string> (move (pattern)), name);
-    };
-
     // Path matching.
     //
-    //                   path      path      *
-    //
     f["path_match"] = [](path pat, path ent, optional<dir_path> start)
     {
       return path_match (pat, ent, start);
     };
 
-    f["path_match"] = [](path pat, path ent, names start)
-    {
-      return path_match (pat, ent, convert<dir_path> (move (start)));
-    };
-
-    //                   path      untyped    *
-    //
-    f["path_match"] = [](path pat, names ent, optional<dir_path> start)
-    {
-      return path_match (pat, convert<path> (move (ent)), start);
-    };
-
-    f["path_match"] = [](path pat, names ent, names start)
-    {
-      return path_match (pat,
-                         convert<path> (move (ent)),
-                         convert<dir_path> (move (start)));
-    };
-
-    //                   untyped    path      *
-    //
-    f["path_match"] = [](names pat, path ent, optional<dir_path> start)
-    {
-      return path_match (convert<path> (move (pat)), ent, start);
-    };
-
-    f["path_match"] = [](names pat, path ent, names start)
-    {
-      return path_match (convert<path> (move (pat)),
-                         ent,
-                         convert<dir_path> (move (start)));
-    };
-
     // The semantics depends on the presence of the start directory or the
     // first two argument syntactic representation.
     //
-    //                   untyped    untyped    *
-    //
-    f["path_match"] = [](names pat, names ent, optional<dir_path> start)
+    f["path_match"] = [](names pat, names ent, optional<names> start)
     {
       auto path_arg = [] (const names& a) -> bool
       {
@@ -257,18 +210,11 @@ namespace build2
       return start || path_arg (pat) || path_arg (ent)
         ? path_match (convert<path> (move (pat)),   // Match as paths.
                       convert<path> (move (ent)),
-                      start)
-        : path_match (convert<string> (move (pat)), // Match as names.
+                      start
+                      ? convert<dir_path> (move (*start))
+                      : optional<dir_path> ())
+        : path_match (convert<string> (move (pat)), // Match as strings.
                       convert<string> (move (ent)));
     };
-
-    f["path_match"] = [](names pat, names ent, names start)
-    {
-      // Match as paths.
-      //
-      return path_match (convert<path> (move (pat)),
-                         convert<path> (move (ent)),
-                         convert<dir_path> (move (start)));
-    };
   }
 }
diff --git a/build2/functions-regex.cxx b/build2/functions-regex.cxx
index 9c428fd..8c1e4c5 100644
--- a/build2/functions-regex.cxx
+++ b/build2/functions-regex.cxx
@@ -423,22 +423,6 @@ namespace build2
       return replace (move (s), re, fmt, move (flags));
     };
 
-    f[".replace"] = [](value s, string re, names fmt, optional<names> flags)
-    {
-      return replace (move (s),
-                      re,
-                      convert<string> (move (fmt)),
-                      move (flags));
-    };
-
-    f[".replace"] = [](value s, names re, string fmt, optional<names> flags)
-    {
-      return replace (move (s),
-                      convert<string> (move (re)),
-                      fmt,
-                      move (flags));
-    };
-
     f[".replace"] = [](value s, names re, names fmt, optional<names> flags)
     {
       return replace (move (s),
@@ -467,22 +451,6 @@ namespace build2
       return split (move (s), re, fmt, move (flags));
     };
 
-    f[".split"] = [](value s, string re, names fmt, optional<names> flags)
-    {
-      return split (move (s),
-                    re,
-                    convert<string> (move (fmt)),
-                    move (flags));
-    };
-
-    f[".split"] = [](value s, names re, string fmt, optional<names> flags)
-    {
-      return split (move (s),
-                    convert<string> (move (re)),
-                    fmt,
-                    move (flags));
-    };
-
     f[".split"] = [](value s, names re, names fmt, optional<names> flags)
     {
       return split (move (s),
@@ -523,93 +491,17 @@ namespace build2
     };
 
     f[".merge"] = [](names s,
-                     string re,
-                     names fmt,
-                     optional<string> delim,
-                     optional<names> flags)
-    {
-      return merge (move (s),
-                    re,
-                    convert<string> (move (fmt)),
-                    move (delim),
-                    move (flags));
-    };
-
-    f[".merge"] = [](names s,
-                     names re,
-                     string fmt,
-                     optional<string> delim,
-                     optional<names> flags)
-    {
-      return merge (move (s),
-                    convert<string> (move (re)),
-                    fmt,
-                    move (delim),
-                    move (flags));
-    };
-
-    f[".merge"] = [](names s,
                      names re,
                      names fmt,
-                     optional<string> delim,
+                     optional<names> delim,
                      optional<names> flags)
     {
       return merge (move (s),
                     convert<string> (move (re)),
                     convert<string> (move (fmt)),
-                    move (delim),
-                    move (flags));
-    };
-
-    f[".merge"] = [](names s,
-                     string re,
-                     string fmt,
-                     names delim,
-                     optional<names> flags)
-    {
-      return merge (move (s),
-                    re,
-                    fmt,
-                    convert<string> (move (delim)),
-                    move (flags));
-    };
-
-    f[".merge"] = [](names s,
-                     string re,
-                     names fmt,
-                     names delim,
-                     optional<names> flags)
-    {
-      return merge (move (s),
-                    re,
-                    convert<string> (move (fmt)),
-                    convert<string> (move (delim)),
-                    move (flags));
-    };
-
-    f[".merge"] = [](names s,
-                     names re,
-                     string fmt,
-                     names delim,
-                     optional<names> flags)
-    {
-      return merge (move (s),
-                    convert<string> (move (re)),
-                    fmt,
-                    convert<string> (move (delim)),
-                    move (flags));
-    };
-
-    f[".merge"] = [](names s,
-                     names re,
-                     names fmt,
-                     names delim,
-                     optional<names> flags)
-    {
-      return merge (move (s),
-                    convert<string> (move (re)),
-                    convert<string> (move (fmt)),
-                    convert<string> (move (delim)),
+                    delim
+                    ? convert<string> (move (*delim))
+                    : optional<string> (),
                     move (flags));
     };
 
@@ -639,22 +531,6 @@ namespace build2
       return apply (move (s), re, fmt, move (flags));
     };
 
-    f[".apply"] = [](names s, string re, names fmt, optional<names> flags)
-    {
-      return apply (move (s),
-                    re,
-                    convert<string> (move (fmt)),
-                    move (flags));
-    };
-
-    f[".apply"] = [](names s, names re, string fmt, optional<names> flags)
-    {
-      return apply (move (s),
-                    convert<string> (move (re)),
-                    fmt,
-                    move (flags));
-    };
-
     f[".apply"] = [](names s, names re, names fmt, optional<names> flags)
     {
       return apply (move (s),
diff --git a/tests/function/builtin/testscript b/tests/function/builtin/testscript
index 6108455..d15eeb3 100644
--- a/tests/function/builtin/testscript
+++ b/tests/function/builtin/testscript
@@ -86,16 +86,7 @@
 
     : path
     :
-    $* <'print $getenv([path] a)' 2>>~/EOE/ != 0
-    <stdin>:1:8: error: unmatched call to getenv(path)
-    /((
-      info: candidate: getenv(<untyped>), qualified name builtin.getenv
-      info: candidate: getenv(string), qualified name builtin.getenv
-    /)|(
-      info: candidate: getenv(string), qualified name builtin.getenv
-      info: candidate: getenv(<untyped>), qualified name builtin.getenv
-    /))
-    EOE
+    $* <'print $getenv([path] a)' >'[null]'
 
     : none
     :
diff --git a/tests/function/filesystem/testscript b/tests/function/filesystem/testscript
index 61fca19..7dcfde9 100644
--- a/tests/function/filesystem/testscript
+++ b/tests/function/filesystem/testscript
@@ -95,10 +95,7 @@
 
     : string-path
     :
-    $* <'print $path_match([string] "b*", [path] "b")' 2>>~/EOE/ != 0
-    <stdin>:1:8: error: unmatched call to path_match(string, path)
-    /.{11}
-    EOE
+    $* <'print $path_match([string] "b*", [path] "b")' >'true'
   }
 
   : path
diff --git a/unit-tests/function/call.test b/unit-tests/function/call.test
index 127af24..3dd9a85 100644
--- a/unit-tests/function/call.test
+++ b/unit-tests/function/call.test
@@ -74,6 +74,23 @@ buildfile:1:2: error: ambiguous call to ambig(<untyped>)
 /))
 EOE
 
+: unmatched
+:
+$* <'$ambig(abc, def)' 2>>~/EOE/ != 0
+buildfile:1:2: error: unmatched call to ambig(<untyped>, <untyped>)
+/((
+  info: candidate: ambig(<untyped> [, uint64]), qualified name dummy.ambig
+  info: candidate: ambig(<untyped> [, string]), qualified name dummy.ambig
+/)|(
+  info: candidate: ambig(<untyped> [, string]), qualified name dummy.ambig
+  info: candidate: ambig(<untyped> [, uint64]), qualified name dummy.ambig
+/))
+EOE
+
+: reverse
+:
+$* <'print $reverse([string] abc)' >'abc'
+
 : optional-absent
 :
 $* <'print $optional()' >'true'
@@ -118,7 +135,7 @@ EOE
 : print-fovl
 :
 $* <'$ambig([bool] true)' 2>>~/EOE/ != 0
-buildfile:1:2: error: unmatched call to ambig(bool)
+buildfile:1:2: error: ambiguous call to ambig(bool)
 /((
   info: candidate: ambig(<untyped> [, uint64]), qualified name dummy.ambig
   info: candidate: ambig(<untyped> [, string]), qualified name dummy.ambig
diff --git a/unit-tests/function/driver.cxx b/unit-tests/function/driver.cxx
index e97596a..393b456 100644
--- a/unit-tests/function/driver.cxx
+++ b/unit-tests/function/driver.cxx
@@ -54,6 +54,8 @@ namespace build2
     f["ambig"] = [](names a, optional<string>)   {return a;};
     f["ambig"] = [](names a, optional<uint64_t>) {return a;};
 
+    f["reverse"] = [](names a) {return a;};
+
     f["scoped"]      = [](const scope*, names a) {return a;};
     f["scoped_void"] = [](const scope*, names) {};
     f["scoped"]      = &scoped;