Implemented a CallDAG to allow for more AST analysis

src/compiler.gypi

@@ -26,13 +26,13 @@
             'compiler/translator/BuiltInFunctionEmulator.h',
             'compiler/translator/BuiltInFunctionEmulatorGLSL.cpp',
             'compiler/translator/BuiltInFunctionEmulatorGLSL.h',
+            'compiler/translator/CallDAG.cpp',
+            'compiler/translator/CallDAG.h',
             'compiler/translator/CodeGen.cpp',
             'compiler/translator/Common.h',
             'compiler/translator/Compiler.cpp',
             'compiler/translator/Compiler.h',
             'compiler/translator/ConstantUnion.h',
-            'compiler/translator/DetectCallDepth.cpp',
-            'compiler/translator/DetectCallDepth.h',
             'compiler/translator/Diagnostics.cpp',
             'compiler/translator/Diagnostics.h',
             'compiler/translator/DirectiveHandler.cpp',

src/compiler/translator/CallDAG.cpp

+//
+// Copyright (c) 2002-2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+// CallDAG.h: Implements a call graph DAG of functions to be re-used accross
+// analyses, allows to efficiently traverse the functions in topological
+// order.
+
+#include "compiler/translator/CallDAG.h"
+#include "compiler/translator/InfoSink.h"
+
+// The CallDAGCreator does all the processing required to create the CallDAG
+// structure so that the latter contains only the necessary variables.
+class CallDAG::CallDAGCreator : public TIntermTraverser
+{
+  public:
+    CallDAGCreator(TInfoSinkBase *info)
+        : TIntermTraverser(true, false, true),
+          mCurrentFunction(nullptr),
+          mCurrentIndex(0),
+          mCreationInfo(info)
+    {
+    }
+
+    InitResult assignIndices()
+    {
+        int skipped = 0;
+        for (auto &it : mFunctions)
+        {
+            // Skip unimplemented functions
+            if (it.second.node)
+            {
+                InitResult result = assignIndicesInternal(&it.second);
+                if (result != INITDAG_SUCCESS)
+                {
+                    return result;
+                }
+            }
+            else
+            {
+                skipped++;
+            }
+        }
+        ASSERT(mFunctions.size() == mCurrentIndex + skipped);
+        return INITDAG_SUCCESS;
+    }
+
+    void fillDataStructures(std::vector<Record> *records, std::map<int, int> *idToIndex)
+    {
+        ASSERT(records->empty());
+        ASSERT(idToIndex->empty());
+
+        records->resize(mCurrentIndex);
+
+        for (auto &it : mFunctions)
+        {
+            CreatorFunctionData &data = it.second;
+            // Skip unimplemented functions
+            if (!data.node)
+            {
+                continue;
+            }
+            ASSERT(data.index < records->size());
+            Record &record = (*records)[data.index];
+
+            record.name = data.name.data();
+            record.node = data.node;
+
+            record.callees.reserve(data.callees.size());
+            for (auto &callee : data.callees)
+            {
+                record.callees.push_back(callee->index);
+            }
+
+            (*idToIndex)[data.node->getFunctionId()] = data.index;
+        }
+    }
+
+  private:
+
+    struct CreatorFunctionData
+    {
+        CreatorFunctionData()
+            : node(nullptr),
+              index(0),
+              indexAssigned(false),
+              visiting(false)
+        {
+        }
+
+        std::set<CreatorFunctionData*> callees;
+        TIntermAggregate *node;
+        TString name;
+        size_t index;
+        bool indexAssigned;
+        bool visiting;
+    };
+
+    // Aggregates the AST node for each function as well as the name of the functions called by it
+    bool visitAggregate(Visit visit, TIntermAggregate *node) override
+    {
+        switch (node->getOp())
+        {
+          case EOpPrototype:
+            if (visit == PreVisit)
+            {
+                // Function declaration, create an empty record.
+                mFunctions[node->getName()];
+            }
+            break;
+          case EOpFunction:
+            {
+                // Function definition, create the record if need be and remember the node.
+                if (visit == PreVisit)
+                {
+                    auto it = mFunctions.find(node->getName());
+
+                    if (it == mFunctions.end())
+                    {
+                        mCurrentFunction = &mFunctions[node->getName()];
+                    }
+                    else
+                    {
+                        mCurrentFunction = &it->second;
+                    }
+
+                    mCurrentFunction->node = node;
+                    mCurrentFunction->name = node->getName();
+
+                }
+                else if (visit == PostVisit)
+                {
+                    mCurrentFunction = nullptr;
+                }
+                break;
+            }
+          case EOpFunctionCall:
+            {
+                // Function call, add the callees
+                if (visit == PreVisit)
+                {
+                    // Do not handle calls to builtin functions
+                    if (node->isUserDefined())
+                    {
+                        auto it = mFunctions.find(node->getName());
+                        ASSERT(it != mFunctions.end());
+
+                        // We might be in a top-level function call to set a global variable
+                        if (mCurrentFunction)
+                        {
+                            mCurrentFunction->callees.insert(&it->second);
+                        }
+                    }
+                }
+                break;
+            }
+          default:
+            break;
+        }
+        return true;
+    }
+
+    // Recursively assigns indices to a sub DAG
+    InitResult assignIndicesInternal(CreatorFunctionData *function)
+    {
+        ASSERT(function);
+
+        if (!function->node)
+        {
+            *mCreationInfo << "Undefined function: " << function->name;
+            return INITDAG_UNDEFINED;
+        }
+
+        if (function->indexAssigned)
+        {
+            return INITDAG_SUCCESS;
+        }
+
+        if (function->visiting)
+        {
+            if (mCreationInfo)
+            {
+                *mCreationInfo << "Recursive function call in the following call chain: " << function->name;
+            }
+            return INITDAG_RECURSION;
+        }
+        function->visiting = true;
+
+        for (auto &callee : function->callees)
+        {
+            InitResult result = assignIndicesInternal(callee);
+            if (result == INITDAG_RECURSION)
+            {
+                // We know that there is a recursive function call chain in the AST,
+                // print the link of the chain we were processing.
+                if (mCreationInfo)
+                {
+                    *mCreationInfo << " <- " << function->name;
+                }
+                return INITDAG_RECURSION;
+            }
+            else if (result == INITDAG_UNDEFINED)
+            {
+                return INITDAG_UNDEFINED;
+            }
+        }
+
+        function->index = mCurrentIndex++;
+        function->indexAssigned = true;
+
+        function->visiting = false;
+        return INITDAG_SUCCESS;
+    }
+
+    TInfoSinkBase *mCreationInfo;
+
+    std::map<TString, CreatorFunctionData> mFunctions;
+    CreatorFunctionData *mCurrentFunction;
+    size_t mCurrentIndex;
+};
+
+// CallDAG
+
+CallDAG::CallDAG()
+{
+}
+
+CallDAG::~CallDAG()
+{
+}
+
+const size_t CallDAG::InvalidIndex = std::numeric_limits<size_t>::max();
+
+size_t CallDAG::findIndex(const TIntermAggregate *function) const
+{
+    TOperator op = function->getOp();
+    ASSERT(op == EOpPrototype || op == EOpFunction || op == EOpFunctionCall);
+
+    auto it = mFunctionIdToIndex.find(function->getFunctionId());
+
+    if (it == mFunctionIdToIndex.end())
+    {
+        return InvalidIndex;
+    }
+    else
+    {
+        return it->second;
+    }
+}
+
+const CallDAG::Record &CallDAG::getRecordFromIndex(size_t index) const
+{
+    ASSERT(index != InvalidIndex && index < mRecords.size());
+    return mRecords[index];
+}
+
+const CallDAG::Record &CallDAG::getRecord(const TIntermAggregate *function) const
+{
+    size_t index = findIndex(function);
+    ASSERT(index != InvalidIndex && index < mRecords.size());
+    return mRecords[index];
+}
+
+size_t CallDAG::size() const
+{
+    return mRecords.size();
+}
+
+void CallDAG::clear()
+{
+    mRecords.clear();
+    mFunctionIdToIndex.clear();
+}
+
+CallDAG::InitResult CallDAG::init(TIntermNode *root, TInfoSinkBase *info)
+{
+    CallDAGCreator creator(info);
+
+    // Creates the mapping of functions to callees
+    root->traverse(&creator);
+
+    // Does the topological sort and detects recursions
+    InitResult result = creator.assignIndices();
+    if (result != INITDAG_SUCCESS)
+    {
+        return result;
+    }
+
+    creator.fillDataStructures(&mRecords, &mFunctionIdToIndex);
+    return INITDAG_SUCCESS;
+}

