Move the variable stack to a common class.

This Cl extracts the variable stack from the SPIR-V builder and makes it
available as a general class.

Change-Id: I3505d7cc4ec34f78bbc3360b064fa99479fd97d6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/18701
Reviewed-by: David Neto <dneto@google.com>

src/CMakeLists.txt

@@ -185,6 +185,7 @@ set(TINT_LIB_SRCS
   context.cc
   reader/reader.cc
   reader/reader.h
+  scope_stack.h
   source.h
   type_determiner.cc
   type_determiner.h
@@ -306,6 +307,7 @@ set(TINT_TEST_SRCS
   ast/unless_statement_test.cc
   ast/variable_decl_statement_test.cc
   ast/variable_test.cc
+  scope_stack_test.cc
   type_manager_test.cc
   validator_impl_import_test.cc
 )

src/scope_stack.h

@@ -0,0 +1,87 @@
+// Copyright 2020 The Tint Authors.  //
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SRC_SCOPE_STACK_H_
+#define SRC_SCOPE_STACK_H_
+
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+namespace tint {
+
+/// Used to store a stack of scope information.
+/// The stack starts with a global scope which can not be popped.
+template <class T>
+class ScopeStack {
+ public:
+  /// Constructor
+  ScopeStack() {
+    // Push global bucket
+    stack_.push_back({});
+  }
+  /// Copy Constructor
+  ScopeStack(const ScopeStack&) = default;
+  ~ScopeStack() = default;
+
+  /// Push a new scope on to the stack
+  void push_scope() { stack_.push_back({}); }
+
+  /// Pop the scope off the top of the stack
+  void pop_scope() {
+    if (stack_.size() > 1) {
+      stack_.pop_back();
+    }
+  }
+
+  /// Set a global variable in the stack
+  /// @param name the name of the variable
+  /// @param val the value
+  void set_global(const std::string& name, T val) { stack_[0][name] = val; }
+
+  /// Sets variable into the top most scope of the stack
+  /// @param name the name of the variable
+  /// @param val the value
+  void set(const std::string& name, T val) { stack_.back()[name] = val; }
+
+  /// Checks for the given |name| in the stack
+  /// @param name the name to look for
+  /// @returns true if the stack contains |name|
+  bool has(const std::string& name) const { return get(name, nullptr); }
+
+  /// Retrieves a given name from the stack
+  /// @param name the name to look for
+  /// @param ret where to place the name
+  /// @returns true if the name was successfully found, false otherwise
+  bool get(const std::string& name, T* ret) const {
+    for (auto iter = stack_.rbegin(); iter != stack_.rend(); ++iter) {
+      auto& map = *iter;
+
+      auto val = map.find(name);
+      if (val != map.end()) {
+        if (ret) {
+          *ret = val->second;
+        }
+        return true;
+      }
+    }
+    return false;
+  }
+
+ private:
+  std::vector<std::unordered_map<std::string, T>> stack_;
+};
+
+}  // namespace tint
+
+#endif  // SRC_SCOPE_STACK_H_

src/scope_stack_test.cc

@@ -0,0 +1,94 @@
+// Copyright 2020 The Tint Authors.  //
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/scope_stack.h"
+
+#include "gtest/gtest.h"
+#include "src/ast/variable.h"
+
+namespace tint {
+namespace {
+
+using ScopeStackTest = testing::Test;
+
+TEST_F(ScopeStackTest, Global) {
+  ScopeStack<uint32_t> s;
+  s.set_global("var", 5);
+
+  uint32_t val = 0;
+  EXPECT_TRUE(s.get("var", &val));
+  EXPECT_EQ(val, 5);
+}
+
+TEST_F(ScopeStackTest, Global_SetWithPointer) {
+  ast::Variable v;
+  v.set_name("my_var");
+
+  ScopeStack<ast::Variable*> s;
+  s.set_global("var", &v);
+
+  ast::Variable* v2;
+  EXPECT_TRUE(s.get("var", &v2));
+  EXPECT_EQ(v2->name(), "my_var");
+}
+
+TEST_F(ScopeStackTest, Global_CanNotPop) {
+  ScopeStack<uint32_t> s;
+  s.set_global("var", 5);
+  s.pop_scope();
+
+  uint32_t val = 0;
+  EXPECT_TRUE(s.get("var", &val));
+  EXPECT_EQ(val, 5);
+}
+
+TEST_F(ScopeStackTest, Scope) {
+  ScopeStack<uint32_t> s;
+  s.push_scope();
+  s.set("var", 5);
+
+  uint32_t val = 0;
+  EXPECT_TRUE(s.get("var", &val));
+  EXPECT_EQ(val, 5);
+}
+
+TEST_F(ScopeStackTest, Get_MissingName) {
+  ScopeStack<uint32_t> s;
+  uint32_t ret = 0;
+  EXPECT_FALSE(s.get("val", &ret));
+  EXPECT_EQ(ret, 0);
+}
+
+TEST_F(ScopeStackTest, Has) {
+  ScopeStack<uint32_t> s;
+  s.set_global("var2", 3);
+  s.push_scope();
+  s.set("var", 5);
+
+  EXPECT_TRUE(s.has("var"));
+  EXPECT_TRUE(s.has("var2"));
+}
+
+TEST_F(ScopeStackTest, ReturnsScopeBeforeGlobalFirst) {
+  ScopeStack<uint32_t> s;
+  s.set_global("var", 3);
+  s.push_scope();
+  s.set("var", 5);
+
+  uint32_t ret;
+  EXPECT_TRUE(s.get("var", &ret));
+  EXPECT_EQ(ret, 5);
+}
+
+}  // namespace
+}  // namespace tint

src/writer/spirv/builder.cc

@@ -78,10 +78,7 @@ uint32_t pipeline_stage_to_execution_model(ast::PipelineStage stage) {
 
 }  // namespace
 
-Builder::Builder() {
-  // Push the global variable map onto the stack
-  variable_stack_.push_back({});
-}
+Builder::Builder() : scope_stack_({}) {}
 
 Builder::~Builder() = default;
 
@@ -233,7 +230,7 @@ bool Builder::GenerateFunction(ast::Function* func) {
   std::vector<Instruction> params;
   push_function(Function{definition_inst, result_op(), std::move(params)});
 
-  variable_stack_.push_back({});
+  scope_stack_.push_scope();
 
   for (const auto& stmt : func->body()) {
     if (!GenerateStatement(stmt.get())) {
@@ -241,7 +238,7 @@ bool Builder::GenerateFunction(ast::Function* func) {
     }
   }
 
-  variable_stack_.pop_back();
+  scope_stack_.pop_scope();
 
   func_name_to_id_[func->name()] = func_id;
   return true;
@@ -282,7 +279,7 @@ bool Builder::GenerateFunctionVariable(ast::Variable* var) {
       error_ = "missing constructor for constant";
       return false;
     }
-    variable_stack_.back()[var->name()] = init_id;
+    scope_stack_.set(var->name(), init_id);
     return true;
   }
 
@@ -308,7 +305,7 @@ bool Builder::GenerateFunctionVariable(ast::Variable* var) {
                        {Operand::Int(var_id), Operand::Int(init_id)});
   }
 
-  variable_stack_.back()[var->name()] = var_id;
+  scope_stack_.set(var->name(), var_id);
 
   return true;
 }
