Symbol table now properly handles scopes for function declarations.

Previously, they all went into one big pile that was never cleaned up;
this was the wrong thing to do in a world where one might have a 
function declaration inside another functions, say.

ctx.cpp

@@ -824,10 +824,13 @@ llvm::Value *
 FunctionEmitContext::LaneMask(llvm::Value *v) {
     // Call the target-dependent movmsk function to turn the vector mask
     // into an i32 value
-    std::vector<Symbol *> *mm = m->symbolTable->LookupFunction("__movmsk");
+    std::vector<Symbol *> mm;
+    m->symbolTable->LookupFunction("__movmsk", &mm);
     // There should be one with signed int signature, one unsigned int.
-    assert(mm && mm->size() == 2); 
+    assert(mm.size() == 2); 
-    llvm::Function *fmm = (*mm)[0]->function;
+    // We can actually call either one, since both are i32s as far as
+    // LLVM's type system is concerned...
+    llvm::Function *fmm = mm[0]->function;
     return CallInst(fmm, NULL, v, "val_movmsk");
 }
 

expr.cpp

@@ -1537,10 +1537,10 @@ BinaryExpr::Optimize() {
                 Type::Equal(type1, AtomicType::UniformConstFloat) ||
                 Type::Equal(type1, AtomicType::VaryingConstFloat)) {
                 // Get the symbol for the appropriate builtin
-                std::vector<Symbol *> *rcpFuns = 
+                std::vector<Symbol *> rcpFuns;
-                    m->symbolTable->LookupFunction("rcp");
+                m->symbolTable->LookupFunction("rcp", &rcpFuns);
-                if (rcpFuns != NULL) {
+                if (rcpFuns.size() > 0) {
-                    assert(rcpFuns->size() == 2);
+                    assert(rcpFuns.size() == 2);
                     Expr *rcpSymExpr = new FunctionSymbolExpr("rcp", rcpFuns, pos);
                     ExprList *args = new ExprList(arg1, arg1->pos);
                     Expr *rcpCall = new FunctionCallExpr(rcpSymExpr, args, 
@@ -6100,13 +6100,12 @@ SymbolExpr::Print() const {
 // FunctionSymbolExpr
 
 FunctionSymbolExpr::FunctionSymbolExpr(const char *n,
-                                       std::vector<Symbol *> *candidates,
+                                       const std::vector<Symbol *> &candidates,
                                        SourcePos p) 
   : Expr(p) {
     name = n;
     candidateFunctions = candidates;
-    matchingFunc = (candidates && candidates->size() == 1) ? 
+    matchingFunc = (candidates.size() == 1) ? candidates[0] : NULL;
-        (*candidates)[0] : NULL;
     triedToResolve = false;
 }
 
@@ -6457,12 +6456,12 @@ bool
 FunctionSymbolExpr::tryResolve(int (*matchFunc)(const Type *, const Type *),
                                const std::vector<const Type *> &callTypes,
                                const std::vector<bool> *argCouldBeNULL) {
-    const char *funName = candidateFunctions->front()->name.c_str();
+    const char *funName = candidateFunctions.front()->name.c_str();
 
     std::vector<std::pair<int, Symbol *> > matches;
     std::vector<Symbol *>::iterator iter;
-    for (iter = candidateFunctions->begin(); 
+    for (iter = candidateFunctions.begin(); 
-         iter != candidateFunctions->end(); ++iter) {
+         iter != candidateFunctions.end(); ++iter) {
         // Loop over the set of candidate functions and try each one
         Symbol *candidateFunction = *iter;
         const FunctionType *ft = 
@@ -6588,10 +6587,10 @@ FunctionSymbolExpr::ResolveOverloads(const std::vector<const Type *> &argTypes,
     }
 
     // failure :-(
-    const char *funName = candidateFunctions->front()->name.c_str();
+    const char *funName = candidateFunctions.front()->name.c_str();
     Error(pos, "Unable to find matching overload for call to function \"%s\"%s.",
           funName, exactMatchOnly ? " only considering exact matches" : "");
-    lPrintFunctionOverloads(funName, *candidateFunctions);
+    lPrintFunctionOverloads(funName, candidateFunctions);
     lPrintPassedTypes(funName, argTypes);
     return false;
 }

expr.h

@@ -613,7 +613,7 @@ private:
  */    
 class FunctionSymbolExpr : public Expr {
 public:
-    FunctionSymbolExpr(const char *name, std::vector<Symbol *> *candidateFunctions,
+    FunctionSymbolExpr(const char *name, const std::vector<Symbol *> &candFuncs,
                        SourcePos pos);
 
     llvm::Value *GetValue(FunctionEmitContext *ctx) const;
@@ -649,7 +649,7 @@ private:
     /** All of the functions with the name given in the function call;
         there may be more then one, in which case we need to resolve which
         overload is the best match. */
-    std::vector<Symbol *> *candidateFunctions;
+    std::vector<Symbol *> candidateFunctions;
 
     /** The actual matching function found after overload resolution. */
     Symbol *matchingFunc;

module.cpp

@@ -226,7 +226,7 @@ Module::AddGlobalVariable(Symbol *sym, Expr *initExpr, bool isConst) {
         return;
     }
 
-    if (symbolTable->LookupFunction(sym->name.c_str()) != NULL) {
+    if (symbolTable->LookupFunction(sym->name.c_str())) {
         Error(sym->pos, "Global variable \"%s\" shadows previously-declared function.",
               sym->name.c_str());
         return;
@@ -400,11 +400,11 @@ Module::AddFunctionDeclaration(Symbol *funSym, bool isInline) {
         return;
     }
 
-    std::vector<Symbol *> *overloadFuncs = 
+    std::vector<Symbol *> overloadFuncs;
-        symbolTable->LookupFunction(funSym->name.c_str());
+    symbolTable->LookupFunction(funSym->name.c_str(), &overloadFuncs);
-    if (overloadFuncs != NULL) {
+    if (overloadFuncs.size() > 0) {
-        for (unsigned int i = 0; i < overloadFuncs->size(); ++i) {
+        for (unsigned int i = 0; i < overloadFuncs.size(); ++i) {
-            Symbol *overloadFunc = (*overloadFuncs)[i];
+            Symbol *overloadFunc = overloadFuncs[i];
 
             // Check for a redeclaration of a function with the same
             // name and type
@@ -444,21 +444,21 @@ Module::AddFunctionDeclaration(Symbol *funSym, bool isInline) {
             return;
         }
 
-        std::vector<Symbol *> *funcs =
+        std::vector<Symbol *> funcs;
-            symbolTable->LookupFunction(funSym->name.c_str());
+        symbolTable->LookupFunction(funSym->name.c_str(), &funcs);
-        if (funcs != NULL) {
+        if (funcs.size() > 0) {
-            if (funcs->size() > 1) {
+            if (funcs.size() > 1) {
                 // Multiple functions with this name have already been declared; 
                 // can't overload here
                 Error(funSym->pos, "Can't overload extern \"C\" function \"%s\"; "
                       "%d functions with the same name have already been declared.",
-                      funSym->name.c_str(), (int)funcs->size());
+                      funSym->name.c_str(), (int)funcs.size());
                 return;
             }
 
             // One function with the same name has been declared; see if it
             // has the same type as this one, in which case it's ok.
-            if (Type::Equal((*funcs)[0]->type, funSym->type))
+            if (Type::Equal(funcs[0]->type, funSym->type))
                 return;
             else {
                 Error(funSym->pos, "Can't overload extern \"C\" function \"%s\".",
@@ -543,7 +543,7 @@ Module::AddFunctionDeclaration(Symbol *funSym, bool isInline) {
 #endif
         }
 
-        if (symbolTable->LookupFunction(argName.c_str()) != NULL)
+        if (symbolTable->LookupFunction(argName.c_str()))
             Warning(argPos, "Function parameter \"%s\" shadows a function "
                     "declared in global scope.", argName.c_str());
         

parse.yy

@@ -246,8 +246,9 @@ primary_expression
         if (s)
             $$ = new SymbolExpr(s, @1);       
         else {
-            std::vector<Symbol *> *funs = m->symbolTable->LookupFunction(name);
+            std::vector<Symbol *> funs;
-            if (funs)
+            m->symbolTable->LookupFunction(name, &funs);
+            if (funs.size() > 0)
                 $$ = new FunctionSymbolExpr(name, funs, @1);
         }
         if ($$ == NULL) {

sym.cpp

@@ -72,23 +72,30 @@ SymbolTable::SymbolTable() {
 
 SymbolTable::~SymbolTable() {
     // Otherwise we have mismatched push/pop scopes
-    assert(variables.size() == 1 && types.size() == 1);
+    assert(variables.size() == 1 && functions.size() == 1 &&
+           types.size() == 1);
     PopScope();
 }
 
+
 void
 SymbolTable::PushScope() { 
-    variables.push_back(new std::vector<Symbol *>); 
+    variables.push_back(new SymbolMapType);
+    functions.push_back(new FunctionMapType);
     types.push_back(new TypeMapType);
 }
 
 
 void
 SymbolTable::PopScope() { 
-    // FIXME: delete Symbols in variables vector<>...
     assert(variables.size() > 1);
     delete variables.back();
     variables.pop_back();
+
+    assert(functions.size() > 1);
+    delete functions.back();
+    functions.pop_back();
+
     assert(types.size() > 1);
     delete types.back();
     types.pop_back();
@@ -101,48 +108,44 @@ SymbolTable::AddVariable(Symbol *symbol) {
 
     // Check to see if a symbol of the same name has already been declared.
     for (int i = (int)variables.size() - 1; i >= 0; --i) {
-        std::vector<Symbol *> &sv = *(variables[i]);
+        SymbolMapType &sm = *(variables[i]);
-        for (int j = (int)sv.size() - 1; j >= 0; --j) {
+        if (sm.find(symbol->name) != sm.end()) {
-            if (sv[j]->name == symbol->name) {
+            if (i == (int)variables.size()-1) {
-                if (i == (int)variables.size()-1) {
+                // If a symbol of the same name was declared in the
-                    // If a symbol of the same name was declared in the
+                // same scope, it's an error.
-                    // same scope, it's an error.
+                Error(symbol->pos, "Ignoring redeclaration of symbol \"%s\".", 
-                    Error(symbol->pos, "Ignoring redeclaration of symbol \"%s\".", 
+                      symbol->name.c_str());
-                          symbol->name.c_str());
+                return false;
-                    return false;
+            }
-                }
+            else {
-                else {
+                // Otherwise it's just shadowing something else, which
-                    // Otherwise it's just shadowing something else, which
+                // is legal but dangerous..
-                    // is legal but dangerous..
+                Warning(symbol->pos, 
-                    Warning(symbol->pos, 
+                        "Symbol \"%s\" shadows symbol declared in outer scope.",
-                            "Symbol \"%s\" shadows symbol declared in outer scope.",
+                        symbol->name.c_str());
-                            symbol->name.c_str());
+                (*variables.back())[symbol->name] = symbol;
-                    variables.back()->push_back(symbol);
+                return true;
-                    return true;
-                }
             }
         }
     }
 
     // No matches, so go ahead and add it...
-    variables.back()->push_back(symbol);
+    (*variables.back())[symbol->name] = symbol;
     return true;
 }
 
 
 Symbol *
 SymbolTable::LookupVariable(const char *name) {
-    // Note that we iterate through the variables vectors backwards, sinec
+    // Note that we iterate through the variables vectors backwards, since
     // we want to search from the innermost scope to the outermost, so that
     // we get the right symbol if we have multiple variables in different
     // scopes that shadow each other.
-    std::vector<std::vector<Symbol *> *>::reverse_iterator liter = variables.rbegin();
+    for (int i = (int)variables.size() - 1; i >= 0; --i) {
-    while (liter != variables.rend()) {
+        SymbolMapType &sm = *(variables[i]);
-        std::vector<Symbol *> &sv = *(*liter);
+        SymbolMapType::iterator iter = sm.find(name);
-        for (int i = (int)sv.size() - 1; i >= 0; --i)
+        if (iter != sm.end())
-            if (sv[i]->name == name) 
+            return iter->second;
-                return sv[i];
-        ++liter;
     }
     return NULL;
 }
@@ -157,28 +160,44 @@ SymbolTable::AddFunction(Symbol *symbol) {
         // the symbol table
         return false;
 
-    functions[symbol->name].push_back(symbol);
+    std::vector<Symbol *> &funOverloads = (*functions.back())[symbol->name];
+    funOverloads.push_back(symbol);
     return true;
 }
 
 
-std::vector<Symbol *> *
+bool
-SymbolTable::LookupFunction(const char *name) {
+SymbolTable::LookupFunction(const char *name, std::vector<Symbol *> *matches) {
-    if (functions.find(name) != functions.end())
+    for (int i = (int)functions.size() - 1; i >= 0; --i) {
-        return &functions[name];
+        FunctionMapType &fm = *(functions[i]);
-    return NULL;
+        FunctionMapType::iterator iter = fm.find(name);
+        if (iter != fm.end()) {
+            if (matches == NULL)
+                return true;
+            else {
+                const std::vector<Symbol *> &funcs = iter->second;
+                for (int j = 0; j < (int)funcs.size(); ++j)
+                    matches->push_back(funcs[j]);
+            }
+        }
+    }
+    return matches ? (matches->size() > 0) : false;
 }
 
 
 Symbol *
 SymbolTable::LookupFunction(const char *name, const FunctionType *type) {
-    if (functions.find(name) == functions.end())
+    for (int i = (int)functions.size() - 1; i >= 0; --i) {
-        return NULL;
+        FunctionMapType &fm = *(functions[i]);
-
+        FunctionMapType::iterator iter = fm.find(name);
-    std::vector<Symbol *> &funcs = functions[name];
+        if (iter != fm.end()) {
-    for (unsigned int i = 0; i < funcs.size(); ++i)
+            std::vector<Symbol *> funcs = iter->second;
-        if (Type::Equal(funcs[i]->type, type))
+            for (int j = 0; j < (int)funcs.size(); ++j) {
-            return funcs[i];
+                if (Type::Equal(funcs[j]->type, type))
+                    return funcs[j];
+            }
+        }
+    }
     return NULL;
 }
 
@@ -232,19 +251,24 @@ SymbolTable::ClosestVariableOrFunctionMatch(const char *str) const {
     std::vector<std::string> matches[maxDelta+1];
 
     for (int i = 0; i < (int)variables.size(); ++i) {
-        std::vector<Symbol *> &sv = *(variables[i]);
+        const SymbolMapType &sv = *(variables[i]);
-        for (int j = 0; j < (int)sv.size(); ++j) {
+        SymbolMapType::const_iterator iter;
-            int dist = StringEditDistance(str, sv[j]->name, maxDelta+1);
+        for (iter = sv.begin(); iter != sv.end(); ++iter) {
+            const Symbol *sym = iter->second;
+            int dist = StringEditDistance(str, sym->name, maxDelta+1);
             if (dist <= maxDelta)
-                matches[dist].push_back(sv[j]->name);
+                matches[dist].push_back(sym->name);
         }
     }
 
-    std::map<std::string, std::vector<Symbol *> >::const_iterator iter;
+    for (int i = 0; i < (int)functions.size(); ++i) {
-    for (iter = functions.begin(); iter != functions.end(); ++iter) {
+        const FunctionMapType &fm = *(functions[i]);
-        int dist = StringEditDistance(str, iter->first, maxDelta+1);
+        FunctionMapType::const_iterator iter;
+        for (iter = fm.begin(); iter != fm.end(); ++iter) {
+            int dist = StringEditDistance(str, iter->first, maxDelta+1);
             if (dist <= maxDelta)
                 matches[dist].push_back(iter->first);
+        }
     }
 
     // Now, return the first entry of matches[] that is non-empty, if any.
@@ -308,29 +332,29 @@ void
 SymbolTable::Print() {
     int depth = 0;
     fprintf(stderr, "Variables:\n----------------\n");
-    std::vector<std::vector<Symbol *> *>::iterator liter = variables.begin();
+    for (int i = 0; i < (int)variables.size(); ++i) {
-    while (liter != variables.end()) {
+        SymbolMapType &sm = *(variables[i]);
-        fprintf(stderr, "%*c", depth, ' ');
+        SymbolMapType::iterator iter;
-        std::vector<Symbol *>::iterator siter = (*liter)->begin();
+        for (iter = sm.begin(); iter != sm.end(); ++iter) {
-        while (siter != (*liter)->end()) {
+            fprintf(stderr, "%*c", depth, ' ');
-            fprintf(stderr, "%s [%s]", (*siter)->name.c_str(), 
+            Symbol *sym = iter->second;
-                    (*siter)->type->GetString().c_str());
+            fprintf(stderr, "%s [%s]", sym->name.c_str(), 
-            ++siter;
+                    sym->type->GetString().c_str());
         }
-        ++liter;
         fprintf(stderr, "\n");
         depth += 4;
     }
 
     fprintf(stderr, "Functions:\n----------------\n");
-    std::map<std::string, std::vector<Symbol *> >::iterator fiter;
+    for (int i = 0; i < (int)functions.size(); ++i) {
-    fiter = functions.begin();
+        FunctionMapType::iterator fiter = functions[i]->begin();
-    while (fiter != functions.end()) {
+        while (fiter != functions[i]->end()) {
-        fprintf(stderr, "%s\n", fiter->first.c_str());
+            fprintf(stderr, "%s\n", fiter->first.c_str());
-        std::vector<Symbol *> &syms = fiter->second;
+            std::vector<Symbol *> &syms = fiter->second;
-        for (unsigned int i = 0; i < syms.size(); ++i)
+            for (unsigned int j = 0; j < syms.size(); ++j)
-            fprintf(stderr, "    %s\n", syms[i]->type->GetString().c_str());
+                fprintf(stderr, "    %s\n", syms[j]->type->GetString().c_str());
-        ++fiter;
+            ++fiter;
+        }
     }
 
     depth = 0;

sym.h

@@ -169,11 +169,11 @@ public:
     /** Looks for the function or functions with the given name in the
         symbol name.  If a function has been overloaded and multiple
         definitions are present for a given function name, all of them will
-        be returned and it's up the the caller to resolve which one (if
+        be returned in the provided vector and it's up the the caller to
-        any) to use.
+        resolve which one (if any) to use.  Returns true if any matches
-
+        were found. */
-        @return vector of Symbol pointers to functions with the given name. */
+    bool LookupFunction(const char *name, 
-    std::vector<Symbol *> *LookupFunction(const char *name);
+                        std::vector<Symbol *> *matches = NULL);
 
     /** Looks for a function with the given name and type
         in the symbol table.
@@ -248,24 +248,27 @@ private:
     std::vector<std::string> closestTypeMatch(const char *str, 
                                               bool structsVsEnums) const;
 
-    /** This member variable holds one \c vector of Symbol pointers for
+    /** This member variable holds one SymbolMap for each of the current
-        each of the current active scopes as the program is being parsed.
+        active scopes as the program is being parsed.  New maps are added
-        New vectors of symbols are added and removed from the end of the
+        and removed from the end of the main vector, so searches for
-        main vector, so searches for symbols start looking at the end of \c
+        symbols start looking at the end of \c variables and work
-        variables and work backwards.
+        backwards.
      */
-    std::vector<std::vector<Symbol *> *> variables;
+    typedef std::map<std::string, Symbol *> SymbolMapType;
-    /** Because there is no scoping for function symbols, functions are
+    std::vector<SymbolMapType *> variables;
-        represented with a single STL \c map from names to symbols.  A STL
+
-        \c vector is used to store the function symbols for a given name
+    /** Function declarations are also scoped., A STL \c vector is used to
-        since, due to function overloading, a name can have multiple
+        store the function symbols for a given name since, due to function
-        function symbols associated with it. */
+        overloading, a name can have multiple function symbols associated
-    std::map<std::string, std::vector<Symbol *> > functions;
+        with it. */
-    typedef std::map<std::string, const Type *> TypeMapType;
+    typedef std::map<std::string, std::vector<Symbol *> > FunctionMapType;
-    /** Like variables, type definitions can be scoped.  A new \c TypeMapType
+    std::vector<FunctionMapType *> functions;
+
+    /** Type definitions can also be scoped.  A new \c TypeMapType
         is added to the back of the \c types \c vector each time a new scope
         is entered.  (And it's removed when the scope exits).
      */
+    typedef std::map<std::string, const Type *> TypeMapType;
     std::vector<TypeMapType *> types;
 };
 
@@ -275,12 +278,15 @@ SymbolTable::GetMatchingFunctions(Predicate pred,
                                   std::vector<Symbol *> *matches) const {
     // Iterate through all function symbols and apply the given predicate.
     // If it returns true, add the Symbol * to the provided vector.
-    std::map<std::string, std::vector<Symbol *> >::const_iterator iter;
+    for (unsigned int i = 0; i < functions.size(); ++i) {
-    for (iter = functions.begin(); iter != functions.end(); ++iter) {
+        FunctionMapType &fm = *(functions[i]);
-        const std::vector<Symbol *> &syms = iter->second;
+        FunctionMapType::const_iterator iter;
-        for (unsigned int i = 0; i < syms.size(); ++i) {
+        for (iter = fm.begin(); iter != fm.end(); ++iter) {
-            if (pred(syms[i]))
+            const std::vector<Symbol *> &syms = iter->second;
-                matches->push_back(syms[i]);
+            for (unsigned int j = 0; j < syms.size(); ++j) {
+                if (pred(syms[j]))
+                    matches->push_back(syms[j]);
+            }
         }
     }
 }
@@ -289,10 +295,14 @@ SymbolTable::GetMatchingFunctions(Predicate pred,
 template <typename Predicate> void
 SymbolTable::GetMatchingVariables(Predicate pred, 
                                   std::vector<Symbol *> *matches) const {
-    for (unsigned int i = 0; i < variables.size(); ++i)
+    for (unsigned int i = 0; i < variables.size(); ++i) {
-        for (unsigned int j = 0; j < variables[i]->size(); ++j)
+        SymbolMapType &sm = *(variables[i]);
-            if (pred((*variables[i])[j]))
+        SymbolMapType::const_iterator iter;
-                matches->push_back((*variables[i])[j]);
+        for (iter = sm.begin(); iter != sm.end(); ++iter) {
+            if (pred(iter->second))
+                matches->push_back(iter->second);
+        }
+    }
 }
 
 #endif // ISPC_SYM_H