src/compiler/translator/CallDAG.h

+//
+// Copyright (c) 2002-2015 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+
+// CallDAG.h: Defines a call graph DAG of functions to be re-used accross
+// analyses, allows to efficiently traverse the functions in topological
+// order.
+
+#ifndef COMPILER_TRANSLATOR_CALLDAG_H_
+#define COMPILER_TRANSLATOR_CALLDAG_H_
+
+#include <map>
+
+#include "compiler/translator/IntermNode.h"
+#include "compiler/translator/VariableInfo.h"
+
+
+// The translator needs to analyze the the graph of the function calls
+// to run checks and analyses; since in GLSL recursion is not allowed
+// that graph is a DAG.
+// This class is used to precompute that function call DAG so that it
+// can be reused by multiple analyses.
+//
+// It stores a vector of function records, with one record per function.
+// Records are accessed by index but a mangled function name can be converted
+// to the index of the corresponding record. The records mostly contain the
+// AST node of the function and the indices of the function's callees.
+//
+// In addition, records are in reverse topological order: a function F being
+// called by a function G will have index index(F) < index(G), that way
+// depth-first analysis becomes analysis in the order of indices.
+
+class CallDAG : angle::NonCopyable
+{
+  public:
+    CallDAG();
+    ~CallDAG();
+
+    struct Record
+    {
+        std::string name;
+        TIntermAggregate *node;
+        std::vector<int> callees;
+    };
+
+    enum InitResult
+    {
+        INITDAG_SUCCESS,
+        INITDAG_RECURSION,
+        INITDAG_UNDEFINED,
+    };
+
+    // Returns INITDAG_SUCCESS if it was able to create the DAG, otherwise prints
+    // the initialization error in info, if present.
+    InitResult init(TIntermNode *root, TInfoSinkBase *info);
+
+    // Returns InvalidIndex if the function wasn't found
+    size_t findIndex(const TIntermAggregate *function) const;
+
+    const Record &getRecordFromIndex(size_t index) const;
+    const Record &getRecord(const TIntermAggregate *function) const;
+    size_t size() const;
+    void clear();
+
+    const static size_t InvalidIndex;
+  private:
+    std::vector<Record> mRecords;
+    std::map<int, int> mFunctionIdToIndex;
+
+    class CallDAGCreator;
+};
+
+#endif  // COMPILER_TRANSLATOR_CALLDAG_H_

