Merge pull request #647 from steve-lunarg/default-fn-params

Test/baseResults/hlsl.intrinsics.negative.frag.out

 hlsl.intrinsics.negative.frag
 ERROR: 0:10: 'determinant' : no matching overloaded function found 
 ERROR: 0:12: 'f32tof16' : unimplemented intrinsic: handle natively 
+ERROR: 0:23: 'length' : ambiguous best function under implicit type conversion 
+ERROR: 0:25: 'normalize' : ambiguous best function under implicit type conversion 
+ERROR: 0:26: 'reflect' : ambiguous best function under implicit type conversion 
+ERROR: 0:27: 'refract' : ambiguous best function under implicit type conversion 
 ERROR: 0:28: 'refract' : no matching overloaded function found 
 ERROR: 0:30: 'transpose' : no matching overloaded function found 
 ERROR: 0:39: 'GetRenderTargetSamplePosition' : no matching overloaded function found 
@@ -59,7 +63,7 @@ ERROR: 0:133: 'normalize' : no matching overloaded function found
 ERROR: 0:133: 'reflect' : no matching overloaded function found 
 ERROR: 0:133: 'refract' : no matching overloaded function found 
 ERROR: 0:133: 'reversebits' : no matching overloaded function found 
-ERROR: 60 compilation errors.  No code generated.
+ERROR: 64 compilation errors.  No code generated.
 
 
 Shader version: 450

Test/hlsl.params.default.frag

+uniform int4 ui4;
+
+static const int cia = -4;
+static const int cib = -42;
+
+// ERROR: Ambiguous with fn1 below.
+// int4 fn1(int4 p0) { return int4(1,2,3,4); }
+
+int4 fn1(int4 p0, bool b1, bool b2 = false) {
+    return p0;
+}
+
+int4 fn1(int4 p0,
+         int4 p1    : FOO = int4(-1,-2,-3, cia),
+         int  p2[2] : BAR = { int(1), 2 },
+         int  p3          = abs(cib) )
+{
+    return p0 + p1 + p2[0] + p3;
+}
+
+// These should not be ambiguous if given either an int or a float explicit second parameter.
+int4 fn2(int4 p0, int x = 3)
+{
+    return int4(10,11,12,13);
+}
+
+int4 fn2(int4 p0, float x = sin(3.3)) // OK to have a const expression as a default value
+{
+    return p0 + int4(20,21,22,23);
+}
+
+void fn3(int p0 = 3) { }
+
+
+int4 main() : SV_Target0
+{
+    int myarray[2] = {30,31};
+
+    fn3();
+    fn3(5);
+
+    return fn1(100) +
+           fn1(101, ui4) +
+           fn1(102, ui4, myarray) +
+           fn1(103, ui4, myarray, 99) +
+           fn1(104, false) +
+           fn1(105, false, true) +
+
+           fn2(110, 11.11) +             // calls int4, float form
+           fn2(111, 12);                // calls int4, int form
+}

Test/hlsl.params.default.negative.frag

+uniform int4 ui4;
+uniform float ufvar;
+
+static const int cia = -4;
+static const int cib = -42;
+
+int4 fn1(int4 p0) { return int4(1,2,3,4); }
+
+int4 fn1(int4 p0, bool b1, bool b2 = false) {
+    return p0;
+}
+
+int4 fn1(int4 p0,
+         int4 p1 : FOO = int4(-1,-2,-3, cia),
+         int p2[2] : BAR = { int(1), 2 },
+         int p3 = abs(cib) )
+{
+    return p0 + p1 + p2[0] + p3;
+}
+
+// These should not be ambiguous if given either an int or a float explicit second parameter.
+int4 fn2(int4 p0, int x = 3)
+{
+    return int4(10,11,12,13);
+}
+
+int4 fn2(int4 p0, float x = ufvar) // ERROR: non-const expression
+{
+    return p0 + int4(20,21,22,23);
+}
+
+void fn3(int p0 = 5, int p1)  // ERROR no-default param after default param
+{
+}
+
+int4 main() : SV_Target0
+{
+    int myarray[2] = {30,31};
+
+    return fn1(100) +                    // ERROR: ambiguous
+           fn1(101, ui4) +
+           fn1(102, ui4, myarray) +
+           fn1(103, ui4, myarray, 99) +
+           fn1(104, false) +
+           fn1(105, false, true) +
+
+           fn2(112) +                    // ERROR: ambiguous
+           fn2(110, 11.11) +             // calls int4, float form
+           fn2(111, 12);                 // calls int4, int form
+}

