Fix interpolant ES error

The restriction of no swizzling and no struct fields as an interpolant
were not being checked when using the ES profile.

Fixes #3277.

Test/glsl.interpOp.error.frag

@@ -0,0 +1,73 @@
+#version 320 es
+
+struct S
+{
+    highp float a;
+    highp float b;
+};
+layout(location = 0) in S v_var;
+
+layout(location = 2) in highp float v;
+
+struct S0 {
+    highp vec4 s_v;
+};
+
+layout(location = 3) in FIn {
+    highp float x;
+    highp vec4 xyz[1];
+    S0 s0;
+};
+
+layout(location = 7) in highp float z[1];
+
+layout(location = 8) in highp vec4 w;
+
+layout(location = 0) out mediump vec4 fragColor;
+void main (void)
+{
+    // Centroid
+    {
+        // valid
+        fragColor = vec4(interpolateAtCentroid(v));
+        fragColor = vec4(interpolateAtCentroid(x));
+        fragColor = vec4(interpolateAtCentroid(z[0]));
+        fragColor = interpolateAtCentroid(w);
+        fragColor = interpolateAtCentroid(xyz[0]);
+
+        //// invalid
+        fragColor = vec4(interpolateAtCentroid(v_var.a));
+        fragColor = vec4(interpolateAtCentroid(w.x));
+        fragColor = vec4(interpolateAtCentroid(s0.s_v));
+    }
+
+    // Sample
+    {
+        // valid
+        fragColor = vec4(interpolateAtSample(v, 0));
+        fragColor = vec4(interpolateAtSample(x, 0));
+        fragColor = vec4(interpolateAtSample(z[0], 0));
+        fragColor = interpolateAtSample(w, 0);
+        fragColor = interpolateAtSample(xyz[0], 0);
+
+        // invalid
+        fragColor = vec4(interpolateAtSample(v_var.a, 0));
+        fragColor = vec4(interpolateAtSample(w.x, 0));
+        fragColor = vec4(interpolateAtSample(s0.s_v, 0));
+    }
+
+    // Offset
+    {
+        // valid
+        fragColor = vec4(interpolateAtOffset(v,    vec2(0)));
+        fragColor = vec4(interpolateAtOffset(x,    vec2(0)));
+        fragColor = vec4(interpolateAtOffset(z[0], vec2(0)));
+        fragColor = interpolateAtOffset(w,         vec2(0));
+        fragColor = interpolateAtOffset(xyz[0],    vec2(0));
+
+        // invalid
+        fragColor = vec4(interpolateAtOffset(v_var.a, vec2(0)));
+        fragColor = vec4(interpolateAtOffset(w.x,     vec2(0)));
+        fragColor = vec4(interpolateAtOffset(s0.s_v, vec2(0)));
+    }
+}

glslang/MachineIndependent/Intermediate.cpp