src/compiler/translator/Compiler.cpp

@@ -5,7 +5,7 @@
 //
 
 #include "compiler/translator/Compiler.h"
-#include "compiler/translator/DetectCallDepth.h"
+#include "compiler/translator/CallDAG.h"
 #include "compiler/translator/ForLoopUnroll.h"
 #include "compiler/translator/Initialize.h"
 #include "compiler/translator/InitializeParseContext.h"
@@ -231,8 +231,20 @@ TIntermNode *TCompiler::compileTreeImpl(const char* const shaderStrings[],
         if (success && (compileOptions & SH_LIMIT_EXPRESSION_COMPLEXITY))
             success = limitExpressionComplexity(root);
 
+        // Create the function DAG and check there is no recursion
         if (success)
-            success = detectCallDepth(root, infoSink, (compileOptions & SH_LIMIT_CALL_STACK_DEPTH) != 0);
+            success = initCallDag(root);
+
+        if (success && (compileOptions & SH_LIMIT_CALL_STACK_DEPTH))
+            success = checkCallDepth();
+
+        // Checks which functions are used and if "main" exists
+        if (success)
+        {
+            functionMetadata.clear();
+            functionMetadata.resize(mCallDag.size());
+            success = tagUsedFunctions();
+        }
 
         if (success && shaderVersion == 300 && shaderType == GL_FRAGMENT_SHADER)
             success = validateOutputs(root);
@@ -456,30 +468,108 @@ void TCompiler::clearResults()
     mSourcePath = NULL;
 }
 