glslang/MachineIndependent/Constant.cpp

@@ -629,6 +629,9 @@ TIntermTyped* TIntermConstantUnion::fold(TOperator op, const TType& returnType)
 //
 TIntermTyped* TIntermediate::fold(TIntermAggregate* aggrNode)
 {
+    if (aggrNode == nullptr)
+        return aggrNode;
+
     if (! areAllChildConst(aggrNode))
         return aggrNode;
 

glslang/MachineIndependent/ParseContextBase.cpp

@@ -348,13 +348,18 @@ const TFunction* TParseContextBase::selectFunction(
     for (auto it = candidateList.begin(); it != candidateList.end(); ++it) {
         const TFunction& candidate = *(*it);
 
-        // to even be a potential match, number of arguments has to match
-        if (call.getParamCount() != candidate.getParamCount())
+        // to even be a potential match, number of arguments must be >= the number of
+        // fixed (non-default) parameters, and <= the total (including parameter with defaults).
+        if (call.getParamCount() < candidate.getFixedParamCount() ||
+            call.getParamCount() > candidate.getParamCount())
             continue;
 
         // see if arguments are convertible
         bool viable = true;
-        for (int param = 0; param < candidate.getParamCount(); ++param) {
+
+        // The call can have fewer parameters than the candidate, if some have defaults.
+        const int paramCount = std::min(call.getParamCount(), candidate.getParamCount());
+        for (int param = 0; param < paramCount; ++param) {
             if (candidate[param].type->getQualifier().isParamInput()) {
                 if (! convertible(*call[param].type, *candidate[param].type, candidate.getBuiltInOp(), param)) {
                     viable = false;
@@ -382,7 +387,7 @@ const TFunction* TParseContextBase::selectFunction(
         return viableCandidates.front();
 
     // 4. find best...
-    auto betterParam = [&call, &better](const TFunction& can1, const TFunction& can2) -> bool {
+    const auto betterParam = [&call, &better](const TFunction& can1, const TFunction& can2) -> bool {
         // is call -> can2 better than call -> can1 for any parameter
         bool hasBetterParam = false;
         for (int param = 0; param < call.getParamCount(); ++param) {
@@ -394,6 +399,16 @@ const TFunction* TParseContextBase::selectFunction(
         return hasBetterParam;
     };
 
+    const auto equivalentParams = [&call, &better](const TFunction& can1, const TFunction& can2) -> bool {
+        // is call -> can2 equivalent to call -> can1 for all the call parameters?
+        for (int param = 0; param < call.getParamCount(); ++param) {
+            if (better(*call[param].type, *can1[param].type, *can2[param].type) ||
+                better(*call[param].type, *can2[param].type, *can1[param].type))
+                return false;
+        }
+        return true;
+    };
+    
     const TFunction* incumbent = viableCandidates.front();
     for (auto it = viableCandidates.begin() + 1; it != viableCandidates.end(); ++it) {
         const TFunction& candidate = *(*it);
@@ -406,7 +421,10 @@ const TFunction* TParseContextBase::selectFunction(
         if (incumbent == *it)
             continue;
         const TFunction& candidate = *(*it);
-        if (betterParam(*incumbent, candidate))
+        
+        // In the case of default parameters, it may have an identical initial set, which is
+        // also ambiguous
+        if (betterParam(*incumbent, candidate) || equivalentParams(*incumbent, candidate))
             tie = true;
     }
 

glslang/MachineIndependent/SymbolTable.cpp

@@ -295,6 +295,7 @@ TFunction::TFunction(const TFunction& copyOf) : TSymbol(copyOf)
     op = copyOf.op;
     defined = copyOf.defined;
     prototyped = copyOf.prototyped;
+    defaultParamCount = copyOf.defaultParamCount;
 }
 
 TFunction* TFunction::clone() const

glslang/MachineIndependent/SymbolTable.h

@@ -191,6 +191,7 @@ protected:
 struct TParameter {
     TString *name;
     TType* type;
+    TIntermTyped* defaultValue;
     void copyParam(const TParameter& param) 
     {
         if (param.name)
@@ -198,6 +199,7 @@ struct TParameter {
         else
             name = 0;
         type = param.type->clone();
+        defaultValue = param.defaultValue;
     }
 };
 
@@ -209,12 +211,12 @@ public:
     explicit TFunction(TOperator o) :
         TSymbol(0),
         op(o),
-        defined(false), prototyped(false) { }
+        defined(false), prototyped(false), defaultParamCount(0) { }
     TFunction(const TString *name, const TType& retType, TOperator tOp = EOpNull) :
         TSymbol(name),
         mangledName(*name + '('),
         op(tOp),
-        defined(false), prototyped(false) { returnType.shallowCopy(retType); }
+        defined(false), prototyped(false), defaultParamCount(0) { returnType.shallowCopy(retType); }
     virtual TFunction* clone() const;
     virtual ~TFunction();
 
@@ -226,6 +228,9 @@ public:
         assert(writable);
         parameters.push_back(p);
         p.type->appendMangledName(mangledName);
+
+        if (p.defaultValue != nullptr)
+            defaultParamCount++;
     }
 
     virtual const TString& getMangledName() const { return mangledName; }
@@ -238,7 +243,13 @@ public:
     virtual void setPrototyped() { assert(writable); prototyped = true; }
     virtual bool isPrototyped() const { return prototyped; }
 
+    // Return total number of parameters
     virtual int getParamCount() const { return static_cast<int>(parameters.size()); }
+    // Return number of parameters with default values.
+    virtual int getDefaultParamCount() const { return defaultParamCount; }
+    // Return number of fixed parameters (without default values)
+    virtual int getFixedParamCount() const { return getParamCount() - getDefaultParamCount(); }
+
     virtual TParameter& operator[](int i) { assert(writable); return parameters[i]; }
     virtual const TParameter& operator[](int i) const { return parameters[i]; }
 
@@ -255,6 +266,7 @@ protected:
     TOperator op;
     bool defined;
     bool prototyped;
+    int  defaultParamCount;
 };
 
 //

gtests/Hlsl.FromFile.cpp

@@ -160,6 +160,8 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.numericsuffixes.frag", "main"},
         {"hlsl.numthreads.comp", "main_aux1"},
         {"hlsl.overload.frag", "PixelShaderFunction"},
+        {"hlsl.params.default.frag", "main"},
+        {"hlsl.params.default.negative.frag", "main"},
         {"hlsl.partialInit.frag", "PixelShaderFunction"},
         {"hlsl.pp.line.frag", "main"},
         {"hlsl.precise.frag", "main"},

hlsl/hlslGrammar.cpp

@@ -1776,9 +1776,55 @@ bool HlslGrammar::acceptFunctionParameters(TFunction& function)
     return true;
 }
 
+
+// default_parameter_declaration
+//      : EQUAL conditional_expression
+//      : EQUAL initializer
+bool HlslGrammar::acceptDefaultParameterDeclaration(const TType& type, TIntermTyped*& node)
+{
+    node = nullptr;
+
+    // Valid not to have a default_parameter_declaration
+    if (!acceptTokenClass(EHTokAssign))
+        return true;
+
+    if (!acceptConditionalExpression(node)) {
+        if (!acceptInitializer(node))
+            return false;
+
+        // For initializer lists, we have to const-fold into a constructor for the type, so build
+        // that.
+        TFunction* constructor = parseContext.handleConstructorCall(token.loc, type);
+        if (constructor == nullptr)  // cannot construct
+            return false;
+
+        TIntermTyped* arguments = nullptr;
+        for (int i=0; i<int(node->getAsAggregate()->getSequence().size()); i++)
+            parseContext.handleFunctionArgument(constructor, arguments, node->getAsAggregate()->getSequence()[i]->getAsTyped());
+        
+        node = parseContext.handleFunctionCall(token.loc, constructor, node);
+    }
+
+    // If this is simply a constant, we can use it directly.
+    if (node->getAsConstantUnion())
+        return true;
+
+    // Otherwise, it has to be const-foldable.
+    TIntermTyped* origNode = node;
+
+    node = intermediate.fold(node->getAsAggregate());
+
+    if (node != nullptr && origNode != node)
+        return true;
+
+    parseContext.error(token.loc, "invalid default parameter value", "", "");
+
+    return false;
+}
+
 // parameter_declaration
-//      : fully_specified_type post_decls
-//      | fully_specified_type identifier array_specifier post_decls
+//      : fully_specified_type post_decls [ = default_parameter_declaration ]
+//      | fully_specified_type identifier array_specifier post_decls [ = default_parameter_declaration ]
 //
 bool HlslGrammar::acceptParameterDeclaration(TFunction& function)
 {
@@ -1806,9 +1852,19 @@ bool HlslGrammar::acceptParameterDeclaration(TFunction& function)
     // post_decls
     acceptPostDecls(type->getQualifier());
 
+    TIntermTyped* defaultValue;
+    if (!acceptDefaultParameterDeclaration(*type, defaultValue))
+        return false;
+
     parseContext.paramFix(*type);
 
-    TParameter param = { idToken.string, type };
+    // If any prior parameters have default values, all the parameters after that must as well.
+    if (defaultValue == nullptr && function.getDefaultParamCount() > 0) {
+        parseContext.error(idToken.loc, "invalid parameter after default value parameters", idToken.string->c_str(), "");
+        return false;
+    }
+
+    TParameter param = { idToken.string, type, defaultValue };
     function.addParameter(param);
 
     return true;

hlsl/hlslGrammar.h

@@ -111,6 +111,7 @@ namespace glslang {
         bool acceptDefaultLabel(TIntermNode*&);
         void acceptArraySpecifier(TArraySizes*&);
         void acceptPostDecls(TQualifier&);
+        bool acceptDefaultParameterDeclaration(const TType&, TIntermTyped*&);
 
         HlslParseContext& parseContext;  // state of parsing and helper functions for building the intermediate
         TIntermediate& intermediate;     // the final product, the intermediate representation, includes the AST

hlsl/hlslParseHelper.cpp

@@ -1375,7 +1375,7 @@ TIntermNode* HlslParseContext::handleReturnValue(const TSourceLoc& loc, TIntermT
 
 void HlslParseContext::handleFunctionArgument(TFunction* function, TIntermTyped*& arguments, TIntermTyped* newArg)
 {
-    TParameter param = { 0, new TType };
+    TParameter param = { 0, new TType, nullptr };
     param.type->shallowCopy(newArg->getType());
     function->addParameter(param);
     if (arguments)
@@ -2643,7 +2643,7 @@ void HlslParseContext::decomposeIntrinsic(const TSourceLoc& loc, TIntermTyped*&
 //  - user function
 //  - subroutine call (not implemented yet)
 //
-TIntermTyped* HlslParseContext::handleFunctionCall(const TSourceLoc& loc, TFunction* function, TIntermNode* arguments)
+TIntermTyped* HlslParseContext::handleFunctionCall(const TSourceLoc& loc, TFunction* function, TIntermTyped* arguments)
 {
     TIntermTyped* result = nullptr;
 
@@ -2783,10 +2783,10 @@ TIntermTyped* HlslParseContext::handleLengthMethod(const TSourceLoc& loc, TFunct
 //
 // Add any needed implicit conversions for function-call arguments to input parameters.
 //
-void HlslParseContext::addInputArgumentConversions(const TFunction& function, TIntermNode*& arguments)
+void HlslParseContext::addInputArgumentConversions(const TFunction& function, TIntermTyped*& arguments)
 {
     TIntermAggregate* aggregate = arguments->getAsAggregate();
-    const auto setArg = [&](int argNum, TIntermNode* arg) {
+    const auto setArg = [&](int argNum, TIntermTyped* arg) {
         if (function.getParamCount() == 1)
             arguments = arg;
         else {
@@ -4500,8 +4500,8 @@ void HlslParseContext::mergeObjectLayoutQualifiers(TQualifier& dst, const TQuali
 //
 // Return the function symbol if found, otherwise nullptr.
 //
-const TFunction* HlslParseContext::findFunction(const TSourceLoc& loc, const TFunction& call, bool& builtIn,
-                                                TIntermNode* args)
+const TFunction* HlslParseContext::findFunction(const TSourceLoc& loc, TFunction& call, bool& builtIn,
+                                                TIntermTyped*& args)
 {
     // const TFunction* function = nullptr;
 
@@ -4600,6 +4600,22 @@ const TFunction* HlslParseContext::findFunction(const TSourceLoc& loc, const TFu
                 return false;
         }
 
+        // Handle sampler betterness: An exact sampler match beats a non-exact match.
+        // (If we just looked at basic type, all EbtSamplers would look the same).
+        // If any type is not a sampler, just use the linearize function below.
+        if (from.getBasicType() == EbtSampler && to1.getBasicType() == EbtSampler && to2.getBasicType() == EbtSampler) {
+            // We can ignore the vector size in the comparison.
+            TSampler to1Sampler = to1.getSampler();
+            TSampler to2Sampler = to2.getSampler();
+
+            to1Sampler.vectorSize = to2Sampler.vectorSize = from.getSampler().vectorSize;
+
+            if (from.getSampler() == to2Sampler)
+                return from.getSampler() != to1Sampler;
+            if (from.getSampler() == to1Sampler)
+                return false;
+        }
+
         // Might or might not be changing shape, which means basic type might
         // or might not match, so within that, the question is how big a
         // basic-type conversion is being done.
@@ -4689,18 +4705,18 @@ const TFunction* HlslParseContext::findFunction(const TSourceLoc& loc, const TFu
             // Handle aggregates: put all args into the new function call
             for (int arg=0; arg<int(args->getAsAggregate()->getSequence().size()); ++arg) {
                 // TODO: But for constness, we could avoid the new & shallowCopy, and use the pointer directly.
-                TParameter param = { 0, new TType };
+                TParameter param = { 0, new TType, nullptr };
                 param.type->shallowCopy(args->getAsAggregate()->getSequence()[arg]->getAsTyped()->getType());
                 convertedCall.addParameter(param);
             }
         } else if (args->getAsUnaryNode()) {
             // Handle unaries: put all args into the new function call
-            TParameter param = { 0, new TType };
+            TParameter param = { 0, new TType, nullptr };
             param.type->shallowCopy(args->getAsUnaryNode()->getOperand()->getAsTyped()->getType());
             convertedCall.addParameter(param);
         } else if (args->getAsTyped()) {
             // Handle bare e.g, floats, not in an aggregate.
-            TParameter param = { 0, new TType };
+            TParameter param = { 0, new TType, nullptr };
             param.type->shallowCopy(args->getAsTyped()->getType());
             convertedCall.addParameter(param);
         } else {
@@ -4718,6 +4734,13 @@ const TFunction* HlslParseContext::findFunction(const TSourceLoc& loc, const TFu
     if (tie)
         error(loc, "ambiguous best function under implicit type conversion", call.getName().c_str(), "");
 
+    // Append default parameter values if needed
+    if (!tie && bestMatch != nullptr) {
+        for (int defParam = call.getParamCount(); defParam < bestMatch->getParamCount(); ++defParam) {
+            handleFunctionArgument(&call, args, (*bestMatch)[defParam].defaultValue);
+        }
+    }
+
     return bestMatch;
 }
 

hlsl/hlslParseHelper.h

@@ -79,12 +79,12 @@ public:
     TIntermNode* handleReturnValue(const TSourceLoc&, TIntermTyped*);
     void handleFunctionArgument(TFunction*, TIntermTyped*& arguments, TIntermTyped* newArg);
     TIntermTyped* handleAssign(const TSourceLoc&, TOperator, TIntermTyped* left, TIntermTyped* right) const;
-    TIntermTyped* handleFunctionCall(const TSourceLoc&, TFunction*, TIntermNode*);
+    TIntermTyped* handleFunctionCall(const TSourceLoc&, TFunction*, TIntermTyped*);
     void decomposeIntrinsic(const TSourceLoc&, TIntermTyped*& node, TIntermNode* arguments);
     void decomposeSampleMethods(const TSourceLoc&, TIntermTyped*& node, TIntermNode* arguments);
     void decomposeGeometryMethods(const TSourceLoc&, TIntermTyped*& node, TIntermNode* arguments);
     TIntermTyped* handleLengthMethod(const TSourceLoc&, TFunction*, TIntermNode*);
-    void addInputArgumentConversions(const TFunction&, TIntermNode*&);
+    void addInputArgumentConversions(const TFunction&, TIntermTyped*&);
     TIntermTyped* addOutputArgumentConversions(const TFunction&, TIntermOperator&);
     void builtInOpCheck(const TSourceLoc&, const TFunction&, TIntermOperator&);
     TFunction* handleConstructorCall(const TSourceLoc&, const TType&);
@@ -126,7 +126,7 @@ public:
     void mergeObjectLayoutQualifiers(TQualifier& dest, const TQualifier& src, bool inheritOnly);
     void checkNoShaderLayouts(const TSourceLoc&, const TShaderQualifiers&);
 
-    const TFunction* findFunction(const TSourceLoc& loc, const TFunction& call, bool& builtIn, TIntermNode* args);
+    const TFunction* findFunction(const TSourceLoc& loc, TFunction& call, bool& builtIn, TIntermTyped*& args);
     void declareTypedef(const TSourceLoc&, TString& identifier, const TType&, TArraySizes* typeArray = 0);
     TIntermNode* declareVariable(const TSourceLoc&, TString& identifier, TType&, TIntermTyped* initializer = 0);
     void lengthenList(const TSourceLoc&, TIntermSequence& list, int size);

hlsl/hlslParseables.cpp

@@ -586,7 +586,7 @@ void TBuiltInParseablesHlsl::initialize(int /*version*/, EProfile /*profile*/, c
         { "DeviceMemoryBarrier",              nullptr, nullptr,   "-",              "-",             EShLangPSCS },
         { "DeviceMemoryBarrierWithGroupSync", nullptr, nullptr,   "-",              "-",             EShLangCS },
         { "distance",                         "S",     "F",       "V,",             "F,",            EShLangAll },
-        { "dot",                              "S",     nullptr,   "V,",             "FI,",           EShLangAll },
+        { "dot",                              "S",     nullptr,   "SV,",            "FI,",           EShLangAll },
         { "dst",                              nullptr, nullptr,   "V4,",            "F,",            EShLangAll },
         // { "errorf",                           "-",     "-",       "",             "",             EShLangAll }, TODO: varargs
         { "EvaluateAttributeAtCentroid",      nullptr, nullptr,   "SVM",            "F",             EShLangPS },