Remove repeated "success" check from compileTreeImpl

src/compiler/translator/Compiler.cpp

@@ -335,21 +335,38 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
     TScopedSymbolTableLevel scopedSymbolLevel(&symbolTable);
 
     // Parse shader.
-    bool success = (PaParseStrings(numStrings - firstSource, &shaderStrings[firstSource], nullptr,
-                                   &parseContext) == 0) &&
-                   (parseContext.getTreeRoot() != nullptr);
+    if (PaParseStrings(numStrings - firstSource, &shaderStrings[firstSource], nullptr,
+                       &parseContext) != 0)
+    {
+        return nullptr;
+    }
 
-    shaderVersion = parseContext.getShaderVersion();
-    if (success && MapSpecToShaderVersion(shaderSpec) < shaderVersion)
+    if (parseContext.getTreeRoot() == nullptr)
     {
-        mDiagnostics.globalError("unsupported shader version");
-        success = false;
+        return nullptr;
     }
 
-    TIntermBlock *root = nullptr;
+    setASTMetadata(parseContext);
 
-    if (success)
+    if (MapSpecToShaderVersion(shaderSpec) < shaderVersion)
     {
+        mDiagnostics.globalError("unsupported shader version");
+        return nullptr;
+    }
+
+    TIntermBlock *root = parseContext.getTreeRoot();
+    if (!checkAndSimplifyAST(root, parseContext, compileOptions))
+    {
+        return nullptr;
+    }
+
+    return root;
+}
+
+void TCompiler::setASTMetadata(const TParseContext &parseContext)
+{
+    shaderVersion = parseContext.getShaderVersion();
+
     mPragma = parseContext.pragma();
     symbolTable.setGlobalInvariant(mPragma.stdgl.invariantAll);
 
@@ -358,23 +375,27 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
 
     mNumViews = parseContext.getNumViews();
 
-        root = parseContext.getTreeRoot();
-
     // Highp might have been auto-enabled based on shader version
     fragmentPrecisionHigh = parseContext.getFragmentPrecisionHigh();
 
-        if (success && shaderType == GL_GEOMETRY_SHADER_OES)
+    if (shaderType == GL_GEOMETRY_SHADER_OES)
     {
         mGeometryShaderInputPrimitiveType  = parseContext.getGeometryShaderInputPrimitiveType();
-            mGeometryShaderOutputPrimitiveType =
-                parseContext.getGeometryShaderOutputPrimitiveType();
+        mGeometryShaderOutputPrimitiveType = parseContext.getGeometryShaderOutputPrimitiveType();
         mGeometryShaderMaxVertices         = parseContext.getGeometryShaderMaxVertices();
         mGeometryShaderInvocations         = parseContext.getGeometryShaderInvocations();
     }
+}
 
+bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
+                                    const TParseContext &parseContext,
+                                    ShCompileOptions compileOptions)
+{
     // Disallow expressions deemed too complex.
-        if (success && (compileOptions & SH_LIMIT_EXPRESSION_COMPLEXITY))
-            success = limitExpressionComplexity(root);
+    if ((compileOptions & SH_LIMIT_EXPRESSION_COMPLEXITY) && !limitExpressionComplexity(root))
+    {
+        return false;
+    }
 
     // We prune no-ops to work around driver bugs and to keep AST processing and output simple.
     // The following kinds of no-ops are pruned:
@@ -383,7 +404,6 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
     //      for float, so float literal statements would end up with no precision which is
     //      invalid ESSL.
     // After this empty declarations are not allowed in the AST.
-        if (success)
     PruneNoOps(root);
 
     // In case the last case inside a switch statement is a certain type of no-op, GLSL
@@ -391,72 +411,75 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
     // end of switch statements. This is also required because PruneNoOps may have left switch
     // statements that only contained an empty declaration inside the final case in an invalid
     // state. Relies on that PruneNoOps has already been run.
-        if (success)
     RemoveNoOpCasesFromEndOfSwitchStatements(root, &symbolTable);
 
     // Remove empty switch statements - this makes output simpler.
-        if (success)
     RemoveEmptySwitchStatements(root);
 
     // Create the function DAG and check there is no recursion
-        if (success)
-            success = initCallDag(root);
+    if (!initCallDag(root))
+    {
+        return false;
+    }
 
-        if (success && (compileOptions & SH_LIMIT_CALL_STACK_DEPTH))
-            success = checkCallDepth();
+    if ((compileOptions & SH_LIMIT_CALL_STACK_DEPTH) && !checkCallDepth())
+    {
+        return false;
+    }
 
     // Checks which functions are used and if "main" exists
-        if (success)
-        {
     functionMetadata.clear();
     functionMetadata.resize(mCallDag.size());
-            success = tagUsedFunctions();
+    if (!tagUsedFunctions())
+    {
+        return false;
     }
 
-        if (success && !(compileOptions & SH_DONT_PRUNE_UNUSED_FUNCTIONS))
-            success = pruneUnusedFunctions(root);
+    if (!(compileOptions & SH_DONT_PRUNE_UNUSED_FUNCTIONS))
+    {
+        pruneUnusedFunctions(root);
+    }
 
-        if (success && shaderVersion >= 310)
+    if (shaderVersion >= 310 && !ValidateVaryingLocations(root, &mDiagnostics, shaderType))
     {
-            success = ValidateVaryingLocations(root, &mDiagnostics, shaderType);
+        return false;
     }
 
-        if (success && shaderVersion >= 300 && shaderType == GL_FRAGMENT_SHADER)
+    if (shaderVersion >= 300 && shaderType == GL_FRAGMENT_SHADER &&
+        !ValidateOutputs(root, getExtensionBehavior(), compileResources.MaxDrawBuffers,
+                         &mDiagnostics))
     {
-            success = ValidateOutputs(root, getExtensionBehavior(), compileResources.MaxDrawBuffers,
-                                      &mDiagnostics);
+        return false;
     }
 
-        if (success && shouldRunLoopAndIndexingValidation(compileOptions))
-            success =
-                ValidateLimitations(root, shaderType, &symbolTable, shaderVersion, &mDiagnostics);
+    if (shouldRunLoopAndIndexingValidation(compileOptions) &&
+        !ValidateLimitations(root, shaderType, &symbolTable, shaderVersion, &mDiagnostics))
+    {
+        return false;
+    }
 
     // Fail compilation if precision emulation not supported.
-        if (success && getResources().WEBGL_debug_shader_precision &&
-            getPragma().debugShaderPrecision)
-        {
-            if (!EmulatePrecision::SupportedInLanguage(outputType))
+    if (getResources().WEBGL_debug_shader_precision && getPragma().debugShaderPrecision &&
+        !EmulatePrecision::SupportedInLanguage(outputType))
     {
         mDiagnostics.globalError("Precision emulation not supported for this output type.");
-                success = false;
-            }
+        return false;
     }
 
     // Built-in function emulation needs to happen after validateLimitations pass.
-        if (success)
-        {
     // TODO(jmadill): Remove global pool allocator.
     GetGlobalPoolAllocator()->lock();
     initBuiltInFunctionEmulator(&builtInFunctionEmulator, compileOptions);
     GetGlobalPoolAllocator()->unlock();
     builtInFunctionEmulator.markBuiltInFunctionsForEmulation(root);
-        }
 
     // Clamping uniform array bounds needs to happen after validateLimitations pass.
-        if (success && (compileOptions & SH_CLAMP_INDIRECT_ARRAY_BOUNDS))
+    if (compileOptions & SH_CLAMP_INDIRECT_ARRAY_BOUNDS)
+    {
         arrayBoundsClamper.MarkIndirectArrayBoundsForClamping(root);
+    }
 
-        if (success && (compileOptions & SH_INITIALIZE_BUILTINS_FOR_INSTANCED_MULTIVIEW) &&
+    if ((compileOptions & SH_INITIALIZE_BUILTINS_FOR_INSTANCED_MULTIVIEW) &&
         parseContext.isExtensionEnabled(TExtension::OVR_multiview) &&
         getShaderType() != GL_COMPUTE_SHADER)
     {
@@ -465,31 +488,30 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
     }
 
     // This pass might emit short circuits so keep it before the short circuit unfolding
-        if (success && (compileOptions & SH_REWRITE_DO_WHILE_LOOPS))
+    if (compileOptions & SH_REWRITE_DO_WHILE_LOOPS)
         RewriteDoWhile(root, &symbolTable);
 
-        if (success && (compileOptions & SH_ADD_AND_TRUE_TO_LOOP_CONDITION))
+    if (compileOptions & SH_ADD_AND_TRUE_TO_LOOP_CONDITION)
         sh::AddAndTrueToLoopCondition(root);
 
-        if (success && (compileOptions & SH_UNFOLD_SHORT_CIRCUIT))
+    if (compileOptions & SH_UNFOLD_SHORT_CIRCUIT)
     {
         UnfoldShortCircuitAST unfoldShortCircuit;
         root->traverse(&unfoldShortCircuit);
         unfoldShortCircuit.updateTree();
     }
 
-        if (success && (compileOptions & SH_REMOVE_POW_WITH_CONSTANT_EXPONENT))
+    if (compileOptions & SH_REMOVE_POW_WITH_CONSTANT_EXPONENT)
     {
         RemovePow(root);
     }
 
-        if (success && shouldCollectVariables(compileOptions))
+    if (shouldCollectVariables(compileOptions))
     {
         ASSERT(!variablesCollected);
         CollectVariables(root, &attributes, &outputVariables, &uniforms, &inputVaryings,
                          &outputVaryings, &uniformBlocks, &shaderStorageBlocks, &inBlocks,
-                             hashFunction, &symbolTable, shaderVersion, shaderType,
-                             extensionBehavior);
+                         hashFunction, &symbolTable, shaderVersion, shaderType, extensionBehavior);
         collectInterfaceBlocks();
         variablesCollected = true;
         if (compileOptions & SH_USE_UNUSED_STANDARD_SHARED_BLOCKS)
@@ -500,13 +522,13 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
         {
             // Returns true if, after applying the packing rules in the GLSL ES 1.00.17 spec
             // Appendix A, section 7, the shader does not use too many uniforms.
-                success = CheckVariablesInPackingLimits(maxUniformVectors, uniforms);
-                if (!success)
+            if (!CheckVariablesInPackingLimits(maxUniformVectors, uniforms))
             {
                 mDiagnostics.globalError("too many uniforms");
+                return false;
             }
         }
-            if (success && (compileOptions & SH_INIT_OUTPUT_VARIABLES))
+        if (compileOptions & SH_INIT_OUTPUT_VARIABLES)
         {
             initializeOutputVariables(root);
         }
@@ -515,9 +537,8 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
     // gl_Position is always written in compatibility output mode.
     // It may have been already initialized among other output variables, in that case we don't
     // need to initialize it twice.
-        if (success && shaderType == GL_VERTEX_SHADER && !mGLPositionInitialized &&
-            ((compileOptions & SH_INIT_GL_POSITION) ||
-             (outputType == SH_GLSL_COMPATIBILITY_OUTPUT)))
+    if (shaderType == GL_VERTEX_SHADER && !mGLPositionInitialized &&
+        ((compileOptions & SH_INIT_GL_POSITION) || (outputType == SH_GLSL_COMPATIBILITY_OUTPUT)))
     {
         initializeGLPosition(root);
         mGLPositionInitialized = true;
@@ -525,22 +546,23 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
 
     // Removing invariant declarations must be done after collecting variables.
     // Otherwise, built-in invariant declarations don't apply.
-        if (success && RemoveInvariant(shaderType, shaderVersion, outputType, compileOptions))
+    if (RemoveInvariant(shaderType, shaderVersion, outputType, compileOptions))
+    {
         sh::RemoveInvariantDeclaration(root);
+    }
 
-        if (success && (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS))
+    if (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
     {
-            ScalarizeVecAndMatConstructorArgs(root, shaderType, fragmentPrecisionHigh,
-                                              &symbolTable);
+        ScalarizeVecAndMatConstructorArgs(root, shaderType, fragmentPrecisionHigh, &symbolTable);
     }
 
-        if (success && (compileOptions & SH_REGENERATE_STRUCT_NAMES))
+    if (compileOptions & SH_REGENERATE_STRUCT_NAMES)
     {
         RegenerateStructNames gen(&symbolTable, shaderVersion);
         root->traverse(&gen);
     }
 
-        if (success && shaderType == GL_FRAGMENT_SHADER && shaderVersion == 100 &&
+    if (shaderType == GL_FRAGMENT_SHADER && shaderVersion == 100 &&
         compileResources.EXT_draw_buffers && compileResources.MaxDrawBuffers > 1 &&
         IsExtensionEnabled(extensionBehavior, TExtension::EXT_draw_buffers))
     {
@@ -548,32 +570,26 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
                                     &symbolTable, shaderVersion);
     }
 
-        if (success)
-        {
     DeferGlobalInitializers(root, needToInitializeGlobalsInAST(), &symbolTable);
-        }
 
     // Split multi declarations and remove calls to array length().
-        if (success)
-        {
     // Note that SimplifyLoopConditions needs to be run before any other AST transformations
     // that may need to generate new statements from loop conditions or loop expressions.
-            SimplifyLoopConditions(root,
-                                   IntermNodePatternMatcher::kMultiDeclaration |
-                                       IntermNodePatternMatcher::kArrayLengthMethod,
+    SimplifyLoopConditions(
+        root,
+        IntermNodePatternMatcher::kMultiDeclaration | IntermNodePatternMatcher::kArrayLengthMethod,
         &getSymbolTable(), getShaderVersion());
 
     // Note that separate declarations need to be run before other AST transformations that
     // generate new statements from expressions.
     SeparateDeclarations(root);
 
-            SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod,
-                                  &getSymbolTable(), getShaderVersion());
+    SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod, &getSymbolTable(),
+                          getShaderVersion());
 
     RemoveArrayLengthMethod(root);
-        }
 
-        if (success && (compileOptions & SH_INITIALIZE_UNINITIALIZED_LOCALS) && getOutputType())
+    if ((compileOptions & SH_INITIALIZE_UNINITIALIZED_LOCALS) && getOutputType())
     {
         // Initialize uninitialized local variables.
         // In some cases initializing can generate extra statements in the parent block, such as
@@ -593,17 +609,12 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
         InitializeUninitializedLocals(root, getShaderVersion());
     }
 
-        if (success && getShaderType() == GL_VERTEX_SHADER &&
-            (compileOptions & SH_CLAMP_POINT_SIZE))
+    if (getShaderType() == GL_VERTEX_SHADER && (compileOptions & SH_CLAMP_POINT_SIZE))
     {
         ClampPointSize(root, compileResources.MaxPointSize, &getSymbolTable());
     }
-    }
-
-    if (success)
-        return root;
 
-    return nullptr;
+    return true;
 }
 
 bool TCompiler::compile(const char *const shaderStrings[],
@@ -985,7 +996,7 @@ class TCompiler::UnusedPredicate
     const std::vector<FunctionMetadata> *mMetadatas;
 };
 
-bool TCompiler::pruneUnusedFunctions(TIntermBlock *root)
+void TCompiler::pruneUnusedFunctions(TIntermBlock *root)
 {
     UnusedPredicate isUnused(&mCallDag, &functionMetadata);
     TIntermSequence *sequence = root->getSequence();
@@ -995,8 +1006,6 @@ bool TCompiler::pruneUnusedFunctions(TIntermBlock *root)
         sequence->erase(std::remove_if(sequence->begin(), sequence->end(), isUnused),
                         sequence->end());
     }
-
-    return true;
 }
 
 bool TCompiler::limitExpressionComplexity(TIntermBlock *root)

src/compiler/translator/Compiler.h

@@ -30,6 +30,7 @@ namespace sh
 {
 
 class TCompiler;
+class TParseContext;
 #ifdef ANGLE_ENABLE_HLSL
 class TranslatorHLSL;
 #endif  // ANGLE_ENABLE_HLSL
@@ -218,12 +219,21 @@ class TCompiler : public TShHandleBase
 
     // Removes unused function declarations and prototypes from the AST
     class UnusedPredicate;
-    bool pruneUnusedFunctions(TIntermBlock *root);
+    void pruneUnusedFunctions(TIntermBlock *root);
 
     TIntermBlock *compileTreeImpl(const char *const shaderStrings[],
                                   size_t numStrings,
                                   const ShCompileOptions compileOptions);
 
+    // Fetches and stores shader metadata that is not stored within the AST itself, such as shader
+    // version.
+    void setASTMetadata(const TParseContext &parseContext);
+
+    // Does checks that need to be run after parsing is complete and returns true if they pass.
+    bool checkAndSimplifyAST(TIntermBlock *root,
+                             const TParseContext &parseContext,
+                             ShCompileOptions compileOptions);
+
     sh::GLenum shaderType;
     ShShaderSpec shaderSpec;
     ShShaderOutput outputType;