-bool TCompiler::detectCallDepth(TIntermNode* inputRoot, TInfoSink& inputInfoSink, bool limitCallStackDepth)
+bool TCompiler::initCallDag(TIntermNode *root)
 {
-    DetectCallDepth detect(inputInfoSink, limitCallStackDepth, maxCallStackDepth);
-    inputRoot->traverse(&detect);
-    switch (detect.detectCallDepth())
+    mCallDag.clear();
+
+    switch (mCallDag.init(root, &infoSink.info))
     {
-      case DetectCallDepth::kErrorNone:
+      case CallDAG::INITDAG_SUCCESS:
         return true;
-      case DetectCallDepth::kErrorMissingMain:
-        inputInfoSink.info.prefix(EPrefixError);
-        inputInfoSink.info << "Missing main()";
-        return false;
-      case DetectCallDepth::kErrorRecursion:
-        inputInfoSink.info.prefix(EPrefixError);
-        inputInfoSink.info << "Function recursion detected";
-        return false;
-      case DetectCallDepth::kErrorMaxDepthExceeded:
-        inputInfoSink.info.prefix(EPrefixError);
-        inputInfoSink.info << "Function call stack too deep";
+      case CallDAG::INITDAG_RECURSION:
+        infoSink.info.prefix(EPrefixError);
+        infoSink.info << "Function recursion detected";
         return false;
-      default:
-        UNREACHABLE();
+      case CallDAG::INITDAG_UNDEFINED:
+        infoSink.info.prefix(EPrefixError);
+        infoSink.info << "Unimplemented function detected";
         return false;
     }
+
+    UNREACHABLE();
+    return true;
+}
+
+bool TCompiler::checkCallDepth()
+{
+    std::vector<int> depths(mCallDag.size());
+
+    for (size_t i = 0; i < mCallDag.size(); i++)
+    {
+        int depth = 0;
+        auto &record = mCallDag.getRecordFromIndex(i);
+
+        for (auto &calleeIndex : record.callees)
+        {
+            depth = std::max(depth, depths[calleeIndex] + 1);
+        }
+
+        depths[i] = depth;
+
+        if (depth >= maxCallStackDepth)
+        {
+            // Trace back the function chain to have a meaningful info log.
+            infoSink.info.prefix(EPrefixError);
+            infoSink.info << "Call stack too deep (larger than " << maxCallStackDepth
+                          << ") with the following call chain: " << record.name;
+
+            int currentFunction = i;
+            int currentDepth = depth;
+
+            while (currentFunction != -1)
+            {
+                infoSink.info << " -> " << mCallDag.getRecordFromIndex(currentFunction).name;
+
+                int nextFunction = -1;
+                for (auto& calleeIndex : mCallDag.getRecordFromIndex(currentFunction).callees)
+                {
+                    if (depths[calleeIndex] == currentDepth - 1)
+                    {
+                        currentDepth--;
+                        nextFunction = calleeIndex;
+                    }
+                }
+
+                currentFunction = nextFunction;
+            }
+
+            return false;
+        }
+    }
+
+    return true;
+}
+
+bool TCompiler::tagUsedFunctions()
+{
+    // Search from main, starting from the end of the DAG as it usually is the root.
+    for (int i = mCallDag.size(); i-- > 0;)
+    {
+        if (mCallDag.getRecordFromIndex(i).name == "main(")
+        {
+            internalTagUsedFunction(i);
+            return true;
+        }
+    }
+
+    infoSink.info.prefix(EPrefixError);
+    infoSink.info << "Missing main()";
+    return false;
+}
+
+void TCompiler::internalTagUsedFunction(size_t index)
+{
+    if (functionMetadata[index].used)
+    {
+        return;
+    }
+
+    functionMetadata[index].used = true;
+
+    for (int calleeIndex : mCallDag.getRecordFromIndex(index).callees)
+    {
+        internalTagUsedFunction(calleeIndex);
+    }
 }
 
 bool TCompiler::validateOutputs(TIntermNode* root)

