Refactor binary node creation

src/compiler/translator/IntermNode.cpp

@@ -17,6 +17,7 @@
 
 #include "common/mathutil.h"
 #include "common/matrix_utils.h"
+#include "compiler/translator/Diagnostics.h"
 #include "compiler/translator/HashNames.h"
 #include "compiler/translator/IntermNode.h"
 #include "compiler/translator/SymbolTable.h"
@@ -565,7 +566,7 @@ void TIntermUnary::promote(const TType *funcReturnType)
 // For lots of operations it should already be established that the operand
 // combination is valid, but returns false if operator can't work on operands.
 //
-bool TIntermBinary::promote(TInfoSink &infoSink)
+bool TIntermBinary::promote()
 {
     ASSERT(mLeft->isArray() == mRight->isArray());
 
@@ -686,8 +687,7 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
         }
         else
         {
-            infoSink.info.message(EPrefixInternalError, getLine(),
-                                  "Missing elses");
+            UNREACHABLE();
             return false;
         }
 
@@ -744,8 +744,7 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
         }
         else
         {
-            infoSink.info.message(EPrefixInternalError, getLine(),
-                                  "Missing elses");
+            UNREACHABLE();
             return false;
         }
 
@@ -836,7 +835,7 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
     return true;
 }
 
-TIntermTyped *TIntermBinary::fold(TInfoSink &infoSink)
+TIntermTyped *TIntermBinary::fold(TDiagnostics *diagnostics)
 {
     TIntermConstantUnion *leftConstant = mLeft->getAsConstantUnion();
     TIntermConstantUnion *rightConstant = mRight->getAsConstantUnion();
@@ -844,7 +843,7 @@ TIntermTyped *TIntermBinary::fold(TInfoSink &infoSink)
     {
         return nullptr;
     }
-    TConstantUnion *constArray = leftConstant->foldBinary(mOp, rightConstant, infoSink);
+    TConstantUnion *constArray = leftConstant->foldBinary(mOp, rightConstant, diagnostics);
 
     // Nodes may be constant folded without being qualified as constant.
     TQualifier resultQualifier = EvqConst;
@@ -917,7 +916,9 @@ TIntermTyped *TIntermAggregate::fold(TInfoSink &infoSink)
 //
 // Returns the constant value to keep using or nullptr.
 //
-TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUnion *rightNode, TInfoSink &infoSink)
+TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op,
+                                                 TIntermConstantUnion *rightNode,
+                                                 TDiagnostics *diagnostics)
 {
     const TConstantUnion *leftArray  = getUnionArrayPointer();
     const TConstantUnion *rightArray = rightNode->getUnionArrayPointer();
@@ -966,14 +967,7 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
 
       case EOpMatrixTimesMatrix:
         {
-            if (getType().getBasicType() != EbtFloat ||
-                rightNode->getBasicType() != EbtFloat)
-            {
-                infoSink.info.message(
-                    EPrefixInternalError, getLine(),
-                    "Constant Folding cannot be done for matrix multiply");
-                return nullptr;
-            }
+            ASSERT(getType().getBasicType() == EbtFloat && rightNode->getBasicType() == EbtFloat);
 
             const int leftCols = getCols();
             const int leftRows = getRows();
@@ -1011,8 +1005,8 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
                   case EbtFloat:
                     if (rightArray[i] == 0.0f)
                     {
-                        infoSink.info.message(EPrefixWarning, getLine(),
-                                              "Divide by zero error during constant folding");
+                        diagnostics->warning(
+                            getLine(), "Divide by zero error during constant folding", "/", "");
                         resultArray[i].setFConst(leftArray[i].getFConst() < 0 ? -FLT_MAX : FLT_MAX);
                     }
                     else
@@ -1025,8 +1019,8 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
                   case EbtInt:
                     if (rightArray[i] == 0)
                     {
-                        infoSink.info.message(EPrefixWarning, getLine(),
-                                              "Divide by zero error during constant folding");
+                        diagnostics->warning(
+                            getLine(), "Divide by zero error during constant folding", "/", "");
                         resultArray[i].setIConst(INT_MAX);
                     }
                     else
@@ -1046,8 +1040,8 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
                   case EbtUInt:
                     if (rightArray[i] == 0)
                     {
-                        infoSink.info.message(EPrefixWarning, getLine(),
-                                              "Divide by zero error during constant folding");
+                        diagnostics->warning(
+                            getLine(), "Divide by zero error during constant folding", "/", "");
                         resultArray[i].setUConst(UINT_MAX);
                     }
                     else
@@ -1065,9 +1059,8 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
                     break;
 
                   default:
-                    infoSink.info.message(EPrefixInternalError, getLine(),
-                                          "Constant folding cannot be done for \"/\"");
-                    return nullptr;
+                      UNREACHABLE();
+                      return nullptr;
                 }
             }
         }
