Fix variable vs. function name conflict in HLSL output

src/compiler/translator/OutputHLSL.cpp

@@ -783,8 +783,7 @@ void OutputHLSL::visitSymbol(TIntermSymbol *node)
 
             ensureStructDefined(nodeType);
 
-            const TName &nameWithMetadata = node->getName();
-            out << DecorateUniform(nameWithMetadata, nodeType);
+            out << DecorateVariableIfNeeded(node->getName());
         }
         else if (qualifier == EvqAttribute || qualifier == EvqVertexIn)
         {
@@ -873,7 +872,7 @@ void OutputHLSL::visitSymbol(TIntermSymbol *node)
         }
         else
         {
-            out << DecorateIfNeeded(node->getName());
+            out << DecorateVariableIfNeeded(node->getName());
         }
     }
 }
@@ -1593,7 +1592,7 @@ bool OutputHLSL::visitFunctionDefinition(Visit visit, TIntermFunctionDefinition
     }
     else
     {
-        out << DecorateIfNeeded(node->getFunctionSymbolInfo()->getNameObj())
+        out << DecorateFunctionIfNeeded(node->getFunctionSymbolInfo()->getNameObj())
             << DisambiguateFunctionName(parameters) << (mOutputLod0Function ? "Lod0(" : "(");
     }
 
@@ -1725,7 +1724,7 @@ bool OutputHLSL::visitFunctionPrototype(Visit visit, TIntermFunctionPrototype *n
 
     TIntermSequence *arguments = node->getSequence();
 
-    TString name = DecorateIfNeeded(node->getFunctionSymbolInfo()->getNameObj());
+    TString name = DecorateFunctionIfNeeded(node->getFunctionSymbolInfo()->getNameObj());
     out << TypeString(node->getType()) << " " << name << DisambiguateFunctionName(arguments)
         << (mOutputLod0Function ? "Lod0(" : "(");
 
@@ -1779,7 +1778,7 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
                 ASSERT(index != CallDAG::InvalidIndex);
                 lod0 &= mASTMetadataList[index].mNeedsLod0;
 
-                out << DecorateIfNeeded(node->getFunctionSymbolInfo()->getNameObj());
+                out << DecorateFunctionIfNeeded(node->getFunctionSymbolInfo()->getNameObj());
                 out << DisambiguateFunctionName(node->getSequence());
                 out << (lod0 ? "Lod0(" : "(");
             }
@@ -1787,7 +1786,7 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
             {
                 // This path is used for internal functions that don't have their definitions in the
                 // AST, such as precision emulation functions.
-                out << DecorateIfNeeded(node->getFunctionSymbolInfo()->getNameObj()) << "(";
+                out << DecorateFunctionIfNeeded(node->getFunctionSymbolInfo()->getNameObj()) << "(";
             }
             else
             {
@@ -2537,7 +2536,7 @@ TString OutputHLSL::argumentString(const TIntermSymbol *symbol)
     }
     else
     {
-        nameStr = DecorateIfNeeded(name);
+        nameStr = DecorateVariableIfNeeded(name);
     }
 
     if (IsSampler(type.getBasicType()))

src/compiler/translator/UniformHLSL.cpp

