From d204ab5aaddda29a998baa5a20ea39e7802e2492 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Sun, 17 Dec 2017 22:08:20 -0500
Subject: [PATCH 1/8] Fix poorly defined behavior when choosing certain
 function overloads. Previously the order of overload declarations would
 affect which was chosen.

---
 CMakeLists.txt                              |   5 +-
 src/liboslcomp/ast.h                        |  11 -
 src/liboslcomp/typecheck.cpp                | 349 +++++++++++++++-----
 testsuite/function-overloads/a_fcn.h        |  11 +
 testsuite/function-overloads/a_ivp.h        |  11 +
 testsuite/function-overloads/b_nci.h        |  12 +
 testsuite/function-overloads/b_vpf.h        |  11 +
 testsuite/function-overloads/c_cnf.h        |  11 +
 testsuite/function-overloads/c_vpi.h        |  11 +
 testsuite/function-overloads/ref/out.txt    |  46 +++
 testsuite/function-overloads/run.py         |  15 +
 testsuite/function-overloads/test.osl       | 120 +++++++
 testsuite/oslc-err-funcoverload/ref/out.txt |   9 +
 testsuite/oslc-err-funcoverload/run.py      |   5 +
 testsuite/oslc-err-funcoverload/test.osl    |  11 +
 15 files changed, 540 insertions(+), 98 deletions(-)
 create mode 100644 testsuite/function-overloads/a_fcn.h
 create mode 100644 testsuite/function-overloads/a_ivp.h
 create mode 100644 testsuite/function-overloads/b_nci.h
 create mode 100644 testsuite/function-overloads/b_vpf.h
 create mode 100644 testsuite/function-overloads/c_cnf.h
 create mode 100644 testsuite/function-overloads/c_vpi.h
 create mode 100644 testsuite/function-overloads/ref/out.txt
 create mode 100755 testsuite/function-overloads/run.py
 create mode 100644 testsuite/function-overloads/test.osl
 create mode 100644 testsuite/oslc-err-funcoverload/ref/out.txt
 create mode 100755 testsuite/oslc-err-funcoverload/run.py
 create mode 100644 testsuite/oslc-err-funcoverload/test.osl

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1af9e14055..0153462147 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -248,6 +248,7 @@ TESTSUITE ( and-or-not-synonyms aastep arithmetic array array-derivs array-range
             error-dupes exit exponential
             fprintf
             function-earlyreturn function-simple function-outputelem
+            function-overloads
             geomath getattribute-camera getattribute-shader
             getsymbol-nonheap gettextureinfo
             group-outputs groupstring
@@ -266,8 +267,8 @@ TESTSUITE ( and-or-not-synonyms aastep arithmetic array array-derivs array-range
             operator-overloading
             oslc-comma oslc-D
             oslc-err-arrayindex oslc-err-closuremul oslc-err-field
-            oslc-err-format oslc-err-funcredef oslc-err-intoverflow
-            oslc-err-noreturn oslc-err-notfunc
+            oslc-err-format oslc-err-funcoverload oslc-err-funcredef
+            oslc-err-intoverflow oslc-err-noreturn oslc-err-notfunc
             oslc-err-outputparamvararray oslc-err-paramdefault
             oslc-err-struct-array-init oslc-err-struct-ctr
             oslc-err-struct-dup
diff --git a/src/liboslcomp/ast.h b/src/liboslcomp/ast.h
index a8c2c028e3..8d567ac18d 100644
--- a/src/liboslcomp/ast.h
+++ b/src/liboslcomp/ast.h
@@ -881,17 +881,6 @@ class ASTfunction_call : public ASTNode
     }
 
 private:
-    /// Typecheck all polymorphic versions, return UNKNOWN if no match was
-    /// found, or a real type if there was a match.  Also, upon matching,
-    /// re-jigger m_sym to point to the specific polymorphic match.
-    /// Allow arguments to be coerced (e.g., substituting a vector where
-    /// a point was expected, or a float where a color was expected) only
-    /// if coerceargs is true.  For return values, allow spatial triples to
-    /// mutually match if 'equivreturn' is true, and allow any coercive
-    /// return type if 'expected' is TypeSpec() (i.e., unknown).
-    TypeSpec typecheck_all_poly (TypeSpec expected, bool coerceargs,
-                                 bool equivreturn);
-
     /// Handle all the special cases for built-ins.  This includes
     /// irregular patterns of which args are read vs written, special
     /// checks for printf- and texture-like, etc.
diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 0beedfd609..130756a954 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -904,30 +904,6 @@ ASTNode::check_arglist (const char *funcname, ASTNode::ref arg,
 
 
 
-TypeSpec
-ASTfunction_call::typecheck_all_poly (TypeSpec expected, bool coerceargs,
-                                      bool equivreturn)
-{
-    for (FunctionSymbol *poly = func();  poly;  poly = poly->nextpoly()) {
-        const char *code = poly->argcodes().c_str();
-        int advance;
-        TypeSpec returntype = m_compiler->type_from_code (code, &advance);
-        code += advance;
-        if (check_arglist (m_name.c_str(), args(), code, coerceargs)) {
-            // Return types also must match if not coercible
-            if (expected == returntype ||
-                (equivreturn && equivalent(expected,returntype)) ||
-                expected == TypeSpec()) {
-                m_sym = poly;
-                return returntype;
-            }
-        }
-    }
-    return TypeSpec();
-}
-
-
-
 void
 ASTfunction_call::mark_optional_output (int firstopt, const char **tags)
 {
@@ -1180,58 +1156,277 @@ ASTfunction_call::typecheck_struct_constructor ()
 }
 
 
+/// <doc CandidateFunctions>
+///
+/// Score a set of polymorphic functions based on arguments & return type.
+///
+/// When choosing which varaint to use precedence is given to arguments and the
+/// return type is used as a possible tie breaker in case of any ambiguity.
+/// The highest score for an argument is given when it is an exact match.
+///
+class CandidateFunctions {
+    enum {
+        kExactMatch     = 100,
+        kIntegralToFP   = 77,
+        kArrayMatch     = 44,
+        kCoercable      = 23,
+        kMatchAnything  = 1,
+        kNoMatch        = 0,
+
+        // Additional named rules
+        kFPToIntegral   = 60,          // = kIntegralToFP to match c++
+    };
+    struct Candidate {
+        FunctionSymbol* sym;
+        TypeSpec rtype;
+        int ascore;
+        int rscore;
+
+        Candidate(FunctionSymbol *s, TypeSpec rt, int as, int rs) :
+            sym(s), rtype(rt), ascore(as), rscore(rs) {}
+
+        string_view name() const { return sym->name(); }
+    };
+    typedef std::vector<Candidate> Candidates;
+
+    OSLCompilerImpl* m_compiler;
+    Candidates m_candidates;
+    std::set<ustring> m_scored;
+    TypeSpec m_rval;
+    ASTNode::ref m_args;
+    size_t m_nargs;
+
+    const char* scoreWildcard(int& argscore, size_t& fargs, const char* args) const {
+        while (fargs < m_nargs) {
+            argscore += kMatchAnything;
+            ++fargs;
+        }
+        return args + 1;
+    }
 
-TypeSpec
-ASTfunction_call::typecheck (TypeSpec expected)
-{
-    typecheck_children ();
+    static int scoreType(TypeSpec expected, TypeSpec actual) {
+        if (expected == actual)
+            return kExactMatch;
 
-    if (is_struct_ctr()) {
-        // Looks like function call, but is actually struct constructor
-        return typecheck_struct_constructor ();
+        if (!actual.is_closure() && actual.is_scalarnum() &&
+            !expected.is_closure() && expected.is_scalarnum())
+            return expected.is_int() ? kFPToIntegral : kIntegralToFP;
+
+        if (expected.is_unsized_array() && actual.is_sized_array() &&
+            expected.elementtype() == actual.elementtype()) {
+            // Allow a fixed-length array match to a formal array with
+            // unspecified length, if the element types are the same.
+            return kArrayMatch;
+        }
+
+        if (assignable (expected, actual))
+            return kCoercable;
+
+        return kNoMatch;
     }
 
-    bool match = false;
+    int addCandidate(FunctionSymbol* func) {
+        // Early out if this declaration has already been scored
+        if (m_scored.count(func->argcodes()))
+            return kNoMatch;
+        m_scored.insert(func->argcodes());
+
+        int advance;
+        const char *formals = func->argcodes().c_str();
+        TypeSpec rtype = m_compiler->type_from_code (formals, &advance);
+        formals += advance;
+
+        int argscore = 0;
+        size_t fargs = 0;
+        for (ASTNode::ref arg = m_args; *formals && arg; ++fargs, arg = arg->next()) {
+            switch (*formals) {
+                case '*':  // Will match anything left
+                    formals = scoreWildcard(argscore, fargs, formals);
+                    ASSERT (*formals == 0);
+                    continue;
+
+                case '.':  // Token/value pairs
+                    if (arg->typespec().is_string() && arg->next()) {
+                        formals = scoreWildcard(argscore, fargs, formals);
+                        ASSERT (*formals == 0);
+                        continue;
+                    }
+                    return kNoMatch;
+
+                case '?':
+                    if (formals[1] == '[' && formals[2] == ']') {
+                        // Any array
+                        formals += 3;
+                        if (!arg->typespec().is_array())
+                            return kNoMatch; // wanted an array, didn't get one
+                        argscore += kMatchAnything;
+                    } else if (!arg->typespec().is_array()) {
+                        formals += 1;       // match anything
+                        argscore += kMatchAnything;
+                    } else
+                        return kNoMatch;    // wanted any scalar, got an array
+                    continue;
+
+                default:
+                    break;
+            }
+            // To many arguments for the function, done without a match.
+            if (fargs >= m_nargs)
+                return kNoMatch;
+
+            TypeSpec formaltype = m_compiler->type_from_code (formals, &advance);
+            formals += advance;
 
-    // Look for an exact match, including expected return type
-    m_typespec = typecheck_all_poly (expected, false, false);
-    if (m_typespec != TypeSpec())
-        match = true;
+            int score = scoreType(formaltype, arg->typespec());
+            if (score == kNoMatch)
+                return kNoMatch;
 
-    // Now look for an exact match for arguments, but equivalent return type
-    m_typespec = typecheck_all_poly (expected, false, true);
-    if (m_typespec != TypeSpec())
-        match = true;
+            argscore += score;
+        }
+
+        // Check any remaining arguments
+        switch (*formals) {
+            case '*':
+            case '.':
+                // Skip over the unused optional args
+                ++formals;
+                ++fargs;
+            case '\0':
+                if (fargs < m_nargs)
+                    return kNoMatch;
+                break;
+
+            default:
+                // TODO: Scoring default function arguments would go here
+                // Curently an unused formal argument, so no match at all.
+                return kNoMatch;
+        }
+        ASSERT (*formals == 0);
+
+        int highscore = m_candidates.empty() ? 0 : m_candidates.front().ascore;
+        if (argscore < highscore)
+            return kNoMatch;
+
+        // clear any prior ambiguous matches
+        if (argscore != highscore)
+            m_candidates.clear();
 
-    // Now look for an exact match on args, but any return type
-    if (! match && expected != TypeSpec()) {
-        m_typespec = typecheck_all_poly (TypeSpec(), false, false);
-        if (m_typespec != TypeSpec())
-            match = true;
+        // append the latest high scoring function
+        m_candidates.emplace_back(func, rtype, argscore,
+                                  scoreType(m_rval, rtype));
+
+        return argscore;
     }
 
-    // Now look for a coercible match of args, exact march on return type
-    if (! match) {
-        m_typespec = typecheck_all_poly (expected, true, false);
-        if (m_typespec != TypeSpec())
-            match = true;
+public:
+    CandidateFunctions(OSLCompilerImpl* compiler, TypeSpec rval, ASTNode::ref args, FunctionSymbol* func) :
+        m_compiler(compiler), m_rval(rval), m_args(args), m_nargs(0) {
+
+        //std::cerr << "Matching " << func->name() << " formals='" << (rval.simpletype().basetype != TypeDesc::UNKNOWN ?  compiler->code_from_type (rval) : " ");
+        for (ASTNode::ref arg = m_args; arg; arg = arg->next()) {
+            //std::cerr << compiler->code_from_type (arg->typespec());
+            ++m_nargs;
+        }
+        //std::cerr << "'\n";
+
+        while (func) {
+            //int score =
+            addCandidate(func);
+            //std::cerr << '\t' << func->name() << " formals='" << func->argcodes().c_str() << "'  " << score << ", " << (score ? m_candidates.back().rscore : 0) << "\n";
+            func = func->nextpoly();
+        }
     }
 
-    // Now look for a coercible match of args, equivalent march on return type
-    if (! match) {
-        m_typespec = typecheck_all_poly (expected, true, true);
-        if (m_typespec != TypeSpec())
-            match = true;
+    void reportError(ASTNode* caller,
+                     std::string name = "", bool showArgs = true,
+                     bool candidateMsg = true,
+                     const char* msg = "No matching function call to") const {
+        if (showArgs) {
+            name += " (";
+            const char *comma = "";
+            for (ASTNode::ref arg = m_args; arg; arg = arg->next()) {
+                name += comma;
+                name += arg->typespec().string();
+                comma = ", ";
+            }
+            name += ")";
+        }
+
+        caller->error ("%s '%s'", msg, name);
+        if (candidateMsg)
+            m_compiler->errhandler().message("  Candidates are:\n");
+    }
+
+    void reportAmbiguity(FunctionSymbol* sym) const {
+        int advance;
+        const char *formals = sym->argcodes().c_str();
+        TypeSpec returntype = m_compiler->type_from_code (formals, &advance);
+        formals += advance;
+
+        auto& errh =  m_compiler->errhandler();
+        errh.message("    ");
+
+        if (ASTNode* decl = sym->node())
+            errh.message("%s:%d\t", decl->sourcefile(), decl->sourceline());
+
+        errh.message("%s %s (%s)\n", m_compiler->type_c_str(returntype),
+                     sym->name(),
+                     m_compiler->typelist_from_code(formals).c_str());
     }
 
-    // All that failed, try for a coercible match on everything
-    if (! match && expected != TypeSpec()) {
-        m_typespec = typecheck_all_poly (TypeSpec(), true, false);
-        if (m_typespec != TypeSpec())
-            match = true;
+    std::pair<FunctionSymbol*, TypeSpec> best(ASTNode* caller, bool strict = 0) {
+        switch (m_candidates.size()) {
+            case 0:  return { nullptr, TypeSpec() };
+            case 1:  return { m_candidates[0].sym, m_candidates[0].rtype };
+            default: break;
+        }
+
+        int ambiguity = 0;
+        std::pair<const Candidate*, int> c = { nullptr, -1 };
+        for (auto& candidate : m_candidates) {
+            // re-score based on matching return value
+            if (candidate.rscore > c.second)
+                c = std::make_pair(&candidate, candidate.rscore);
+            else if (candidate.rscore == c.second)
+                ambiguity = candidate.rscore;
+        }
+
+        if (ambiguity || strict) {
+            ASSERT (caller);
+            reportError(caller, m_candidates[0].name(), false,
+                        !m_candidates.empty(), "Ambiguous call to");
+            for (auto& candidate : m_candidates) {
+                if (candidate.rscore >= ambiguity)
+                    reportAmbiguity(candidate.sym);
+            }
+        }
+
+        ASSERT (c.first);
+        return {c.first->sym, c.first->rtype};
     }
 
-    if (match) {
+    bool empty() const { return m_candidates.empty(); }
+};
+
+
+
+TypeSpec
+ASTfunction_call::typecheck (TypeSpec expected)
+{
+    typecheck_children ();
+
+    if (is_struct_ctr()) {
+        // Looks like function call, but is actually struct constructor
+        return typecheck_struct_constructor ();
+    }
+
+    // Save the currently choosen symbol for error reporting later
+    FunctionSymbol* poly = func();
+
+    CandidateFunctions candidates(m_compiler, expected, args(), poly);
+    std::tie(m_sym, m_typespec) = candidates.best(this);
+
+    if (m_sym != nullptr) {
         if (is_user_function()) {
             if (func()->number_of_returns() == 0 &&
                 ! func()->typespec().is_void()) {
@@ -1245,35 +1440,19 @@ ASTfunction_call::typecheck (TypeSpec expected)
         return m_typespec;
     }
 
+    // Ambiguity has already been reported.
+    if (!candidates.empty())
+        return TypeSpec();
+
     // Couldn't find any way to match any polymorphic version of the
     // function that we know about.  OK, at least try for helpful error
     // message.
-    std::string choices ("");
-    for (FunctionSymbol *poly = func();  poly;  poly = poly->nextpoly()) {
-        const char *code = poly->argcodes().c_str();
-        int advance;
-        TypeSpec returntype = m_compiler->type_from_code (code, &advance);
-        code += advance;
-        if (choices.length())
-            choices += "\n";
-        choices += Strutil::format ("\t%s %s (%s)",
-                              type_c_str(returntype), m_name.c_str(),
-                              m_compiler->typelist_from_code(code).c_str());
-    }
-
-    std::string actualargs;
-    for (ASTNode::ref arg = args();  arg;  arg = arg->next()) {
-        if (actualargs.length())
-            actualargs += ", ";
-        actualargs += arg->typespec().string();
+    candidates.reportError(this, m_name.string(), true, poly != nullptr);
+    while (poly) {
+        candidates.reportAmbiguity(poly);
+        poly = poly->nextpoly();
     }
 
-    if (choices.size())
-        error ("No matching function call to '%s (%s)'\n    Candidates are:\n%s",
-               m_name.c_str(), actualargs.c_str(), choices.c_str());
-    else
-        error ("No matching function call to '%s (%s)'",
-               m_name.c_str(), actualargs.c_str());
     return TypeSpec();
 }
 
diff --git a/testsuite/function-overloads/a_fcn.h b/testsuite/function-overloads/a_fcn.h
new file mode 100644
index 0000000000..3156d9bce3
--- /dev/null
+++ b/testsuite/function-overloads/a_fcn.h
@@ -0,0 +1,11 @@
+
+void testA(float a, float b, float c) {
+    printf("testA float\n");
+}
+void testA(color a, float b, float c) {
+    printf("testA color\n");
+}
+void testA(normal a, float b, float c) {
+    printf("testA normal\n");
+}
+
diff --git a/testsuite/function-overloads/a_ivp.h b/testsuite/function-overloads/a_ivp.h
new file mode 100644
index 0000000000..f572e5ec2a
--- /dev/null
+++ b/testsuite/function-overloads/a_ivp.h
@@ -0,0 +1,11 @@
+
+void testA(int a, float b, float c) {
+    printf("testA int\n");
+}
+void testA(vector a, float b, float c) {
+    printf("testA vector\n");
+}
+void testA(point a, float b, float c) {
+    printf("testA point\n");
+}
+
diff --git a/testsuite/function-overloads/b_nci.h b/testsuite/function-overloads/b_nci.h
new file mode 100644
index 0000000000..27aaf125f1
--- /dev/null
+++ b/testsuite/function-overloads/b_nci.h
@@ -0,0 +1,12 @@
+
+
+void testB(normal a, float b, float c) {
+    printf("testB normal\n");
+}
+void testB(color a, float b, float c) {
+    printf("testB color\n");
+}
+void testB(int a, float b, float c) {
+    printf("testB int\n");
+}
+
diff --git a/testsuite/function-overloads/b_vpf.h b/testsuite/function-overloads/b_vpf.h
new file mode 100644
index 0000000000..1bd8c6a7f0
--- /dev/null
+++ b/testsuite/function-overloads/b_vpf.h
@@ -0,0 +1,11 @@
+
+void testB(vector a, float b, float c) {
+    printf("testB vector\n");
+}
+void testB(point a, float b, float c) {
+    printf("testB point\n");
+}
+void testB(float a, float b, float c) {
+    printf("testB float\n");
+}
+
diff --git a/testsuite/function-overloads/c_cnf.h b/testsuite/function-overloads/c_cnf.h
new file mode 100644
index 0000000000..68a1aaceb4
--- /dev/null
+++ b/testsuite/function-overloads/c_cnf.h
@@ -0,0 +1,11 @@
+
+
+void testC(color a, float b, float c) {
+    printf("testC color\n");
+}
+void testC(normal a, float b, float c) {
+    printf("testC normal\n");
+}
+void testC(float a, float b, float c) {
+    printf("testC float\n");
+}
diff --git a/testsuite/function-overloads/c_vpi.h b/testsuite/function-overloads/c_vpi.h
new file mode 100644
index 0000000000..b343810104
--- /dev/null
+++ b/testsuite/function-overloads/c_vpi.h
@@ -0,0 +1,11 @@
+
+void testC(vector a, float b, float c) {
+    printf("testC vector\n");
+}
+void testC(point a, float b, float c) {
+    printf("testC point\n");
+}
+void testC(int a, float b, float c) {
+    printf("testC int\n");
+}
+
diff --git a/testsuite/function-overloads/ref/out.txt b/testsuite/function-overloads/ref/out.txt
new file mode 100644
index 0000000000..71a7f1a25c
--- /dev/null
+++ b/testsuite/function-overloads/ref/out.txt
@@ -0,0 +1,46 @@
+Compiled test.osl -> test.oso
+testA int
+testB int
+testC int
+testD int
+testA int
+testB int
+testC int
+testD int2
+
+testA float
+testB float
+testC float
+testD float
+testA float
+testB float
+testC float
+testD float
+
+testD int
+testD int2
+testD float
+testD vector
+testD int
+testD int2
+testD float
+testD point
+testD int
+testD int2
+testD float
+testD color
+testD int
+testD int2
+testD float
+testD normal
+testD int
+testD int2
+testD float
+testD int
+testD int2
+testD float
+
+testE color
+testE vector
+
+
diff --git a/testsuite/function-overloads/run.py b/testsuite/function-overloads/run.py
new file mode 100755
index 0000000000..75c92e3991
--- /dev/null
+++ b/testsuite/function-overloads/run.py
@@ -0,0 +1,15 @@
+#!/usr/bin/env python
+
+realruntest = runtest
+
+def runtest (command, *args, **kwargs) :
+    passed = True
+    for arg in  ("-DORDER_1 ", ""):
+        command =  oslc(arg + "test.osl")
+        command += testshade("-g 1 1 test")
+        if realruntest(command, *args, **kwargs):
+            passed = False
+    return not passed
+
+command = ""
+
diff --git a/testsuite/function-overloads/test.osl b/testsuite/function-overloads/test.osl
new file mode 100644
index 0000000000..bf117aa9bb
--- /dev/null
+++ b/testsuite/function-overloads/test.osl
@@ -0,0 +1,120 @@
+
+#ifdef ORDER_1
+  #include "a_fcn.h"
+  #include "b_nci.h"
+  #include "c_vpi.h"
+
+  #include "a_ivp.h"
+  #include "b_vpf.h"
+  #include "c_cnf.h"
+#else
+  #include "a_ivp.h"
+  #include "b_vpf.h"
+  #include "c_cnf.h"
+
+  #include "a_fcn.h"
+  #include "b_nci.h"
+  #include "c_vpi.h"
+#endif
+
+int intval() { return 0; }
+float floatval() { return 0.0; }
+
+
+normal testD(normal a, float b, float c) {
+    printf("testD normal\n");
+    return normal(0);
+}
+color testD(color a, float b, float c) {
+    printf("testD color\n");
+    return color(1);
+}
+vector testD(vector a, float b, float c) {
+    printf("testD vector\n");
+    return vector(2);
+}
+point testD(point a, float b, float c) {
+    printf("testD point\n");
+    return point(3);
+}
+int testD(int a, float b, float c) {
+    printf("testD int\n");
+    return 4;
+}
+float testD(float a, float b, float c) {
+    printf("testD float\n");
+    return 5;
+}
+
+// This would break C++
+int testD(int a, int b, float c) {
+    printf("testD int2\n");
+    return 4;
+}
+
+int testE(int a, float b, color s) {
+    printf("testE color\n");
+    return 4;
+}
+int testE(float a, int b, vector v) {
+    printf("testE vector\n");
+    return 4;
+}
+
+shader test ()
+{
+  {
+      testA(intval(), 1.0, 1.0);
+      testB(intval(), 1.0, 1.0);
+      testC(intval(), 1.0, 1.0);
+      testD(intval(), 1.0, 1.0);
+      testA(intval(), 1, 1);
+      testB(intval(), 1, 1);
+      testC(intval(), 1, 1);
+      testD(intval(), 1, 1);
+      printf("\n");
+  }
+
+  {
+      testA(floatval(), 1.0, 1.0);
+      testB(floatval(), 1.0, 1.0);
+      testC(floatval(), 1.0, 1.0);
+      testD(floatval(), 1.0, 1.0);
+      testA(floatval(), 1, 1);
+      testB(floatval(), 1, 1);
+      testC(floatval(), 1, 1);
+      testD(floatval(), 1, 1);
+      printf("\n");
+  }
+
+  {
+    vector v0 = testD(intval(), 1.0, 1.0);
+    vector v1 = testD(intval(), 1, 1);
+    vector v2 = testD(floatval(), 1, 1);
+    vector v3 = testD(vector(0), 1, 1);
+    point  p0 = testD(intval(), 1.0, 1.0);
+    point  p1 = testD(intval(), 1, 1);
+    point  p2 = testD(floatval(), 1, 1);
+    point  p3 = testD(point(0), 1, 1);
+    color  c0 = testD(intval(), 1.0, 1.0);
+    color  c1 = testD(intval(), 1, 1);
+    color  c2 = testD(floatval(), 1, 1);
+    color  c3 = testD(color(0), 1, 1);
+    normal n0 = testD(intval(), 1.0, 1.0);
+    normal n1 = testD(intval(), 1, 1);
+    normal n2 = testD(floatval(), 1, 1);
+    normal n3 = testD(normal(0), 1, 1);
+    int    i0 = testD(intval(), 1.0, 1.0);
+    int    i1 = testD(intval(), 1, 1);
+    int    i2 = (int) testD(floatval(), 1, 1);
+    float  f0 = testD(intval(), 1.0, 1.0);
+    float  f1 = testD(intval(), 1, 1);
+    float  f2 = testD(floatval(), 1, 1);
+    printf("\n");
+  }
+  {
+    testE(1.0, 1, color(0));
+    testE(1, 1.0, vector(0));
+    printf("\n");
+  }
+}
diff --git a/testsuite/oslc-err-funcoverload/ref/out.txt b/testsuite/oslc-err-funcoverload/ref/out.txt
new file mode 100644
index 0000000000..76b8c7ad07
--- /dev/null
+++ b/testsuite/oslc-err-funcoverload/ref/out.txt
@@ -0,0 +1,9 @@
+test.osl:8: error: No matching function call to 'funca ()'
+  Candidates are:
+    test.osl:3	void funca (int, int)
+    test.osl:2	void funca (int)
+test.osl:9: error: No matching function call to 'funca (int, int, int)'
+  Candidates are:
+    test.osl:3	void funca (int, int)
+    test.osl:2	void funca (int)
+FAILED test.osl
diff --git a/testsuite/oslc-err-funcoverload/run.py b/testsuite/oslc-err-funcoverload/run.py
new file mode 100755
index 0000000000..d901e097a5
--- /dev/null
+++ b/testsuite/oslc-err-funcoverload/run.py
@@ -0,0 +1,5 @@
+#!/usr/bin/env python
+
+# command = oslc("test.osl")
+# don't even need that -- it's automatic
+failureok = 1     # this test is expected to have oslc errors
diff --git a/testsuite/oslc-err-funcoverload/test.osl b/testsuite/oslc-err-funcoverload/test.osl
new file mode 100644
index 0000000000..a53cfe2821
--- /dev/null
+++ b/testsuite/oslc-err-funcoverload/test.osl
@@ -0,0 +1,11 @@
+
+void funca(int a) {}
+void funca(int a, int b) {}
+
+
+shader test()
+{
+    funca();
+    funca(1,2,3);
+}
+

From b6331b50122c1d898a5b46fbde437c42f8f3e5dc Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Sun, 21 Jan 2018 12:54:25 -0500
Subject: [PATCH 2/8] Disallow float to int argument conversion.

---
 src/liboslcomp/typecheck.cpp                | 5 ++++-
 testsuite/function-overloads/test.osl       | 5 +++--
 testsuite/oslc-err-funcoverload/ref/out.txt | 8 ++++++++
 testsuite/oslc-err-funcoverload/test.osl    | 6 ++++--
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 130756a954..809714f50d 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1164,6 +1164,9 @@ ASTfunction_call::typecheck_struct_constructor ()
 /// return type is used as a possible tie breaker in case of any ambiguity.
 /// The highest score for an argument is given when it is an exact match.
 ///
+/// Float to int coercion is scored, but is currently a synmonym for kNoMatch
+/// as the spec does not allow implicit float to int conversion.
+///
 class CandidateFunctions {
     enum {
         kExactMatch     = 100,
@@ -1174,7 +1177,7 @@ class CandidateFunctions {
         kNoMatch        = 0,
 
         // Additional named rules
-        kFPToIntegral   = 60,          // = kIntegralToFP to match c++
+        kFPToIntegral   = kNoMatch,    // = kIntegralToFP to match c++
     };
     struct Candidate {
         FunctionSymbol* sym;
diff --git a/testsuite/function-overloads/test.osl b/testsuite/function-overloads/test.osl
index bf117aa9bb..082b99fd19 100644
--- a/testsuite/function-overloads/test.osl
+++ b/testsuite/function-overloads/test.osl
@@ -112,9 +112,10 @@ shader test ()
     float  f2 = testD(floatval(), 1, 1);
     printf("\n");
   }
+
   {
-    testE(1.0, 1, color(0));
-    testE(1, 1.0, vector(0));
+    testE(1, 1, color(0));
+    testE(1, 1, vector(0));
     printf("\n");
   }
 }
diff --git a/testsuite/oslc-err-funcoverload/ref/out.txt b/testsuite/oslc-err-funcoverload/ref/out.txt
index 76b8c7ad07..d4df2e206d 100644
--- a/testsuite/oslc-err-funcoverload/ref/out.txt
+++ b/testsuite/oslc-err-funcoverload/ref/out.txt
@@ -6,4 +6,12 @@ test.osl:9: error: No matching function call to 'funca (int, int, int)'
   Candidates are:
     test.osl:3	void funca (int, int)
     test.osl:2	void funca (int)
+test.osl:10: error: No matching function call to 'funca (float, int)'
+  Candidates are:
+    test.osl:3	void funca (int, int)
+    test.osl:2	void funca (int)
+test.osl:11: error: No matching function call to 'funca (int, float)'
+  Candidates are:
+    test.osl:3	void funca (int, int)
+    test.osl:2	void funca (int)
 FAILED test.osl
diff --git a/testsuite/oslc-err-funcoverload/test.osl b/testsuite/oslc-err-funcoverload/test.osl
index a53cfe2821..9d60986249 100644
--- a/testsuite/oslc-err-funcoverload/test.osl
+++ b/testsuite/oslc-err-funcoverload/test.osl
@@ -5,7 +5,9 @@ void funca(int a, int b) {}
 
 shader test()
 {
-    funca();
-    funca(1,2,3);
+    funca();        // fail too few arguments
+    funca(1, 2, 3); // fail too many arguments
+    funca(1.0, 2);  // fail float -> int
+    funca(1, 2.0);  // fail float -> int
 }
 

From 6939f66648bce4e9313d7cbb51443881444e87af Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Sun, 21 Jan 2018 13:07:59 -0500
Subject: [PATCH 3/8] Error on return type ambiguity.

---
 src/liboslcomp/typecheck.cpp                |  9 +++++----
 testsuite/function-overloads/ref/out.txt    |  3 +++
 testsuite/function-overloads/test.osl       | 10 ++++++++++
 testsuite/isconstant/test.osl               |  2 +-
 testsuite/oslc-err-funcoverload/ref/out.txt | 13 +++++++++----
 testsuite/oslc-err-funcoverload/test.osl    |  6 ++++++
 6 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 809714f50d..3ff59c6996 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1384,17 +1384,18 @@ class CandidateFunctions {
             default: break;
         }
 
-        int ambiguity = 0;
+        int ambiguity = -1;
         std::pair<const Candidate*, int> c = { nullptr, -1 };
         for (auto& candidate : m_candidates) {
             // re-score based on matching return value
-            if (candidate.rscore > c.second)
+            if (candidate.rscore > c.second) {
+                ambiguity = -1; // higher score, no longer ambiguous
                 c = std::make_pair(&candidate, candidate.rscore);
-            else if (candidate.rscore == c.second)
+            } else if (candidate.rscore == c.second)
                 ambiguity = candidate.rscore;
         }
 
-        if (ambiguity || strict) {
+        if ((ambiguity != -1) || strict) {
             ASSERT (caller);
             reportError(caller, m_candidates[0].name(), false,
                         !m_candidates.empty(), "Ambiguous call to");
diff --git a/testsuite/function-overloads/ref/out.txt b/testsuite/function-overloads/ref/out.txt
index 71a7f1a25c..be70720adf 100644
--- a/testsuite/function-overloads/ref/out.txt
+++ b/testsuite/function-overloads/ref/out.txt
@@ -43,4 +43,7 @@ testD float
 testE color
 testE vector
 
+funcb.color
+funcb.float
+funcb.int
 
diff --git a/testsuite/function-overloads/test.osl b/testsuite/function-overloads/test.osl
index 082b99fd19..5dab22a0d4 100644
--- a/testsuite/function-overloads/test.osl
+++ b/testsuite/function-overloads/test.osl
@@ -61,6 +61,10 @@ int testE(float a, int b, vector v) {
     return 4;
 }
 
+int   funcb() { printf("funcb.int\n");   return 1; }
+float funcb() { printf("funcb.float\n"); return 2; }
+color funcb() { printf("funcb.color\n"); return 3; }
+
 shader test ()
 {
   {
@@ -118,4 +122,10 @@ shader test ()
     testE(1, 1, vector(0));
     printf("\n");
   }
+
+  {
+    (color) funcb();
+    (float) funcb();
+    (int) funcb();
+  }
 }
diff --git a/testsuite/isconstant/test.osl b/testsuite/isconstant/test.osl
index 22566a3419..30c2270030 100644
--- a/testsuite/isconstant/test.osl
+++ b/testsuite/isconstant/test.osl
@@ -4,5 +4,5 @@ shader test (float A = 1)
 {
     printf ("Is param A const? %d\n", isconstant(A));
     printf ("Is 2*A const? %d\n", isconstant(2*A));
-    printf ("Is noise(P) const? %d\n", isconstant(noise(P)));
+    printf ("Is noise(P) const? %d\n", isconstant((color)noise(P)));
 }
diff --git a/testsuite/oslc-err-funcoverload/ref/out.txt b/testsuite/oslc-err-funcoverload/ref/out.txt
index d4df2e206d..a6151ea6ce 100644
--- a/testsuite/oslc-err-funcoverload/ref/out.txt
+++ b/testsuite/oslc-err-funcoverload/ref/out.txt
@@ -1,17 +1,22 @@
-test.osl:8: error: No matching function call to 'funca ()'
+test.osl:11: error: No matching function call to 'funca ()'
   Candidates are:
     test.osl:3	void funca (int, int)
     test.osl:2	void funca (int)
-test.osl:9: error: No matching function call to 'funca (int, int, int)'
+test.osl:12: error: No matching function call to 'funca (int, int, int)'
   Candidates are:
     test.osl:3	void funca (int, int)
     test.osl:2	void funca (int)
-test.osl:10: error: No matching function call to 'funca (float, int)'
+test.osl:13: error: No matching function call to 'funca (float, int)'
   Candidates are:
     test.osl:3	void funca (int, int)
     test.osl:2	void funca (int)
-test.osl:11: error: No matching function call to 'funca (int, float)'
+test.osl:14: error: No matching function call to 'funca (int, float)'
   Candidates are:
     test.osl:3	void funca (int, int)
     test.osl:2	void funca (int)
+test.osl:15: error: Ambiguous call to 'funcb'
+  Candidates are:
+    test.osl:7	color funcb ()
+    test.osl:6	float funcb ()
+    test.osl:5	int funcb ()
 FAILED test.osl
diff --git a/testsuite/oslc-err-funcoverload/test.osl b/testsuite/oslc-err-funcoverload/test.osl
index 9d60986249..acc5a63082 100644
--- a/testsuite/oslc-err-funcoverload/test.osl
+++ b/testsuite/oslc-err-funcoverload/test.osl
@@ -2,6 +2,9 @@
 void funca(int a) {}
 void funca(int a, int b) {}
 
+int funcb() { return 1; }
+float funcb() { return 2; }
+color funcb() { return 3; }
 
 shader test()
 {
@@ -9,5 +12,8 @@ shader test()
     funca(1, 2, 3); // fail too many arguments
     funca(1.0, 2);  // fail float -> int
     funca(1, 2.0);  // fail float -> int
+    funcb();        // fail unresolvable ambiguity
+
+    (int) funcb();  // ok
 }
 

From c7d0dc379e92db75de8e176ad0794d6647d0a893 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Sun, 21 Jan 2018 13:27:54 -0500
Subject: [PATCH 4/8] Prefer conversion between spatial types over color, and
 between triple types over promotion.

---
 src/liboslcomp/typecheck.cpp             | 20 ++++++++++++++++++--
 testsuite/function-overloads/ref/out.txt |  1 +
 testsuite/function-overloads/test.osl    |  1 +
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 3ff59c6996..207399d86a 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1163,6 +1163,11 @@ ASTfunction_call::typecheck_struct_constructor ()
 /// When choosing which varaint to use precedence is given to arguments and the
 /// return type is used as a possible tie breaker in case of any ambiguity.
 /// The highest score for an argument is given when it is an exact match.
+/// If the argument needs to be coerced into a type OSL will prefer going
+/// from any triple-types to any other triple-type before promotion of a
+/// float to a triple.  When converting one triple to another triple, OSL will
+/// prefer to coerce from one of the spatial triples (vector/point/normal) to
+/// another spatial triple before coercion from a spatial triple to color.
 ///
 /// Float to int coercion is scored, but is currently a synmonym for kNoMatch
 /// as the spec does not allow implicit float to int conversion.
@@ -1177,7 +1182,9 @@ class CandidateFunctions {
         kNoMatch        = 0,
 
         // Additional named rules
-        kFPToIntegral   = kNoMatch,    // = kIntegralToFP to match c++
+        kFPToIntegral   = kNoMatch,       // = kIntegralToFP to match c++
+        kSpatialCoerce  = kCoercable + 9, // prefer vector/point/normal conversion over color
+        kTripleCoerce   = kCoercable + 4, // prefer triple conversion over float promotion
     };
     struct Candidate {
         FunctionSymbol* sym;
@@ -1222,8 +1229,17 @@ class CandidateFunctions {
             return kArrayMatch;
         }
 
-        if (assignable (expected, actual))
+        if (assignable (expected, actual)) {
+            // Prefer conversion between spatial types
+            if (actual.is_vectriple_based() && expected.is_vectriple_based())
+                return kSpatialCoerce;
+            // Prefer conversion between triple types
+            if (!actual.is_closure() && actual.is_triple() &&
+                !expected.is_closure() && expected.is_triple())
+                return kTripleCoerce;
+            // Everything else
             return kCoercable;
+        }
 
         return kNoMatch;
     }
diff --git a/testsuite/function-overloads/ref/out.txt b/testsuite/function-overloads/ref/out.txt
index be70720adf..08ac7cc218 100644
--- a/testsuite/function-overloads/ref/out.txt
+++ b/testsuite/function-overloads/ref/out.txt
@@ -42,6 +42,7 @@ testD float
 
 testE color
 testE vector
+testE vector
 
 funcb.color
 funcb.float
diff --git a/testsuite/function-overloads/test.osl b/testsuite/function-overloads/test.osl
index 5dab22a0d4..3df00c4c9e 100644
--- a/testsuite/function-overloads/test.osl
+++ b/testsuite/function-overloads/test.osl
@@ -120,6 +120,7 @@ shader test ()
   {
     testE(1, 1, color(0));
     testE(1, 1, vector(0));
+    testE(1, 1, point(0));
     printf("\n");
   }
 

From 9f40157e644807fb8a4ac76b52a11fe6b43c5659 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Sun, 21 Jan 2018 15:05:21 -0500
Subject: [PATCH 5/8] Add legacy overload checking via
 OSL_LEGACY_FUNCTION_RESOLUTION env variable.

---
 src/liboslcomp/typecheck.cpp | 120 +++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 207399d86a..26dde6a354 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1429,6 +1429,104 @@ class CandidateFunctions {
 };
 
 
+///
+/// Check how a polymorphic function variant was chosen in prior versions.
+/// Check is performed based on the env variable OSL_LEGACY_FUNCTION_RESOLUTION
+///
+/// OSL_LEGACY_FUNCTION_RESOLUTION      // check resolution matches old behavior
+/// OSL_LEGACY_FUNCTION_RESOLUTION=0    // no checking
+/// OSL_LEGACY_FUNCTION_RESOLUTION=err  // check and error on mismatch
+/// OSL_LEGACY_FUNCTION_RESOLUTION=use  // check and use prior on mismatch
+class LegacyOverload
+{
+    OSLCompilerImpl* m_compiler;
+    ASTfunction_call* m_func;
+    FunctionSymbol* m_root;
+    bool (ASTNode::*m_check_arglist) (const char *funcname, ASTNode::ref arg,
+                                    const char *formals, bool coerce);
+
+    std::pair<FunctionSymbol*,TypeSpec>
+    typecheck_polys (TypeSpec expected, bool coerceargs, bool equivreturn)
+    {
+        const char* name = m_func->func()->name().c_str();
+        for (FunctionSymbol* poly = m_root; poly; poly = poly->nextpoly()) {
+            const char *code = poly->argcodes().c_str();
+            int advance;
+            TypeSpec returntype = m_compiler->type_from_code (code, &advance);
+            code += advance;
+            if ((m_func->*m_check_arglist) (name, m_func->args(), code, coerceargs)) {
+                // Return types also must match if not coercible
+                if (expected == returntype ||
+                    (equivreturn && equivalent(expected, returntype)) ||
+                    expected == TypeSpec()) {
+                    return { poly, returntype };
+                }
+            }
+        }
+        return { nullptr, TypeSpec() };
+    }
+
+public:
+
+    LegacyOverload ( OSLCompilerImpl* comp, ASTfunction_call* func,
+                     FunctionSymbol* root,
+                     bool (ASTNode::*checkfunc) (const char *funcname,
+                                                 ASTNode::ref arg,
+                                                 const char *formals,
+                                                 bool coerce))
+        : m_compiler(comp), m_func(func), m_root(root), m_check_arglist(checkfunc) {
+    }
+
+    FunctionSymbol*
+    operator () (TypeSpec expected)
+    {
+        bool match = false;
+        TypeSpec typespec;
+        FunctionSymbol* sym;
+
+        // Look for an exact match, including expected return type
+        std::tie(sym, typespec) = typecheck_polys (expected, false, false);
+        if (typespec != TypeSpec())
+            match = true;
+
+        // Now look for an exact match for arguments, but equivalent return type
+        std::tie(sym, typespec) = typecheck_polys (expected, false, true);
+        if (typespec != TypeSpec())
+            match = true;
+
+        // Now look for an exact match on args, but any return type
+        if (! match && expected != TypeSpec()) {
+            std::tie(sym, typespec) = typecheck_polys (TypeSpec(), false, false);
+            if (typespec != TypeSpec())
+                match = true;
+        }
+
+        // Now look for a coercible match of args, exact march on return type
+        if (! match) {
+            std::tie(sym, typespec) = typecheck_polys (expected, true, false);
+            if (typespec != TypeSpec())
+                match = true;
+        }
+
+        // Now look for a coercible match of args, equivalent march on return type
+        if (! match) {
+            std::tie(sym, typespec) = typecheck_polys (expected, true, true);
+            if (typespec != TypeSpec())
+                match = true;
+        }
+
+        // All that failed, try for a coercible match on everything
+        if (! match && expected != TypeSpec()) {
+            std::tie(sym, typespec) = typecheck_polys (TypeSpec(), true, false);
+            if (typespec != TypeSpec())
+                match = true;
+        }
+
+        return match ? sym : nullptr;
+    }
+};
+
+
 
 TypeSpec
 ASTfunction_call::typecheck (TypeSpec expected)
@@ -1446,6 +1544,28 @@ ASTfunction_call::typecheck (TypeSpec expected)
     CandidateFunctions candidates(m_compiler, expected, args(), poly);
     std::tie(m_sym, m_typespec) = candidates.best(this);
 
+    // Check resolution against prior versions of OSL
+    static const char* OSL_LEGACY = ::getenv("OSL_LEGACY_FUNCTION_RESOLUTION");
+    if (OSL_LEGACY && strcmp(OSL_LEGACY, "0")) {
+        auto* legacy = LegacyOverload(m_compiler, this, poly,
+                                   &ASTfunction_call::check_arglist)(expected);
+        if (m_sym != legacy) {
+            strcasecmp(OSL_LEGACY, "err") == 0
+                ? error("overload chosen differs from OSL 1.9")
+                : warning("overload chosen differs from OSL 1.9");
+
+            m_compiler->errhandler().message ("  Current overload is ");
+            !m_sym ? m_compiler->errhandler().message("<none>")
+                   : candidates.reportAmbiguity(static_cast<FunctionSymbol*>(m_sym));
+            m_compiler->errhandler().message ("  Prior overload was ");
+            !legacy ? m_compiler->errhandler().message("<none>")
+                       : candidates.reportAmbiguity(legacy);
+        
+            if (strcasecmp(OSL_LEGACY, "use") == 0)
+                m_sym = legacy;
+        }
+    }
+
     if (m_sym != nullptr) {
         if (is_user_function()) {
             if (func()->number_of_returns() == 0 &&

From 5383e642350c707b192a9fa898137511f18fb103 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Mon, 29 Jan 2018 15:28:29 -0500
Subject: [PATCH 6/8] Update CandidateFunctions documentation.

---
 src/liboslcomp/typecheck.cpp | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 26dde6a354..2e7ec9cd93 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1160,14 +1160,23 @@ ASTfunction_call::typecheck_struct_constructor ()
 ///
 /// Score a set of polymorphic functions based on arguments & return type.
 ///
-/// When choosing which varaint to use precedence is given to arguments and the
-/// return type is used as a possible tie breaker in case of any ambiguity.
-/// The highest score for an argument is given when it is an exact match.
-/// If the argument needs to be coerced into a type OSL will prefer going
-/// from any triple-types to any other triple-type before promotion of a
-/// float to a triple.  When converting one triple to another triple, OSL will
-/// prefer to coerce from one of the spatial triples (vector/point/normal) to
-/// another spatial triple before coercion from a spatial triple to color.
+/// The idea is that every function with the same name (visible from the scope)
+/// is evaluated and 'scored' against the actual arguments.
+///
+/// An exact match of all arguments will have the highest score and be chosen.
+/// If there is no exact match, then each possible overload is given a score
+/// based on the number and type of substitutions or coercions they require.
+///
+/// Different types of coercions having different costs so that: int to float is
+/// scored relatively high, beating out all other coercions; spatial-triple
+/// to spatial-triple is a closer match than spatial-triple to color;
+/// and triple to triple is a closer match than float to triple.
+///
+/// If a single choice has a best score, it wins.
+/// If there is a tie (and only then), the return type is considered in the score.
+/// If there is still not a single winner using the return type, it is considered
+/// an error whose message will show all the high scoring possibilities that
+/// cannot be dis-ambiguated.
 ///
 /// Float to int coercion is scored, but is currently a synmonym for kNoMatch
 /// as the spec does not allow implicit float to int conversion.

From 486aee8643e2e42295e1c17add8b283e45ddeb6f Mon Sep 17 00:00:00 2001
From: Larry Gritz <lg@larrygritz.com>
Date: Tue, 30 Jan 2018 14:21:36 -0800
Subject: [PATCH 7/8] Touch up typecheck of type_constructor to help with new
 overload rules

---
 src/liboslcomp/typecheck.cpp | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 2e7ec9cd93..8ede55c8d5 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -779,9 +779,6 @@ ASTtypecast_expression::typecheck (TypeSpec expected)
 TypeSpec
 ASTtype_constructor::typecheck (TypeSpec expected)
 {
-    // FIXME - closures
-    typecheck_children ();
-
     // Hijack the usual function arg-checking routines.
     // So we have a set of valid patterns for each type constructor:
     static const char *float_patterns[] = { "ff", "fi", NULL };
@@ -793,19 +790,28 @@ ASTtype_constructor::typecheck (TypeSpec expected)
     static const char *int_patterns[] = { "if", "ii", NULL };
     // Select the pattern for the type of constructor we are...
     const char **patterns = NULL;
-    if (typespec().is_float())
+    TypeSpec argexpected;  // default to unknown
+    if (typespec().is_float()) {
         patterns = float_patterns;
-    else if (typespec().is_triple())
+    } else if (typespec().is_triple()) {
         patterns = triple_patterns;
-    else if (typespec().is_matrix())
+        // For triples, the constructor that takes just one argument is often
+        // is used as a typecast, i.e. (vector)foo <==> vector(foo)
+        // So pass on the expected type so it can resolve polymorphism in
+        // the expected way.
+        if (listlength(args()) == 1)
+            argexpected = m_typespec;
+    } else if (typespec().is_matrix()) {
         patterns = matrix_patterns;
-    else if (typespec().is_int())
+    } else if (typespec().is_int()) {
         patterns = int_patterns;
-    if (! patterns) {
+    } else {
         error ("Cannot construct type '%s'", type_c_str(typespec()));
         return m_typespec;
     }
 
+    typecheck_children (argexpected);
+
     // Try to get a match, first without type coercion of the arguments,
     // then with coercion.
     for (int co = 0;  co < 2;  ++co) {

From 6aca4441e361ca92f82181472c794bd931a188c8 Mon Sep 17 00:00:00 2001
From: Frederich Munch <marsupial@users.noreply.github.com>
Date: Thu, 1 Feb 2018 13:49:42 -0500
Subject: [PATCH 8/8] Update function overload rules to only warn on ambiguity
 and select one based on return type.

---
 src/liboslcomp/typecheck.cpp                | 144 ++++++++++++++------
 testsuite/function-overloads/ref/out.txt    |  65 +++++++++
 testsuite/function-overloads/test.osl       |  54 ++++++++
 testsuite/oslc-err-funcoverload/ref/out.txt |   7 +-
 4 files changed, 225 insertions(+), 45 deletions(-)

diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp
index 8ede55c8d5..e798887f06 100644
--- a/src/liboslcomp/typecheck.cpp
+++ b/src/liboslcomp/typecheck.cpp
@@ -1180,9 +1180,11 @@ ASTfunction_call::typecheck_struct_constructor ()
 ///
 /// If a single choice has a best score, it wins.
 /// If there is a tie (and only then), the return type is considered in the score.
-/// If there is still not a single winner using the return type, it is considered
-/// an error whose message will show all the high scoring possibilities that
-/// cannot be dis-ambiguated.
+/// If there is still not a single winner then a function is chosen by ranking
+/// the possible return types, using the following precedence:
+///   float, int, color, vector, point, normal, matrix, string
+/// A warning is shown, printing which was function chosen and the list of all
+/// that were considered ambiguous.
 ///
 /// Float to int coercion is scored, but is currently a synmonym for kNoMatch
 /// as the spec does not allow implicit float to int conversion.
@@ -1221,6 +1223,8 @@ class CandidateFunctions {
     ASTNode::ref m_args;
     size_t m_nargs;
 
+    FunctionSymbol* m_called; // Function called by name (can be NULL!)
+
     const char* scoreWildcard(int& argscore, size_t& fargs, const char* args) const {
         while (fargs < m_nargs) {
             argscore += kMatchAnything;
@@ -1352,9 +1356,15 @@ class CandidateFunctions {
         return argscore;
     }
 
+    void candidateHeader(const char* msg) const {
+        m_compiler->errhandler().message("  %s:\n", msg);
+    }
+
 public:
-    CandidateFunctions(OSLCompilerImpl* compiler, TypeSpec rval, ASTNode::ref args, FunctionSymbol* func) :
-        m_compiler(compiler), m_rval(rval), m_args(args), m_nargs(0) {
+    CandidateFunctions(OSLCompilerImpl* compiler, TypeSpec rval, ASTNode::ref args,
+                       FunctionSymbol* func) :
+        m_compiler(compiler), m_rval(rval), m_args(args), m_nargs(0),
+        m_called(func) {
 
         //std::cerr << "Matching " << func->name() << " formals='" << (rval.simpletype().basetype != TypeDesc::UNKNOWN ?  compiler->code_from_type (rval) : " ");
         for (ASTNode::ref arg = m_args; arg; arg = arg->next()) {
@@ -1371,27 +1381,28 @@ class CandidateFunctions {
         }
     }
 
-    void reportError(ASTNode* caller,
-                     std::string name = "", bool showArgs = true,
-                     bool candidateMsg = true,
-                     const char* msg = "No matching function call to") const {
+    void reportAmbiguity(ASTNode* caller, const ustring& funcname,
+                         bool candidateMsg = true,
+                         const char* msg = "No matching function call to",
+                         bool asWarning = true, bool showArgs = true) const {
+        std::string argstr = funcname.string();
         if (showArgs) {
-            name += " (";
+            argstr += " (";
             const char *comma = "";
             for (ASTNode::ref arg = m_args; arg; arg = arg->next()) {
-                name += comma;
-                name += arg->typespec().string();
+                argstr += comma;
+                argstr += arg->typespec().string();
                 comma = ", ";
             }
-            name += ")";
+            argstr += ")";
         }
-
-        caller->error ("%s '%s'", msg, name);
+        asWarning ? caller->warning ("%s '%s'", msg, argstr)
+                  : caller->error ("%s '%s'", msg, argstr);
         if (candidateMsg)
-            m_compiler->errhandler().message("  Candidates are:\n");
+            candidateHeader("Candidates are");
     }
 
-    void reportAmbiguity(FunctionSymbol* sym) const {
+    void reportFunction(FunctionSymbol* sym) const {
         int advance;
         const char *formals = sym->argcodes().c_str();
         TypeSpec returntype = m_compiler->type_from_code (formals, &advance);
@@ -1408,10 +1419,25 @@ class CandidateFunctions {
                      m_compiler->typelist_from_code(formals).c_str());
     }
 
-    std::pair<FunctionSymbol*, TypeSpec> best(ASTNode* caller, bool strict = 0) {
+    std::pair<FunctionSymbol*, TypeSpec>
+    best(ASTNode* caller, const ustring& funcname) {
         switch (m_candidates.size()) {
-            case 0:  return { nullptr, TypeSpec() };
-            case 1:  return { m_candidates[0].sym, m_candidates[0].rtype };
+            case 0:
+                // Nothing at all, Error
+                // If m_called is 0, then user tried to call an undefined func.
+                // Might be nice to fuzzy match funcname against m_compiler->symtab()
+                reportAmbiguity(caller, funcname,
+                                m_called != nullptr /*Candidate Msg?*/,
+                                "No matching function call to",
+                                false /*As warning*/);
+                for (FunctionSymbol* f = m_called; f; f = f->nextpoly())
+                    reportFunction(f);
+
+                return { nullptr, TypeSpec() };
+
+            case 1: // Success
+                return { m_candidates[0].sym, m_candidates[0].rtype };
+
             default: break;
         }
 
@@ -1426,17 +1452,64 @@ class CandidateFunctions {
                 ambiguity = candidate.rscore;
         }
 
-        if ((ambiguity != -1) || strict) {
+        ASSERT (c.first && c.first->sym);
+
+        if (ambiguity != -1) {
             ASSERT (caller);
-            reportError(caller, m_candidates[0].name(), false,
-                        !m_candidates.empty(), "Ambiguous call to");
+            if (true /*m_rval.simpletype().is_unknown()*/) {
+                // Ambiguity because the return type desired is unknown
+                //   float noise(point p)
+                //   color noise(point p);
+                //   float mix(color a, color b, float mx);
+                //   color mix(color a, color b, color mx);
+                //   mix(c0, c1, noise(P));
+
+                // Sort m_candidates, so the ranking code can be much more
+                // legible, and ambiguities will be reported in order they
+                // would be chosen.
+                std::sort(m_candidates.begin(), m_candidates.end(),
+                    [](const Candidate& a, const Candidate& b) -> bool {
+                        auto rank = [](const TypeSpec& s) -> int {
+                            if (s == TypeDesc::TypeFloat)
+                                return 0;
+                            if (s == TypeDesc::TypeInt)
+                                return 1;
+                            if (s == TypeDesc::TypeColor)
+                                return 2;
+                            if (s == TypeDesc::TypeVector)
+                                return 3;
+                            if (s == TypeDesc::TypePoint)
+                                return 4;
+                            if (s == TypeDesc::TypeNormal)
+                                return 5;
+                            if (s == TypeDesc::TypeMatrix)
+                                return 6;
+                            if (s == TypeDesc::TypeString)
+                                return 7;
+                            ASSERT (0 && "Unreachable");
+                            return std::numeric_limits<int>::max();
+                        };
+                        return rank(a.rtype.simpletype())
+                                 < rank(b.rtype.simpletype());
+                    });
+
+                // New choice is now front of the list
+                c = std::make_pair(&m_candidates.front(),
+                                   m_candidates.front().rscore);
+            }
+
+            reportAmbiguity(caller, funcname, false, "Ambiguous call to");
+
+            candidateHeader("Chosen function is");
+            reportFunction(c.first->sym);
+
+            candidateHeader("Other candidates are");
             for (auto& candidate : m_candidates) {
-                if (candidate.rscore >= ambiguity)
-                    reportAmbiguity(candidate.sym);
+                if (candidate.sym != c.first->sym)
+                    reportFunction(candidate.sym);
             }
         }
 
-        ASSERT (c.first);
         return {c.first->sym, c.first->rtype};
     }
 
@@ -1557,7 +1630,7 @@ ASTfunction_call::typecheck (TypeSpec expected)
     FunctionSymbol* poly = func();
 
     CandidateFunctions candidates(m_compiler, expected, args(), poly);
-    std::tie(m_sym, m_typespec) = candidates.best(this);
+    std::tie(m_sym, m_typespec) = candidates.best(this, m_name);
 
     // Check resolution against prior versions of OSL
     static const char* OSL_LEGACY = ::getenv("OSL_LEGACY_FUNCTION_RESOLUTION");
@@ -1571,10 +1644,10 @@ ASTfunction_call::typecheck (TypeSpec expected)
 
             m_compiler->errhandler().message ("  Current overload is ");
             !m_sym ? m_compiler->errhandler().message("<none>")
-                   : candidates.reportAmbiguity(static_cast<FunctionSymbol*>(m_sym));
+                   : candidates.reportFunction(static_cast<FunctionSymbol*>(m_sym));
             m_compiler->errhandler().message ("  Prior overload was ");
             !legacy ? m_compiler->errhandler().message("<none>")
-                       : candidates.reportAmbiguity(legacy);
+                       : candidates.reportFunction(legacy);
         
             if (strcasecmp(OSL_LEGACY, "use") == 0)
                 m_sym = legacy;
@@ -1595,19 +1668,6 @@ ASTfunction_call::typecheck (TypeSpec expected)
         return m_typespec;
     }
 
-    // Ambiguity has already been reported.
-    if (!candidates.empty())
-        return TypeSpec();
-
-    // Couldn't find any way to match any polymorphic version of the
-    // function that we know about.  OK, at least try for helpful error
-    // message.
-    candidates.reportError(this, m_name.string(), true, poly != nullptr);
-    while (poly) {
-        candidates.reportAmbiguity(poly);
-        poly = poly->nextpoly();
-    }
-
     return TypeSpec();
 }
 
diff --git a/testsuite/function-overloads/ref/out.txt b/testsuite/function-overloads/ref/out.txt
index 08ac7cc218..6777c47cb3 100644
--- a/testsuite/function-overloads/ref/out.txt
+++ b/testsuite/function-overloads/ref/out.txt
@@ -1,3 +1,59 @@
+test.osl:177: warning: Ambiguous call to 'freturn ()'
+  Chosen function is:
+    test.osl:71	float freturn ()
+  Other candidates are:
+    test.osl:68	int freturn ()
+    test.osl:72	color freturn ()
+    test.osl:73	vector freturn ()
+    test.osl:69	point freturn ()
+    test.osl:74	normal freturn ()
+    test.osl:75	matrix freturn ()
+    test.osl:70	string freturn ()
+test.osl:178: warning: Ambiguous call to 'ireturn ()'
+  Chosen function is:
+    test.osl:80	int ireturn ()
+  Other candidates are:
+    test.osl:77	color ireturn ()
+    test.osl:81	vector ireturn ()
+    test.osl:78	point ireturn ()
+    test.osl:82	normal ireturn ()
+    test.osl:83	matrix ireturn ()
+    test.osl:79	string ireturn ()
+test.osl:179: warning: Ambiguous call to 'creturn ()'
+  Chosen function is:
+    test.osl:88	color creturn ()
+  Other candidates are:
+    test.osl:85	vector creturn ()
+    test.osl:87	point creturn ()
+    test.osl:86	normal creturn ()
+    test.osl:90	matrix creturn ()
+    test.osl:89	string creturn ()
+test.osl:180: warning: Ambiguous call to 'vreturn ()'
+  Chosen function is:
+    test.osl:93	vector vreturn ()
+  Other candidates are:
+    test.osl:92	point vreturn ()
+    test.osl:95	normal vreturn ()
+    test.osl:94	matrix vreturn ()
+    test.osl:96	string vreturn ()
+test.osl:181: warning: Ambiguous call to 'preturn ()'
+  Chosen function is:
+    test.osl:101	point preturn ()
+  Other candidates are:
+    test.osl:99	normal preturn ()
+    test.osl:100	matrix preturn ()
+    test.osl:98	string preturn ()
+test.osl:182: warning: Ambiguous call to 'nreturn ()'
+  Chosen function is:
+    test.osl:104	normal nreturn ()
+  Other candidates are:
+    test.osl:103	matrix nreturn ()
+    test.osl:105	string nreturn ()
+test.osl:183: warning: Ambiguous call to 'mreturn ()'
+  Chosen function is:
+    test.osl:107	matrix mreturn ()
+  Other candidates are:
+    test.osl:108	string mreturn ()
 Compiled test.osl -> test.oso
 testA int
 testB int
@@ -48,3 +104,12 @@ funcb.color
 funcb.float
 funcb.int
 
+freturn.float
+ireturn.int
+creturn.color
+vreturn.vector
+preturn.point
+nreturn.normal
+mreturn.matrix
+
+
diff --git a/testsuite/function-overloads/test.osl b/testsuite/function-overloads/test.osl
index 3df00c4c9e..2100ba0b0c 100644
--- a/testsuite/function-overloads/test.osl
+++ b/testsuite/function-overloads/test.osl
@@ -65,6 +65,48 @@ int   funcb() { printf("funcb.int\n");   return 1; }
 float funcb() { printf("funcb.float\n"); return 2; }
 color funcb() { printf("funcb.color\n"); return 3; }
 
+int    freturn() { printf("freturn.int\n"); return 1; }
+point  freturn() { printf("freturn.point\n"); return 1; }
+string freturn() { printf("freturn.string\n"); return ""; }
+float  freturn() { printf("freturn.float\n"); return 1; }
+color  freturn() { printf("freturn.color\n"); return 1; }
+vector freturn() { printf("freturn.vector\n"); return 1; }
+normal freturn() { printf("freturn.normal\n"); return 1; }
+matrix freturn() { printf("freturn.matrix\n"); return 1; }
+
+color  ireturn() { printf("ireturn.color\n"); return 1; }
+point  ireturn() { printf("ireturn.point\n"); return 1; }
+string ireturn() { printf("ireturn.string\n"); return ""; }
+int    ireturn() { printf("ireturn.int\n"); return 1; }
+vector ireturn() { printf("ireturn.vector\n"); return 1; }
+normal ireturn() { printf("ireturn.normal\n"); return 1; }
+matrix ireturn() { printf("ireturn.matrix\n"); return 1; }
+
+vector creturn() { printf("creturn.vector\n"); return 1; }
+normal creturn() { printf("creturn.normal\n"); return 1; }
+point  creturn() { printf("creturn.point\n"); return 1; }
+color  creturn() { printf("creturn.color\n"); return 1; }
+string creturn() { printf("creturn.string\n"); return ""; }
+matrix creturn() { printf("creturn.matrix\n"); return 1; }
+
+point  vreturn() { printf("vreturn.point\n"); return 1; }
+vector vreturn() { printf("vreturn.vector\n"); return 1; }
+matrix vreturn() { printf("vreturn.matrix\n"); return 1; }
+normal vreturn() { printf("vreturn.normal\n"); return 1; }
+string vreturn() { printf("vreturn.string\n"); return ""; }
+
+string preturn() { printf("preturn.string\n"); return ""; }
+normal preturn() { printf("preturn.normal\n"); return 1; }
+matrix preturn() { printf("preturn.matrix\n"); return 1; }
+point  preturn() { printf("preturn.point\n"); return 1; }
+
+matrix nreturn() { printf("nreturn.matrix\n"); return 1; }
+normal nreturn() { printf("nreturn.normal\n"); return 1; }
+string nreturn() { printf("nreturn.string\n"); return ""; }
+
+matrix mreturn() { printf("mreturn.matrix\n"); return 1; }
+string mreturn() { printf("mreturn.string\n"); return ""; }
+
 shader test ()
 {
   {
@@ -128,5 +170,17 @@ shader test ()
     (color) funcb();
     (float) funcb();
     (int) funcb();
+    printf("\n");
+  }
+
+  {
+    freturn();
+    ireturn();
+    creturn();
+    vreturn();
+    preturn();
+    nreturn();
+    mreturn();
+    printf("\n");
   }
 }
diff --git a/testsuite/oslc-err-funcoverload/ref/out.txt b/testsuite/oslc-err-funcoverload/ref/out.txt
index a6151ea6ce..144c6a4e39 100644
--- a/testsuite/oslc-err-funcoverload/ref/out.txt
+++ b/testsuite/oslc-err-funcoverload/ref/out.txt
@@ -14,9 +14,10 @@ test.osl:14: error: No matching function call to 'funca (int, float)'
   Candidates are:
     test.osl:3	void funca (int, int)
     test.osl:2	void funca (int)
-test.osl:15: error: Ambiguous call to 'funcb'
-  Candidates are:
-    test.osl:7	color funcb ()
+test.osl:15: warning: Ambiguous call to 'funcb ()'
+  Chosen function is:
     test.osl:6	float funcb ()
+  Other candidates are:
     test.osl:5	int funcb ()
+    test.osl:7	color funcb ()
 FAILED test.osl