src/compiler/translator/Compiler.h

@@ -15,6 +15,7 @@
 //
 
 #include "compiler/translator/BuiltInFunctionEmulator.h"
+#include "compiler/translator/CallDAG.h"
 #include "compiler/translator/ExtensionBehavior.h"
 #include "compiler/translator/HashNames.h"
 #include "compiler/translator/InfoSink.h"
@@ -103,8 +104,8 @@ class TCompiler : public TShHandleBase
     void setResourceString();
     // Clears the results from the previous compilation.
     void clearResults();
-    // Return true if function recursion is detected or call depth exceeded.
-    bool detectCallDepth(TIntermNode* root, TInfoSink& infoSink, bool limitCallStackDepth);
+    // Return false if the call depth is exceeded.
+    bool checkCallDepth();
     // Returns true if a program has no conflicting or missing fragment outputs
     bool validateOutputs(TIntermNode* root);
     // Rewrites a shader's intermediate tree according to the CSS Shaders spec.
@@ -158,6 +159,12 @@ class TCompiler : public TShHandleBase
     std::vector<sh::InterfaceBlock> interfaceBlocks;
 
   private:
+    // Creates the function call DAG for further analysis, returning false if there is a recursion
+    bool initCallDag(TIntermNode *root);
+    // Return false if "main" doesn't exist
+    bool tagUsedFunctions();
+    void internalTagUsedFunction(size_t index);
+
     TIntermNode *compileTreeImpl(const char* const shaderStrings[],
         size_t numStrings, int compileOptions);
 
@@ -165,6 +172,18 @@ class TCompiler : public TShHandleBase
     ShShaderSpec shaderSpec;
     ShShaderOutput outputType;
 
+    struct FunctionMetadata
+    {
+        FunctionMetadata()
+            : used(false)
+        {
+        }
+        bool used;
+    };
+
+    CallDAG mCallDag;
+    std::vector<FunctionMetadata> functionMetadata;
+
     int maxUniformVectors;
     int maxExpressionComplexity;
     int maxCallStackDepth;

src/compiler/translator/DetectCallDepth.cpp