@@ -178,8 +178,8 @@ void UniformHLSL::outputHLSLSamplerUniformGroup(
 
         if (type.isArray())
         {
-            out << "static const uint " << DecorateIfNeeded(uniform->getName()) << ArrayString(type)
-                << " = {";
+            out << "static const uint " << DecorateVariableIfNeeded(uniform->getName())
+                << ArrayString(type) << " = {";
             for (unsigned int i = 0u; i < type.getArraySize(); ++i)
             {
                 if (i > 0u)
@@ -190,7 +190,7 @@ void UniformHLSL::outputHLSLSamplerUniformGroup(
         }
         else
         {
-            out << "static const uint " << DecorateIfNeeded(uniform->getName()) << " = "
+            out << "static const uint " << DecorateVariableIfNeeded(uniform->getName()) << " = "
                 << samplerArrayIndex << ";\n";
         }
     }
@@ -218,11 +218,11 @@ void UniformHLSL::outputHLSL4_0_FL9_3Sampler(TInfoSinkBase &out,
                                              const unsigned int registerIndex)
 {
     out << "uniform " << SamplerString(type.getBasicType()) << " sampler_"
-        << DecorateUniform(name, type) << ArrayString(type) << " : register(s" << str(registerIndex)
-        << ");\n";
+        << DecorateVariableIfNeeded(name) << ArrayString(type) << " : register(s"
+        << str(registerIndex) << ");\n";
     out << "uniform " << TextureString(type.getBasicType()) << " texture_"
-        << DecorateUniform(name, type) << ArrayString(type) << " : register(t" << str(registerIndex)
-        << ");\n";
+        << DecorateVariableIfNeeded(name) << ArrayString(type) << " : register(t"
+        << str(registerIndex) << ");\n";
 }
 
 void UniformHLSL::outputUniform(TInfoSinkBase &out,
@@ -245,7 +245,7 @@ void UniformHLSL::outputUniform(TInfoSinkBase &out,
 
     out << "uniform " << typeName << " ";
 
-    out << DecorateUniform(name, type);
+    out << DecorateVariableIfNeeded(name);
 
     out << ArrayString(type) << " : " << registerString << ";\n";
 }

src/compiler/translator/UtilsHLSL.cpp

@@ -199,11 +199,6 @@ TString TextureTypeSuffix(const TBasicType type)
     }
 }
 
-TString DecorateUniform(const TName &name, const TType &type)
-{
-    return DecorateIfNeeded(name);
-}
-
 TString DecorateField(const TString &string, const TStructure &structure)
 {
     if (structure.name().compare(0, 3, "gl_") != 0)
@@ -229,10 +224,13 @@ TString Decorate(const TString &string)
     return string;
 }
 
-TString DecorateIfNeeded(const TName &name)
+TString DecorateVariableIfNeeded(const TName &name)
 {
     if (name.isInternal())
     {
+        // The name should not have a prefix reserved for user-defined variables or functions.
+        ASSERT(name.getString().compare(0, 2, "f_") != 0);
+        ASSERT(name.getString().compare(0, 1, "_") != 0);
         return name.getString();
     }
     else
@@ -241,6 +239,22 @@ TString DecorateIfNeeded(const TName &name)
     }
 }
 