@@ -2647,28 +2647,42 @@ TIntermTyped* TIntermediate::addSwizzle(TSwizzleSelectors<selectorType>& selecto
 // 'swizzleOkay' says whether or not it is okay to consider a swizzle
 // a valid part of the dereference chain.
 //
-// 'BufferReferenceOk' says if type is buffer_reference, the routine stop to find the most left node.
+// 'bufferReferenceOk' says if type is buffer_reference, the routine will stop to find the most left node.
 //
+// 'proc' is an optional function to run on each node that is processed during the traversal. 'proc' must
+// return true to continue the traversal, or false to end the traversal early.
 //
 
-const TIntermTyped* TIntermediate::findLValueBase(const TIntermTyped* node, bool swizzleOkay , bool bufferReferenceOk)
+const TIntermTyped* TIntermediate::traverseLValueBase(const TIntermTyped* node, bool swizzleOkay,
+                                                      bool bufferReferenceOk,
+                                                      std::function<bool(const TIntermNode&)> proc)
 {
     do {
         const TIntermBinary* binary = node->getAsBinaryNode();
-        if (binary == nullptr)
+        if (binary == nullptr) {
+            if (proc) {
+                proc(*node);
+            }
             return node;
+        }
         TOperator op = binary->getOp();
-        if (op != EOpIndexDirect && op != EOpIndexIndirect && op != EOpIndexDirectStruct && op != EOpVectorSwizzle && op != EOpMatrixSwizzle)
+        if (op != EOpIndexDirect && op != EOpIndexIndirect && op != EOpIndexDirectStruct && op != EOpVectorSwizzle &&
+            op != EOpMatrixSwizzle)
             return nullptr;
-        if (! swizzleOkay) {
+        if (!swizzleOkay) {
             if (op == EOpVectorSwizzle || op == EOpMatrixSwizzle)
                 return nullptr;
             if ((op == EOpIndexDirect || op == EOpIndexIndirect) &&
                 (binary->getLeft()->getType().isVector() || binary->getLeft()->getType().isScalar()) &&
-                ! binary->getLeft()->getType().isArray())
+                !binary->getLeft()->getType().isArray())
                 return nullptr;
         }
-        node = node->getAsBinaryNode()->getLeft();
+        if (proc) {
+            if (!proc(*node)) {
+                return node;
+            }
+        }
+        node = binary->getLeft();
         if (bufferReferenceOk && node->isReference())
             return node;
     } while (true);

glslang/MachineIndependent/ParseContextBase.cpp

@@ -208,7 +208,7 @@ bool TParseContextBase::lValueErrorCheck(const TSourceLoc& loc, const char* op,
     //
     // If we get here, we have an error and a message.
     //
-    const TIntermTyped* leftMostTypeNode = TIntermediate::findLValueBase(node, true);
+    const TIntermTyped* leftMostTypeNode = TIntermediate::traverseLValueBase(node, true);
 
     if (symNode)
         error(loc, " l-value required", op, "\"%s\" (%s)", symbol, message);
@@ -234,7 +234,7 @@ void TParseContextBase::rValueErrorCheck(const TSourceLoc& loc, const char* op,
     const TIntermSymbol* symNode = node->getAsSymbolNode();
 
     if (node->getQualifier().isWriteOnly()) {
-        const TIntermTyped* leftMostTypeNode = TIntermediate::findLValueBase(node, true);
+        const TIntermTyped* leftMostTypeNode = TIntermediate::traverseLValueBase(node, true);
 
         if (symNode != nullptr)
             error(loc, "can't read from writeonly object: ", op, symNode->getName().c_str());

glslang/MachineIndependent/ParseHelper.cpp

@@ -2571,7 +2571,7 @@ void TParseContext::builtInOpCheck(const TSourceLoc& loc, const TFunction& fnCan
             requireExtensions(loc, 1, &E_GL_EXT_shader_atomic_float2, fnCandidate.getName().c_str());
         }
 
-        const TIntermTyped* base = TIntermediate::findLValueBase(arg0, true , true);
+        const TIntermTyped* base = TIntermediate::traverseLValueBase(arg0, true, true);
         const char* errMsg = "Only l-values corresponding to shader block storage or shared variables can be used with "
                              "atomic memory functions.";
         if (base) {
@@ -2591,20 +2591,57 @@ void TParseContext::builtInOpCheck(const TSourceLoc& loc, const TFunction& fnCan
     case EOpInterpolateAtCentroid:
     case EOpInterpolateAtSample:
     case EOpInterpolateAtOffset:
-    case EOpInterpolateAtVertex:
-        // Make sure the first argument is an interpolant, or an array element of an interpolant
+    case EOpInterpolateAtVertex: {
         if (arg0->getType().getQualifier().storage != EvqVaryingIn) {
-            // It might still be an array element.
+            // Traverse down the left branch of arg0 to ensure this argument is a valid interpolant.
             //
-            // We could check more, but the semantics of the first argument are already met; the
-            // only way to turn an array into a float/vec* is array dereference and swizzle.
+            // For desktop GL >4.3 we effectively only need to ensure that arg0 represents an l-value from an
+            // input declaration.
             //
-            // ES and desktop 4.3 and earlier:  swizzles may not be used
-            // desktop 4.4 and later: swizzles may be used
-            bool swizzleOkay = (!isEsProfile()) && (version >= 440);
-            const TIntermTyped* base = TIntermediate::findLValueBase(arg0, swizzleOkay);
-            if (base == nullptr || base->getType().getQualifier().storage != EvqVaryingIn)
-                error(loc, "first argument must be an interpolant, or interpolant-array element", fnCandidate.getName().c_str(), "");
+            // For desktop GL <= 4.3 and ES, we must also ensure that swizzling is not used
+            //
+            // For ES, we must also ensure that a field selection operator (i.e., '.') is not used on a named
+            // struct.
+
+            const bool esProfile = isEsProfile();
+            const bool swizzleOkay = !esProfile && (version >= 440);
+
+            std::string interpolantErrorMsg = "first argument must be an interpolant, or interpolant-array element";
+            bool isValid = true; // Assume that the interpolant is valid until we find a condition making it invalid
+            bool isIn = false;   // Checks whether or not the interpolant is a shader input
+            bool structAccessOp = false; // Whether or not the previous node in the chain is a struct accessor
+            TIntermediate::traverseLValueBase(
+                arg0, swizzleOkay, false,
+                [&isValid, &isIn, &interpolantErrorMsg, esProfile, &structAccessOp](const TIntermNode& n) -> bool {
+                    auto* type = n.getAsTyped();
+                    if (type) {
+                        if (type->getType().getQualifier().storage == EvqVaryingIn) {
+                            isIn = true;
+                        }
+                        // If a field accessor was used, it can only be used to access a field with an input block, not a struct.
+                        if (structAccessOp && (type->getType().getBasicType() != EbtBlock)) {
+                            interpolantErrorMsg +=
+                                ". Using the field of a named struct as an interpolant argument is not "
+                                "allowed (ES-only).";
+                            isValid = false;
+                        }
+                    }
+
+                    // ES has different requirements for interpolants than GL
+                    if (esProfile) {
+                        // Swizzling will be taken care of by the 'swizzleOkay' argument passsed to traverseLValueBase,
+                        // so we only ned to check whether or not a field accessor has been used with a named struct.
+                        auto* binary = n.getAsBinaryNode();
+                        if (binary && (binary->getOp() == EOpIndexDirectStruct)) {
+                            structAccessOp = true;
+                        }
+                    }
+                    // Don't continue traversing if we know we have an invalid interpolant at this point.
+                    return isValid;
+                });
+            if (!isIn || !isValid) {
+                error(loc, interpolantErrorMsg.c_str(), fnCandidate.getName().c_str(), "");
+            }
         }
 
         if (callNode.getOp() == EOpInterpolateAtVertex) {
@@ -2620,7 +2657,7 @@ void TParseContext::builtInOpCheck(const TSourceLoc& loc, const TFunction& fnCan
                 }
             }
         }
-        break;
+    } break;
 
     case EOpEmitStreamVertex:
     case EOpEndStreamPrimitive:

glslang/MachineIndependent/localintermediate.h

@@ -43,11 +43,12 @@
 #include "../Public/ShaderLang.h"
 #include "Versions.h"
 
-#include <string>
-#include <vector>
 #include <algorithm>
-#include <set>
 #include <array>
+#include <functional>
+#include <set>
+#include <string>
+#include <vector>
 
 class TInfoSink;
 
@@ -572,7 +573,8 @@ class TIntermediate {
     TIntermTyped* foldSwizzle(TIntermTyped* node, TSwizzleSelectors<TVectorSelector>& fields, const TSourceLoc&);
 
     // Tree ops
-    static const TIntermTyped* findLValueBase(const TIntermTyped*, bool swizzleOkay , bool BufferReferenceOk = false);
+    static const TIntermTyped* traverseLValueBase(const TIntermTyped*, bool swizzleOkay, bool bufferReferenceOk = false,
+                                                  std::function<bool(const TIntermNode&)> proc = {});
 
     // Linkage related
     void addSymbolLinkageNodes(TIntermAggregate*& linkage, EShLanguage, TSymbolTable&);

gtests/AST.FromFile.cpp

@@ -299,7 +299,8 @@ INSTANTIATE_TEST_SUITE_P(
         "EndStreamPrimitive.geom",
         "floatBitsToInt.vert",
         "coord_conventions.frag",
-        "gl_FragCoord.frag"
+        "gl_FragCoord.frag",
+        "glsl.interpOp.error.frag",
     })),
     FileNameAsCustomTestSuffix
 );