-//
-// Copyright (c) 2002-2011 The ANGLE Project Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-//
-
-#include "compiler/translator/DetectCallDepth.h"
-#include "compiler/translator/InfoSink.h"
-
-DetectCallDepth::FunctionNode::FunctionNode(const TString& fname)
-    : name(fname),
-      visit(PreVisit)
-{
-}
-
-const TString& DetectCallDepth::FunctionNode::getName() const
-{
-    return name;
-}
-
-void DetectCallDepth::FunctionNode::addCallee(
-    DetectCallDepth::FunctionNode* callee)
-{
-    for (size_t i = 0; i < callees.size(); ++i) {
-        if (callees[i] == callee)
-            return;
-    }
-    callees.push_back(callee);
-}
-
-int DetectCallDepth::FunctionNode::detectCallDepth(DetectCallDepth* detectCallDepth, int depth)
-{
-    ASSERT(visit == PreVisit);
-    ASSERT(detectCallDepth);
-
-    int retMaxDepth = depth;
-    visit = InVisit;
-    for (size_t i = 0; i < callees.size(); ++i) {
-        switch (callees[i]->visit) {
-            case InVisit:
-                // cycle detected, i.e., recursion detected.
-                return kInfiniteCallDepth;
-            case PostVisit:
-                break;
-            case PreVisit: {
-                // Check before we recurse so we don't go too depth
-                if (detectCallDepth->checkExceedsMaxDepth(depth))
-                    return depth;
-                int callDepth = callees[i]->detectCallDepth(detectCallDepth, depth + 1);
-                // Check after we recurse so we can exit immediately and provide info.
-                if (detectCallDepth->checkExceedsMaxDepth(callDepth)) {
-                    detectCallDepth->getInfoSink().info << "<-" << callees[i]->getName();
-                    return callDepth;
-                }
-                retMaxDepth = std::max(callDepth, retMaxDepth);
-                break;
-            }
-            default:
-                UNREACHABLE();
-                break;
-        }
-    }
-    visit = PostVisit;
-    return retMaxDepth;
-}
-
-void DetectCallDepth::FunctionNode::reset()
-{
-    visit = PreVisit;
-}
-
-DetectCallDepth::DetectCallDepth(TInfoSink& infoSink, bool limitCallStackDepth, int maxCallStackDepth)
-    : TIntermTraverser(true, false, true, false),
-      currentFunction(NULL),
-      infoSink(infoSink),
-      maxDepth(limitCallStackDepth ? maxCallStackDepth : FunctionNode::kInfiniteCallDepth)
-{
-}
-
-DetectCallDepth::~DetectCallDepth()
-{
-    for (size_t i = 0; i < functions.size(); ++i)
-        delete functions[i];
-}
-
-bool DetectCallDepth::visitAggregate(Visit visit, TIntermAggregate* node)
-{
-    switch (node->getOp())
-    {
-        case EOpPrototype:
-            // Function declaration.
-            // Don't add FunctionNode here because node->getName() is the
-            // unmangled function name.
-            break;
-        case EOpFunction: {
-            // Function definition.
-            if (visit == PreVisit) {
-                currentFunction = findFunctionByName(node->getName());
-                if (currentFunction == NULL) {
-                    currentFunction = new FunctionNode(node->getName());
-                    functions.push_back(currentFunction);
-                }
-            } else if (visit == PostVisit) {
-                currentFunction = NULL;
-            }
-            break;
-        }
-        case EOpFunctionCall: {
-            // Function call.
-            if (visit == PreVisit) {
-                FunctionNode* func = findFunctionByName(node->getName());
-                if (func == NULL) {
-                    func = new FunctionNode(node->getName());
-                    functions.push_back(func);
-                }
-                if (currentFunction)
-                    currentFunction->addCallee(func);
-            }
-            break;
-        }
-        default:
-            break;
-    }
-    return true;
-}
-
-bool DetectCallDepth::checkExceedsMaxDepth(int depth)
-{
-    return depth >= maxDepth;
-}
-
-void DetectCallDepth::resetFunctionNodes()
-{
-    for (size_t i = 0; i < functions.size(); ++i) {
-        functions[i]->reset();
-    }
-}
-
-DetectCallDepth::ErrorCode DetectCallDepth::detectCallDepthForFunction(FunctionNode* func)
-{
-    currentFunction = NULL;
-    resetFunctionNodes();
-
-    int maxCallDepth = func->detectCallDepth(this, 1);
-
-    if (maxCallDepth == FunctionNode::kInfiniteCallDepth)
-        return kErrorRecursion;
-
-    if (maxCallDepth >= maxDepth)
-        return kErrorMaxDepthExceeded;
-
-    return kErrorNone;
-}
-
-DetectCallDepth::ErrorCode DetectCallDepth::detectCallDepth()
-{
-    if (maxDepth != FunctionNode::kInfiniteCallDepth) {
-        // Check all functions because the driver may fail on them
-        // TODO: Before detectingRecursion, strip unused functions.
-        for (size_t i = 0; i < functions.size(); ++i) {
-            ErrorCode error = detectCallDepthForFunction(functions[i]);
-            if (error != kErrorNone)
-                return error;
-        }
-    } else {
-        FunctionNode* main = findFunctionByName("main(");
-        if (main == NULL)
-            return kErrorMissingMain;
-
-        return detectCallDepthForFunction(main);
-    }
-
-    return kErrorNone;
-}
-
-DetectCallDepth::FunctionNode* DetectCallDepth::findFunctionByName(
-    const TString& name)
-{
-    for (size_t i = 0; i < functions.size(); ++i) {
-        if (functions[i]->getName() == name)
-            return functions[i];
-    }
-    return NULL;
-}
-

