Implement optimized environment accessors

Use result from find whenever possible.
Avoids the need to fetch key bucket twice.

Author: mgreter

src/environment.cpp

@@ -53,12 +53,25 @@ namespace Sass {
   bool Environment<T>::has_local(const std::string& key) const
   { return local_frame_.find(key) != local_frame_.end(); }
 
+  template <typename T> EnvResult
+  Environment<T>::find_local(const std::string& key)
+  {
+    auto end = local_frame_.end();
+    auto it = local_frame_.find(key);
+    return EnvResult(it, it != end);
+  }
+
   template <typename T>
   T& Environment<T>::get_local(const std::string& key)
   { return local_frame_[key]; }
 
   template <typename T>
-  void Environment<T>::set_local(const std::string& key, T val)
+  void Environment<T>::set_local(const std::string& key, const T& val)
+  {
+    local_frame_[key] = val;
+  }
+  template <typename T>
+  void Environment<T>::set_local(const std::string& key, T&& val)
   {
     local_frame_[key] = val;
   }
@@ -86,7 +99,12 @@ namespace Sass {
   { return (*global_env())[key]; }
 
   template <typename T>
-  void Environment<T>::set_global(const std::string& key, T val)
+  void Environment<T>::set_global(const std::string& key, const T& val)
+  {
+    global_env()->local_frame_[key] = val;
+  }
+  template <typename T>
+  void Environment<T>::set_global(const std::string& key, T&& val)
   {
     global_env()->local_frame_[key] = val;
   }
@@ -126,13 +144,31 @@ namespace Sass {
   // either update already existing lexical value
   // or if flag is set, we create one if no lexical found
   template <typename T>
-  void Environment<T>::set_lexical(const std::string& key, T val)
+  void Environment<T>::set_lexical(const std::string& key, const T& val)
   {
-    auto cur = this;
+    Environment<T>* cur = this;
     bool shadow = false;
     while ((cur && cur->is_lexical()) || shadow) {
-      if (cur->has_local(key)) {
-        cur->set_local(key, val);
+      EnvResult rv(cur->find_local(key));
+      if (rv.found) {
+        rv.it->second = val;
+        return;
+      }
+      shadow = cur->is_shadow();
+      cur = cur->parent_;
+    }
+    set_local(key, val);
+  }
+  // this one moves the value
+  template <typename T>
+  void Environment<T>::set_lexical(const std::string& key, T&& val)
+  {
+    Environment<T>* cur = this;
+    bool shadow = false;
+    while ((cur && cur->is_lexical()) || shadow) {
+      EnvResult rv(cur->find_local(key));
+      if (rv.found) {
+        rv.it->second = val;
         return;
       }
       shadow = cur->is_shadow();
@@ -156,6 +192,20 @@ namespace Sass {
     return false;
   }
 
+  // look on the full stack for key
+  // include all scopes available
+  template <typename T> EnvResult
+  Environment<T>::find(const std::string& key)
+  {
+    auto cur = this;
+    while (true) {
+      EnvResult rv(cur->find_local(key));
+      if (rv.found) return rv;
+      cur = cur->parent_;
+      if (!cur) return rv;
+    }
+  };
+
   // use array access for getter and setter functions
   template <typename T>
   T& Environment<T>::operator[](const std::string& key)

src/environment.hpp

@@ -7,6 +7,17 @@
 
 namespace Sass {
 
+  typedef environment_map<std::string, AST_Node_Obj>::iterator EnvIter;
+
+  class EnvResult {
+    public:
+      EnvIter it;
+      bool found;
+    public:
+      EnvResult(EnvIter it, bool found)
+      : it(it), found(found) {}
+  };
+
   template <typename T>
   class Environment {
     // TODO: test with map
@@ -39,10 +50,13 @@ namespace Sass {
 
     bool has_local(const std::string& key) const;
 
+    EnvResult find_local(const std::string& key);
+
     T& get_local(const std::string& key);
 
     // set variable on the current frame
-    void set_local(const std::string& key, T val);
+    void set_local(const std::string& key, const T& val);
+    void set_local(const std::string& key, T&& val);
 
     void del_local(const std::string& key);
 
@@ -58,7 +72,8 @@ namespace Sass {
     T& get_global(const std::string& key);
 
     // set a variable on the global frame
-    void set_global(const std::string& key, T val);
+    void set_global(const std::string& key, const T& val);
+    void set_global(const std::string& key, T&& val);
 
     void del_global(const std::string& key);
 
@@ -70,12 +85,17 @@ namespace Sass {
     // see if we have a lexical we could update
     // either update already existing lexical value
     // or we create a new one on the current frame
-    void set_lexical(const std::string& key, T val);
+    void set_lexical(const std::string& key, T&& val);
+    void set_lexical(const std::string& key, const T& val);
 
     // look on the full stack for key
     // include all scopes available
     bool has(const std::string& key) const;
 
+    // look on the full stack for key
+    // include all scopes available
+    EnvResult find(const std::string& key);
+
     // use array access for getter and setter functions
     T& operator[](const std::string& key);
 

src/eval.cpp

@@ -989,27 +989,19 @@ namespace Sass {
 
   Expression_Ptr Eval::operator()(Variable_Ptr v)
   {
-    std::string name(v->name());
     Expression_Obj value = 0;
     Env* env = environment();
-    if (env->has(name)) {
-      value = Cast<Expression>((*env)[name]);
-    }
+    const std::string& name(v->name());
+    EnvResult rv(env->find(name));
+    if (rv.found) value = static_cast<Expression*>(rv.it->second.ptr());
     else error("Undefined variable: \"" + v->name() + "\".", v->pstate());
-    if (Argument* arg = Cast<Argument>(value)) {
-      value = arg->value();
-    }
-
-    // behave according to as ruby sass (add leading zero)
-    if (Number_Ptr nr = Cast<Number>(value)) {
-      nr->zero(true);
-    }
-
+    if (Argument_Ptr arg = Cast<Argument>(value)) value = arg->value();
+    if (Number_Ptr nr = Cast<Number>(value)) nr->zero(true); // force flag
     value->is_interpolant(v->is_interpolant());
     if (force) value->is_expanded(false);
     value->set_delayed(false); // verified
     value = value->perform(this);
-    if(!force) (*env)[name] = value;
+    if(!force) rv.it->second = value;
     return value.detach();
   }