@@ -333,7 +330,7 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
       error_ = "missing constructor for constant";
       return false;
     }
-    variable_stack_[0][var->name()] = init_id;
+    scope_stack_.set_global(var->name(), init_id);
     return true;
   }
 
@@ -386,26 +383,21 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
       }
     }
   }
-  variable_stack_[0][var->name()] = var_id;
+  scope_stack_.set_global(var->name(), var_id);
   return true;
 }
 
 uint32_t Builder::GenerateIdentifierExpression(
     ast::IdentifierExpression* expr) {
-  for (auto iter = variable_stack_.rbegin(); iter != variable_stack_.rend();
-       ++iter) {
-    auto& map = *iter;
-
   // TODO(dsinclair): handle names with namespaces in them ...
 
-    auto val = map.find(expr->name()[0]);
-    if (val != map.end()) {
-      return val->second;
-    }
-  }
-
+  uint32_t val = 0;
+  if (!scope_stack_.get(expr->name()[0], &val)) {
     error_ = "unable to find name for identifier: " + expr->name()[0];
     return 0;
+  }
+
+  return val;
 }
 
 void Builder::GenerateImport(ast::Import* imp) {

src/writer/spirv/builder.h

@@ -25,6 +25,7 @@
 #include "src/ast/literal.h"
 #include "src/ast/module.h"
 #include "src/ast/struct_member.h"
+#include "src/scope_stack.h"
 #include "src/writer/spirv/function.h"
 #include "src/writer/spirv/instruction.h"
 
@@ -256,7 +257,7 @@ class Builder {
   std::unordered_map<std::string, uint32_t> func_name_to_id_;
   std::unordered_map<std::string, uint32_t> type_name_to_id_;
   std::unordered_map<std::string, uint32_t> const_to_id_;
-  std::vector<std::unordered_map<std::string, uint32_t>> variable_stack_;
+  ScopeStack<uint32_t> scope_stack_;
 };
 
 }  // namespace spirv