src/compiler/translator/DetectCallDepth.h

-//
-// Copyright (c) 2002-2011 The ANGLE Project Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-//
-
-#ifndef COMPILER_TRANSLATOR_DETECTCALLDEPTH_H_
-#define COMPILER_TRANSLATOR_DETECTCALLDEPTH_H_
-
-#include <limits.h>
-#include "compiler/translator/IntermNode.h"
-#include "compiler/translator/VariableInfo.h"
-
-class TInfoSink;
-
-// Traverses intermediate tree to detect function recursion.
-class DetectCallDepth : public TIntermTraverser {
-public:
-    enum ErrorCode {
-        kErrorMissingMain,
-        kErrorRecursion,
-        kErrorMaxDepthExceeded,
-        kErrorNone
-    };
-
-    DetectCallDepth(TInfoSink& infoSync, bool limitCallStackDepth, int maxCallStackDepth);
-    ~DetectCallDepth();
-
-    virtual bool visitAggregate(Visit, TIntermAggregate*);
-
-    bool checkExceedsMaxDepth(int depth);
-
-    ErrorCode detectCallDepth();
-
-private:
-    class FunctionNode {
-    public:
-        static const int kInfiniteCallDepth = INT_MAX;
-
-        FunctionNode(const TString& fname);
-
-        const TString& getName() const;
-
-        // If a function is already in the callee list, this becomes a no-op.
-        void addCallee(FunctionNode* callee);
-
-        // Returns kInifinityCallDepth if recursive function calls are detected.
-        int detectCallDepth(DetectCallDepth* detectCallDepth, int depth);
-
-        // Reset state.
-        void reset();
-
-    private:
-        // mangled function name is unique.
-        TString name;
-
-        // functions that are directly called by this function.
-        TVector<FunctionNode*> callees;
-
-        Visit visit;
-    };
-
-    ErrorCode detectCallDepthForFunction(FunctionNode* func);
-    FunctionNode* findFunctionByName(const TString& name);
-    void resetFunctionNodes();
-
-    TInfoSink& getInfoSink() { return infoSink; }
-
-    TVector<FunctionNode*> functions;
-    FunctionNode* currentFunction;
-    TInfoSink& infoSink;
-    int maxDepth;
-
-    DetectCallDepth(const DetectCallDepth&);
-    void operator=(const DetectCallDepth&);
-};
-
-#endif  // COMPILER_TRANSLATOR_DETECTCALLDEPTH_H_

src/compiler/translator/IntermNode.h

@@ -435,6 +435,9 @@ class TIntermAggregate : public TIntermOperator
     void setDebug(bool debug) { mDebug = debug; }
     bool getDebug() const { return mDebug; }
 
+    void setFunctionId(int functionId) { mFunctionId = functionId; }
+    int getFunctionId() const { return mFunctionId; }
+
     void setUseEmulatedFunction() { mUseEmulatedFunction = true; }
     bool getUseEmulatedFunction() { return mUseEmulatedFunction; }
 
@@ -447,6 +450,7 @@ class TIntermAggregate : public TIntermOperator
     TIntermSequence mSequence;
     TString mName;
     bool mUserDefined; // used for user defined function names
+    int mFunctionId;
 
     bool mOptimize;
     bool mDebug;

src/compiler/translator/ParseContext.cpp