@@ -1075,12 +1068,7 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
 
       case EOpMatrixTimesVector:
         {
-            if (rightNode->getBasicType() != EbtFloat)
-            {
-                infoSink.info.message(EPrefixInternalError, getLine(),
-                                      "Constant Folding cannot be done for matrix times vector");
-                return nullptr;
-            }
+            ASSERT(rightNode->getBasicType() == EbtFloat);
 
             const int matrixCols = getCols();
             const int matrixRows = getRows();
@@ -1102,12 +1090,7 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
 
       case EOpVectorTimesMatrix:
         {
-            if (getType().getBasicType() != EbtFloat)
-            {
-                infoSink.info.message(EPrefixInternalError, getLine(),
-                                      "Constant Folding cannot be done for vector times matrix");
-                return nullptr;
-            }
+            ASSERT(getType().getBasicType() == EbtFloat);
 
             const int matrixCols = rightNode->getType().getCols();
             const int matrixRows = rightNode->getType().getRows();
@@ -1149,18 +1132,11 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
 
       case EOpLogicalXor:
         {
+            ASSERT(getType().getBasicType() == EbtBool);
             resultArray = new TConstantUnion[objectSize];
             for (size_t i = 0; i < objectSize; i++)
             {
-                switch (getType().getBasicType())
-                {
-                  case EbtBool:
-                    resultArray[i].setBConst(leftArray[i] != rightArray[i]);
-                    break;
-                  default:
-                    UNREACHABLE();
-                    break;
-                }
+                resultArray[i].setBConst(leftArray[i] != rightArray[i]);
             }
         }
         break;
@@ -1240,10 +1216,8 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUn
         break;
 
       default:
-        infoSink.info.message(
-            EPrefixInternalError, getLine(),
-            "Invalid operator for constant folding");
-        return nullptr;
+          UNREACHABLE();
+          return nullptr;
     }
     return resultArray;
 }

src/compiler/translator/IntermNode.h

@@ -27,6 +27,8 @@
 #include "compiler/translator/Operator.h"
 #include "compiler/translator/Types.h"
 
+class TDiagnostics;
+
 class TIntermTraverser;
 class TIntermAggregate;
 class TIntermBinary;
@@ -349,7 +351,9 @@ class TIntermConstantUnion : public TIntermTyped
     void traverse(TIntermTraverser *it) override;
     bool replaceChildNode(TIntermNode *, TIntermNode *) override { return false; }
 
-    TConstantUnion *foldBinary(TOperator op, TIntermConstantUnion *rightNode, TInfoSink &infoSink);
+    TConstantUnion *foldBinary(TOperator op,
+                               TIntermConstantUnion *rightNode,
+                               TDiagnostics *diagnostics);
     TConstantUnion *foldUnaryWithDifferentReturnType(TOperator op, TInfoSink &infoSink);
     TConstantUnion *foldUnaryWithSameReturnType(TOperator op, TInfoSink &infoSink);
 
@@ -406,6 +410,11 @@ class TIntermBinary : public TIntermOperator
         : TIntermOperator(op),
           mAddIndexClamp(false) {}
 
+    TIntermBinary(TOperator op, TIntermTyped *left, TIntermTyped *right)
+        : TIntermOperator(op), mLeft(left), mRight(right), mAddIndexClamp(false)
+    {
+    }
+
     TIntermTyped *deepCopy() const override { return new TIntermBinary(*this); }
 
     TIntermBinary *getAsBinaryNode() override { return this; };
@@ -421,8 +430,8 @@ class TIntermBinary : public TIntermOperator
     void setRight(TIntermTyped *node) { mRight = node; }
     TIntermTyped *getLeft() const { return mLeft; }
     TIntermTyped *getRight() const { return mRight; }