+TString DecorateFunctionIfNeeded(const TName &name)
+{
+    if (name.isInternal())
+    {
+        // The name should not have a prefix reserved for user-defined variables or functions.
+        ASSERT(name.getString().compare(0, 2, "f_") != 0);
+        ASSERT(name.getString().compare(0, 1, "_") != 0);
+        return name.getString();
+    }
+    ASSERT(name.getString().compare(0, 3, "gl_") != 0);
+    // Add an additional f prefix to functions so that they're always disambiguated from variables.
+    // This is necessary in the corner case where a variable declaration hides a function that it
+    // uses in its initializer.
+    return "f_" + name.getString();
+}
+
 TString TypeString(const TType &type)
 {
     const TStructure *structure = type.getStruct();

src/compiler/translator/UtilsHLSL.h

@@ -62,10 +62,10 @@ TString TextureGroupSuffix(const TBasicType type);
 TString TextureTypeSuffix(const TBasicType type);
 TString SamplerString(const TBasicType type);
 TString SamplerString(HLSLTextureSamplerGroup type);
-// Prepends an underscore to avoid naming clashes
+// Adds a prefix to user-defined names to avoid naming clashes.
 TString Decorate(const TString &string);
-TString DecorateIfNeeded(const TName &name);
-TString DecorateUniform(const TName &name, const TType &type);
+TString DecorateVariableIfNeeded(const TName &name);
+TString DecorateFunctionIfNeeded(const TName &name);
 TString DecorateField(const TString &string, const TStructure &structure);
 TString DecoratePrivate(const TString &privateText);
 TString TypeString(const TType &type);

src/tests/compiler_tests/DebugShaderPrecision_test.cpp

@@ -892,7 +892,7 @@ TEST_F(DebugShaderPrecisionTest, FunctionCallParameterQualifiersFromDefinition)
     // otherwise.
     // Test in parameters
     ASSERT_TRUE(foundInAllGLSLCode("v = add(angle_frm(u1), angle_frm(u2))"));
-    ASSERT_TRUE(foundInHLSLCode("v = _add_float4_float4(angle_frm(_u1), angle_frm(_u2))"));
+    ASSERT_TRUE(foundInHLSLCode("v = f_add_float4_float4(angle_frm(_u1), angle_frm(_u2))"));
     // Test inout parameter
     ASSERT_TRUE(foundInAllGLSLCode("compound_add(v, angle_frm(u3))"));
     ASSERT_TRUE(foundInHLSLCode("compound_add_float4_float4(_v, angle_frm(_u3))"));
@@ -933,7 +933,7 @@ TEST_F(DebugShaderPrecisionTest, FunctionCallParameterQualifiersFromPrototype)
     compile(shaderString);
     // Test in parameters
     ASSERT_TRUE(foundInAllGLSLCode("v = add(angle_frm(u1), angle_frm(u2))"));
-    ASSERT_TRUE(foundInHLSLCode("v = _add_float4_float4(angle_frm(_u1), angle_frm(_u2))"));
+    ASSERT_TRUE(foundInHLSLCode("v = f_add_float4_float4(angle_frm(_u1), angle_frm(_u2))"));
     // Test inout parameter
     ASSERT_TRUE(foundInAllGLSLCode("compound_add(v, angle_frm(u3))"));
     ASSERT_TRUE(foundInHLSLCode("compound_add_float4_float4(_v, angle_frm(_u3))"));
@@ -966,9 +966,9 @@ TEST_F(DebugShaderPrecisionTest, NestedFunctionCalls)
     // Test nested calls
     ASSERT_TRUE(foundInAllGLSLCode(
         "v2 = add(compound_add(v, angle_frm(u2)), angle_frm(fract(angle_frm(u3))))"));
-    ASSERT_TRUE(
-        foundInHLSLCode("v2 = _add_float4_float4(_compound_add_float4_float4(_v, angle_frm(_u2)), "
-                        "angle_frm(frac(angle_frm(_u3))))"));
+    ASSERT_TRUE(foundInHLSLCode(
+        "v2 = f_add_float4_float4(f_compound_add_float4_float4(_v, angle_frm(_u2)), "
+        "angle_frm(frac(angle_frm(_u3))))"));
 }
 
 // Test that code inside an index of a function out parameter gets processed.

src/tests/gl_tests/GLSLTest.cpp

@@ -3110,6 +3110,29 @@ TEST_P(GLSLTest_ES3, ConditionInitializerDeclaresVariable)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test that a variable hides a user-defined function with the same name after its initializer.
+// GLSL ES 1.00.17 section 4.2.2: "A variable declaration is visible immediately following the
+// initializer if present, otherwise immediately following the identifier"
+TEST_P(GLSLTest, VariableHidesUserDefinedFunctionAfterInitializer)
+{
+    const std::string &fragmentShader =
+        "precision mediump float;\n"
+        "uniform vec4 u;\n"
+        "vec4 foo()\n"
+        "{\n"
+        "    return u;\n"
+        "}\n"
+        "void main()\n"
+        "{\n"
+        "    vec4 foo = foo();\n"
+        "    gl_FragColor = foo + vec4(0, 1, 0, 1);\n"
+        "}\n";
+
+    ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader);
+    drawQuad(program.get(), "inputAttribute", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(GLSLTest,
                        ES2_D3D9(),