From d25d857487ce3d4b9259d991647e88fb924d8311 Mon Sep 17 00:00:00 2001 From: Frederich Munch Date: Thu, 8 Feb 2018 11:28:13 -0500 Subject: [PATCH] Fix remaining undefined behavior from functions ambiguous by return type. Adds color closures, structs, and void to the defined precedence order of resolving via return type. Errors when disambiguating via return type and more than one user struct is a match. --- src/liboslcomp/typecheck.cpp | 103 ++++++++++----- testsuite/function-overloads/ref/out.txt | 132 +++++++++++++------- testsuite/function-overloads/test.osl | 42 +++++++ testsuite/oslc-err-funcoverload/ref/out.txt | 14 ++- testsuite/oslc-err-funcoverload/test.osl | 8 ++ 5 files changed, 221 insertions(+), 78 deletions(-) diff --git a/src/liboslcomp/typecheck.cpp b/src/liboslcomp/typecheck.cpp index ed6d1a0e0a..661e7befb9 100644 --- a/src/liboslcomp/typecheck.cpp +++ b/src/liboslcomp/typecheck.cpp @@ -1187,10 +1187,14 @@ ASTfunction_call::typecheck_struct_constructor () /// If there is a tie (and only then), the return type is considered in the score. /// 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 +/// float, int, color, vector, point, normal, matrix, string, color closure, +/// struct, void /// A warning is shown, printing which was function chosen and the list of all /// that were considered ambiguous. /// +/// If two or more overloads differ only by return types that are both structs, +/// then the warning above is treated as an error. +/// /// Float to int coercion is scored, but is currently a synmonym for kNoMatch /// as the spec does not allow implicit float to int conversion. /// @@ -1426,6 +1430,8 @@ class CandidateFunctions { std::pair best(ASTNode* caller, const ustring& funcname) { + ASSERT (caller); // Assertion that passed ASTNode::ref was not empty + switch (m_candidates.size()) { case 0: // Nothing at all, Error @@ -1460,7 +1466,45 @@ class CandidateFunctions { ASSERT (c.first && c.first->sym); if (ambiguity != -1) { - ASSERT (caller); + + unsigned userstructs = 0; + + auto rank = [&userstructs] (const TypeSpec& s) -> int { + // Arrays are currently not ranked as they cannot be returned. + ASSERT (!s.is_array()); + ASSERT (!s.is_closure() || s.is_color_closure()); + + const TypeDesc& td = s.simpletype(); + if (td == TypeDesc::TypeFloat) + return 0; + if (td == TypeDesc::TypeInt) + return 1; + if (td == TypeDesc::TypeColor) + return 2; + if (td == TypeDesc::TypeVector) + return 3; + if (td == TypeDesc::TypePoint) + return 4; + if (td == TypeDesc::TypeNormal) + return 5; + if (td == TypeDesc::TypeMatrix) + return 6; + if (td == TypeDesc::TypeString) + return 7; + + if (s.is_color_closure()) + return 8; + if (s.is_structure_based()) { + ++userstructs; + return 9; + } + if (s.is_void()) + return 10; + + ASSERT (0 && "Unranked type"); + return std::numeric_limits::max(); + }; + if (true /*m_rval.simpletype().is_unknown()*/) { // Ambiguity because the return type desired is unknown // float noise(point p) @@ -1473,28 +1517,8 @@ class CandidateFunctions { // 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; - return std::numeric_limits::max(); - }; - return rank(a.rtype.simpletype()) - < rank(b.rtype.simpletype()); + [rank](const Candidate& a, const Candidate& b) -> bool { + return rank(a.rtype) < rank(b.rtype); }); // New choice is now front of the list @@ -1502,14 +1526,31 @@ class CandidateFunctions { m_candidates.front().rscore); } - reportAmbiguity(caller, funcname, false, "Ambiguous call to"); - - candidateHeader("Chosen function is"); - reportFunction(c.first->sym); + if (userstructs) { + // std::sort can call 'rank' multiple times, and we can't use an + // address to store the actual count (as we are sorting!). + userstructs = 0; + for (Candidates::const_reverse_iterator i = m_candidates.rbegin(), + e = m_candidates.rend(); i != e; ++i) { + if (i->rtype.is_structure_based() && ++userstructs > 1) + break; + } + } - candidateHeader("Other candidates are"); - for (auto& candidate : m_candidates) { - if (candidate.sym != c.first->sym) + bool warn = userstructs < 2; + reportAmbiguity(caller, funcname, !warn /* "Candidates are" msg*/, + "Ambiguous call to", warn /* As warning */); + if (warn) { + candidateHeader("Chosen function is"); + reportFunction(c.first->sym); + + candidateHeader("Other candidates are"); + for (auto& candidate : m_candidates) { + if (candidate.sym != c.first->sym) + reportFunction(candidate.sym); + } + } else { + for (auto& candidate : m_candidates) reportFunction(candidate.sym); } } diff --git a/testsuite/function-overloads/ref/out.txt b/testsuite/function-overloads/ref/out.txt index 6777c47cb3..90fc7c3b17 100644 --- a/testsuite/function-overloads/ref/out.txt +++ b/testsuite/function-overloads/ref/out.txt @@ -1,59 +1,103 @@ -test.osl:177: warning: Ambiguous call to 'freturn ()' +test.osl:215: warning: Ambiguous call to 'freturn ()' Chosen function is: - test.osl:71 float freturn () + test.osl:73 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 ()' + test.osl:70 int freturn () + test.osl:74 color freturn () + test.osl:76 vector freturn () + test.osl:71 point freturn () + test.osl:77 normal freturn () + test.osl:78 matrix freturn () + test.osl:72 string freturn () + test.osl:75 closure color freturn () + test.osl:80 struct A freturn () + test.osl:79 void freturn () +test.osl:216: warning: Ambiguous call to 'ireturn ()' Chosen function is: - test.osl:80 int ireturn () + test.osl:87 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 ()' + test.osl:84 color ireturn () + test.osl:88 vector ireturn () + test.osl:85 point ireturn () + test.osl:89 normal ireturn () + test.osl:91 matrix ireturn () + test.osl:86 string ireturn () + test.osl:83 closure color ireturn () + test.osl:82 struct A ireturn () + test.osl:90 void ireturn () +test.osl:217: warning: Ambiguous call to 'creturn ()' Chosen function is: - test.osl:88 color creturn () + test.osl:98 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 ()' + test.osl:93 vector creturn () + test.osl:97 point creturn () + test.osl:94 normal creturn () + test.osl:100 matrix creturn () + test.osl:99 string creturn () + test.osl:101 closure color creturn () + test.osl:96 struct A creturn () + test.osl:95 void creturn () +test.osl:218: warning: Ambiguous call to 'vreturn ()' Chosen function is: - test.osl:93 vector vreturn () + test.osl:105 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 ()' + test.osl:103 point vreturn () + test.osl:109 normal vreturn () + test.osl:106 matrix vreturn () + test.osl:110 string vreturn () + test.osl:104 closure color vreturn () + test.osl:107 struct A vreturn () + test.osl:108 void vreturn () +test.osl:219: warning: Ambiguous call to 'preturn ()' Chosen function is: - test.osl:101 point preturn () + test.osl:118 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 ()' + test.osl:114 normal preturn () + test.osl:116 matrix preturn () + test.osl:112 string preturn () + test.osl:115 closure color preturn () + test.osl:117 struct A preturn () + test.osl:113 void preturn () +test.osl:220: warning: Ambiguous call to 'nreturn ()' Chosen function is: - test.osl:104 normal nreturn () + test.osl:122 normal nreturn () Other candidates are: - test.osl:103 matrix nreturn () - test.osl:105 string nreturn () -test.osl:183: warning: Ambiguous call to 'mreturn ()' + test.osl:120 matrix nreturn () + test.osl:123 string nreturn () + test.osl:124 closure color nreturn () + test.osl:121 struct A nreturn () + test.osl:125 void nreturn () +test.osl:221: warning: Ambiguous call to 'mreturn ()' Chosen function is: - test.osl:107 matrix mreturn () + test.osl:130 matrix mreturn () Other candidates are: - test.osl:108 string mreturn () + test.osl:131 string mreturn () + test.osl:128 closure color mreturn () + test.osl:129 struct A mreturn () + test.osl:127 void mreturn () +test.osl:222: warning: Ambiguous call to 'strreturn ()' + Chosen function is: + test.osl:136 string strreturn () + Other candidates are: + test.osl:134 closure color strreturn () + test.osl:133 struct A strreturn () + test.osl:135 void strreturn () +test.osl:223: warning: Ambiguous call to 'ccreturn ()' + Chosen function is: + test.osl:138 closure color ccreturn () + Other candidates are: + test.osl:140 struct A ccreturn () + test.osl:139 void ccreturn () +test.osl:224: warning: Ambiguous call to 'structreturn ()' + Chosen function is: + test.osl:142 struct A structreturn () + Other candidates are: + test.osl:143 void structreturn () +test.osl:225: warning: Ambiguous call to 'structreturn1 ()' + Chosen function is: + test.osl:146 struct A structreturn1 () + Other candidates are: + test.osl:145 void structreturn1 () Compiled test.osl -> test.oso testA int testB int @@ -111,5 +155,9 @@ vreturn.vector preturn.point nreturn.normal mreturn.matrix +strreturn.string +ccreturn.ccolor +structreturn.struct +structreturn1.struct diff --git a/testsuite/function-overloads/test.osl b/testsuite/function-overloads/test.osl index 2100ba0b0c..aa7d150fa9 100644 --- a/testsuite/function-overloads/test.osl +++ b/testsuite/function-overloads/test.osl @@ -65,48 +65,86 @@ int funcb() { printf("funcb.int\n"); return 1; } float funcb() { printf("funcb.float\n"); return 2; } color funcb() { printf("funcb.color\n"); return 3; } +struct A { float a; }; + 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; } +closure color freturn() { printf("freturn.ccolor\n"); return diffuse(N); } vector freturn() { printf("freturn.vector\n"); return 1; } normal freturn() { printf("freturn.normal\n"); return 1; } matrix freturn() { printf("freturn.matrix\n"); return 1; } +void freturn() { printf("freturn.void\n"); } +A freturn() { printf("freturn.struct\n"); A a; return a; } +A ireturn() { printf("ireturn.struct\n"); A a; return a; } +closure color ireturn() { printf("ireturn.ccolor\n"); return diffuse(N); } 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; } +void ireturn() { printf("ireturn.void\n"); } matrix ireturn() { printf("ireturn.matrix\n"); return 1; } vector creturn() { printf("creturn.vector\n"); return 1; } normal creturn() { printf("creturn.normal\n"); return 1; } +void creturn() { printf("creturn.void\n"); } +A creturn() { printf("creturn.struct\n"); A a; return a; } 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; } +closure color creturn() { printf("creturn.ccolor\n"); return diffuse(N); } point vreturn() { printf("vreturn.point\n"); return 1; } +closure color vreturn() { printf("vreturn.ccolor\n"); return diffuse(N); } vector vreturn() { printf("vreturn.vector\n"); return 1; } matrix vreturn() { printf("vreturn.matrix\n"); return 1; } +A vreturn() { printf("vreturn.struct\n"); A a; return a; } +void vreturn() { printf("vreturn.void\n"); } normal vreturn() { printf("vreturn.normal\n"); return 1; } string vreturn() { printf("vreturn.string\n"); return ""; } string preturn() { printf("preturn.string\n"); return ""; } +void preturn() { printf("preturn.void\n"); } normal preturn() { printf("preturn.normal\n"); return 1; } +closure color preturn() { printf("preturn.ccolor\n"); return diffuse(N); } matrix preturn() { printf("preturn.matrix\n"); return 1; } +A preturn() { printf("preturn.struct\n"); A a; return a; } point preturn() { printf("preturn.point\n"); return 1; } matrix nreturn() { printf("nreturn.matrix\n"); return 1; } +A nreturn() { printf("nreturn.struct\n"); A a; return a; } normal nreturn() { printf("nreturn.normal\n"); return 1; } string nreturn() { printf("nreturn.string\n"); return ""; } +closure color nreturn() { printf("nreturn.ccolor\n"); return diffuse(N); } +void nreturn() { printf("nreturn.void\n"); } +void mreturn() { printf("mreturn.void\n"); } +closure color mreturn() { printf("mreturn.ccolor\n"); return diffuse(N); } +A mreturn() { printf("mreturn.struct\n"); A a; return a; } matrix mreturn() { printf("mreturn.matrix\n"); return 1; } string mreturn() { printf("mreturn.string\n"); return ""; } +A strreturn() { printf("strreturn.struct\n"); A a; return a; } +closure color strreturn() { printf("strreturn.ccolor\n"); return diffuse(N); } +void strreturn() { printf("strreturn.void\n"); } +string strreturn() { printf("strreturn.string\n"); return ""; } + +closure color ccreturn() { printf("ccreturn.ccolor\n"); return diffuse(N); } +void ccreturn() { printf("ccreturn.void\n"); } +A ccreturn() { printf("ccreturn.struct\n"); A a; return a; } + +A structreturn() { printf("structreturn.struct\n"); A a; return a; } +void structreturn() { printf("structreturn.void\n"); } + +void structreturn1() { printf("structreturn1.void\n"); } +A structreturn1() { printf("structreturn1.struct\n"); A a; return a; } + shader test () { { @@ -181,6 +219,10 @@ shader test () preturn(); nreturn(); mreturn(); + strreturn(); + ccreturn(); + structreturn(); + structreturn1(); printf("\n"); } } diff --git a/testsuite/oslc-err-funcoverload/ref/out.txt b/testsuite/oslc-err-funcoverload/ref/out.txt index 144c6a4e39..1358e99268 100644 --- a/testsuite/oslc-err-funcoverload/ref/out.txt +++ b/testsuite/oslc-err-funcoverload/ref/out.txt @@ -1,23 +1,27 @@ -test.osl:11: error: No matching function call to 'funca ()' +test.osl:17: error: No matching function call to 'funca ()' Candidates are: test.osl:3 void funca (int, int) test.osl:2 void funca (int) -test.osl:12: error: No matching function call to 'funca (int, int, int)' +test.osl:18: 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:13: error: No matching function call to 'funca (float, int)' +test.osl:19: 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:14: error: No matching function call to 'funca (int, float)' +test.osl:20: 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: warning: Ambiguous call to 'funcb ()' +test.osl:21: 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 () +test.osl:25: error: Ambiguous call to 'ambig_struct_return ()' + Candidates are: + test.osl:13 struct B ambig_struct_return () + test.osl:12 struct A ambig_struct_return () FAILED test.osl diff --git a/testsuite/oslc-err-funcoverload/test.osl b/testsuite/oslc-err-funcoverload/test.osl index acc5a63082..2f67746733 100644 --- a/testsuite/oslc-err-funcoverload/test.osl +++ b/testsuite/oslc-err-funcoverload/test.osl @@ -6,6 +6,12 @@ int funcb() { return 1; } float funcb() { return 2; } color funcb() { return 3; } +struct A { float a; }; +struct B { float b; }; + +A ambig_struct_return() { A a; return a; } +B ambig_struct_return() { B b; return b; } + shader test() { funca(); // fail too few arguments @@ -15,5 +21,7 @@ shader test() funcb(); // fail unresolvable ambiguity (int) funcb(); // ok + + ambig_struct_return(); }