-    bool promote(TInfoSink &);
-    TIntermTyped *fold(TInfoSink &infoSink);
+    bool promote();
+    TIntermTyped *fold(TDiagnostics *diagnostics);
 
     void setAddIndexClamp() { mAddIndexClamp = true; }
     bool getAddIndexClamp() { return mAddIndexClamp; }

src/compiler/translator/Intermediate.cpp

@@ -38,53 +38,6 @@ TIntermSymbol *TIntermediate::addSymbol(
 }
 
 //
-// Connect two nodes with a new parent that does a binary operation on the nodes.
-//
-// Returns the added node.
-//
-TIntermTyped *TIntermediate::addBinaryMath(
-    TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line)
-{
-    //
-    // Need a new node holding things together then.  Make
-    // one and promote it to the right type.
-    //
-    TIntermBinary *node = new TIntermBinary(op);
-    node->setLine(line);
-
-    node->setLeft(left);
-    node->setRight(right);
-    if (!node->promote(mInfoSink))
-        return NULL;
-
-    // See if we can fold constants.
-    TIntermTyped *foldedNode = node->fold(mInfoSink);
-    if (foldedNode)
-        return foldedNode;
-
-    return node;
-}
-
-//
-// Connect two nodes through an assignment.
-//
-// Returns the added node.
-//
-TIntermTyped *TIntermediate::addAssign(
-    TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line)
-{
-    TIntermBinary *node = new TIntermBinary(op);
-    node->setLine(line);
-
-    node->setLeft(left);
-    node->setRight(right);
-    if (!node->promote(mInfoSink))
-        return NULL;
-
-    return node;
-}
-
-//
 // Connect two nodes through an index operator, where the left node is the base
 // of an array or struct, and the right node is a direct or indirect offset.
 //

src/compiler/translator/Intermediate.h

@@ -28,10 +28,6 @@ class TIntermediate
 
     TIntermSymbol *addSymbol(
         int id, const TString &, const TType &, const TSourceLoc &);
-    TIntermTyped *addBinaryMath(
-        TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &);
-    TIntermTyped *addAssign(
-        TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &);
     TIntermTyped *addIndex(
         TOperator op, TIntermTyped *base, TIntermTyped *index, const TSourceLoc &);
     TIntermTyped *addUnaryMath(

src/compiler/translator/ParseContext.cpp

@@ -3631,7 +3631,18 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op,
             break;
     }
 
-    return intermediate.addBinaryMath(op, left, right, loc);
+    TIntermBinary *node = new TIntermBinary(op, left, right);
+    node->setLine(loc);
+
+    if (!node->promote())
+        return nullptr;
+
+    // See if we can fold constants.
+    TIntermTyped *foldedNode = node->fold(&mDiagnostics);
+    if (foldedNode)
+        return foldedNode;
+
+    return node;
 }
 
 TIntermTyped *TParseContext::addBinaryMath(TOperator op,
@@ -3674,7 +3685,13 @@ TIntermTyped *TParseContext::createAssign(TOperator op,
 {
     if (binaryOpCommonCheck(op, left, right, loc))
     {
-        return intermediate.addAssign(op, left, right, loc);
+        TIntermBinary *node = new TIntermBinary(op, left, right);
+        node->setLine(loc);
+
+        if (!node->promote())
+            return nullptr;
+
+        return node;
     }
     return nullptr;
 }

src/compiler/translator/RemovePow.cpp

@@ -52,8 +52,6 @@ bool RemovePowTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
 {
     if (IsProblematicPow(node))
     {
-        TInfoSink nullSink;
-
         TIntermTyped *x = node->getSequence()->at(0)->getAsTyped();
         TIntermTyped *y = node->getSequence()->at(1)->getAsTyped();
 
@@ -62,11 +60,9 @@ bool RemovePowTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
         log->setLine(node->getLine());
         log->setType(x->getType());
 
-        TIntermBinary *mul = new TIntermBinary(EOpMul);
-        mul->setLeft(y);
-        mul->setRight(log);
+        TIntermBinary *mul = new TIntermBinary(EOpMul, y, log);
         mul->setLine(node->getLine());
-        bool valid = mul->promote(nullSink);
+        bool valid = mul->promote();
         UNUSED_ASSERTION_VARIABLE(valid);
         ASSERT(valid);