@@ -3141,6 +3141,7 @@ TIntermTyped *TParseContext::addFunctionCallOrMethod(TFunction *fnCall, TIntermN
                 if (!builtIn)
                     aggregate->setUserDefined();
                 aggregate->setName(fnCandidate->getMangledName());
+                aggregate->setFunctionId(fnCandidate->getUniqueId());
 
                 // This needs to happen after the name is set
                 if (builtIn)

src/compiler/translator/glslang.y

@@ -613,6 +613,7 @@ declaration
         TIntermAggregate *prototype = new TIntermAggregate;
         prototype->setType(function.getReturnType());
         prototype->setName(function.getMangledName());
+        prototype->setFunctionId(function.getUniqueId());
         
         for (size_t i = 0; i < function.getParamCount(); i++)
         {
@@ -1663,6 +1664,12 @@ function_definition
             context->recover();
         }
         prevDec->setDefined();
+        //
+        // Overload the unique ID of the definition to be the same unique ID as the declaration.
+        // Eventually we will probably want to have only a single definition and just swap the
+        // arguments to be the definition's arguments.
+        //
+        function->setUniqueId(prevDec->getUniqueId());
 
         //
         // Raise error message if main function takes any parameters or return anything other than void
@@ -1735,6 +1742,7 @@ function_definition
         context->intermediate.setAggregateOperator($$, EOpFunction, @1);
         $$->getAsAggregate()->setName($1.function->getMangledName().c_str());
         $$->getAsAggregate()->setType($1.function->getReturnType());
+        $$->getAsAggregate()->setFunctionId($1.function->getUniqueId());
 
         // store the pragma information for debug and optimize and other vendor specific
         // information. This information can be queried from the parse tree

src/compiler/translator/glslang_tab.cpp

@@ -3030,6 +3030,7 @@ yyreduce:
         TIntermAggregate *prototype = new TIntermAggregate;
         prototype->setType(function.getReturnType());
         prototype->setName(function.getMangledName());
+        prototype->setFunctionId(function.getUniqueId());
         
         for (size_t i = 0; i < function.getParamCount(); i++)
         {
@@ -4821,6 +4822,12 @@ yyreduce:
             context->recover();
         }
         prevDec->setDefined();
+        //
+        // Overload the unique ID of the definition to be the same unique ID as the declaration.
+        // Eventually we will probably want to have only a single definition and just swap the
+        // arguments to be the definition's arguments.
+        //
+        function->setUniqueId(prevDec->getUniqueId());
 
         //
         // Raise error message if main function takes any parameters or return anything other than void
@@ -4898,6 +4905,7 @@ yyreduce:
         context->intermediate.setAggregateOperator((yyval.interm.intermNode), EOpFunction, (yylsp[-2]));
         (yyval.interm.intermNode)->getAsAggregate()->setName((yyvsp[-2].interm).function->getMangledName().c_str());
         (yyval.interm.intermNode)->getAsAggregate()->setType((yyvsp[-2].interm).function->getReturnType());
+        (yyval.interm.intermNode)->getAsAggregate()->setFunctionId((yyvsp[-2].interm).function->getUniqueId());
 
         // store the pragma information for debug and optimize and other vendor specific
         // information. This information can be queried from the parse tree

src/tests/compiler_tests/ExpressionLimit_test.cpp

@@ -170,7 +170,7 @@ protected:
 const char* ExpressionLimitTest::kExpressionTooComplex =
     "Expression too complex";
 const char* ExpressionLimitTest::kCallStackTooDeep =
-    "call stack too deep";
+    "Call stack too deep";
 const char* ExpressionLimitTest::kHasRecursion =
     "Function recursion detected";
 
@@ -481,10 +481,11 @@ TEST_F(ExpressionLimitTest, Recursion)
     EXPECT_TRUE(CheckShaderCompilation(
         vertexCompiler, shaderWithRecursion5,
         compileOptions, kHasRecursion));
-    // Check unused recursions passes.
+
+    // Check some more forms of recursion
     EXPECT_TRUE(CheckShaderCompilation(
         vertexCompiler, shaderWithRecursion6,
-        compileOptions, NULL));
+        compileOptions, kHasRecursion));
     EXPECT_TRUE(CheckShaderCompilation(
         vertexCompiler, shaderWithRecursion7,
         compileOptions, kHasRecursion));