Cleanup ast containers

Author: mgreter

src/ast_callable.hpp

@@ -22,8 +22,8 @@ namespace Sass {
   typedef sass::vector<SassFnPair> SassFnPairs;
 
   /////////////////////////////////////////////////////////////////////////
+  // Base class for everything that can be called on demand.
   /////////////////////////////////////////////////////////////////////////
-
   class Callable : public AstNode
   {
   public:
@@ -46,13 +46,12 @@ namespace Sass {
     // Declare up-casting methods
     DECLARE_ISA_CASTER(BuiltInCallable);
     DECLARE_ISA_CASTER(BuiltInCallables);
-    DECLARE_ISA_CASTER(PlainCssCallable2);
     DECLARE_ISA_CASTER(UserDefinedCallable);
     DECLARE_ISA_CASTER(ExternalCallable);
   };
 
   /////////////////////////////////////////////////////////////////////////
-  // Individual argument objects for mixin and function calls.
+  // Individual argument object for function signatures.
   /////////////////////////////////////////////////////////////////////////
   class Argument final : public AstNode
   {
@@ -76,7 +75,7 @@ namespace Sass {
 
   //////////////////////////////////////////////////////////////////////
   // Object for the function signature holding which parameters a
-  // callable can have or expects and to which variable it's assigned.
+  // callable can have or expects, with optional rest arguments.
   //////////////////////////////////////////////////////////////////////
   class CallableSignature final : public AstNode
   {
@@ -86,7 +85,6 @@ namespace Sass {
     ADD_REF(sass::vector<ArgumentObj>, arguments);
 
     // The name of the rest argument (as in `$args...`),
-    // or `null` if none was declared.
     ADD_CONSTREF(EnvKey, restArg);
 
     // This is only used for debugging

src/ast_callables.cpp

@@ -22,12 +22,12 @@ namespace Sass {
 
   BuiltInCallable::BuiltInCallable(
     const EnvKey& envkey,
-    CallableSignature* parameters,
+    CallableSignature* signature,
     const SassFnSig& callback) :
     Callable(SourceSpan::internal("[BUILTIN]")),
     envkey_(envkey),
     // Create a single entry in overloaded function
-    function_(SassFnPair{ parameters, callback })
+    function_(SassFnPair{ signature, callback })
   {}
 
   // Return callback with matching signature
@@ -170,7 +170,7 @@ namespace Sass {
     }
   }
 
-  // Parse source into arguments
+  // Parse `source` into signature
   CallableSignature* CallableSignature::parse(
     Compiler& context, SourceData* source)
   {

src/ast_callables.hpp

@@ -1,6 +1,11 @@
 /*****************************************************************************/
 /* Part of LibSass, released under the MIT license (See LICENSE.txt).        */
 /*****************************************************************************/
+// This file was deliberately split from `ast_callable.hpp`. It contains
+// all the specialized implementations for callables. These often need
+// access to higher level classes, which poses include dependency issues
+// if everything would be defined in a single header file.
+/*****************************************************************************/
 #ifndef SASS_AST_CALLABLES_HPP
 #define SASS_AST_CALLABLES_HPP
 
@@ -18,15 +23,14 @@
 namespace Sass {
 
   /////////////////////////////////////////////////////////////////////////
+  // Internal callables provided by LibSass itself.
   /////////////////////////////////////////////////////////////////////////
-
   class BuiltInCallable final : public Callable {
 
-    // The function name
+    // Name of this callable/function
     ADD_CONSTREF(EnvKey, envkey);
 
-    ADD_CONSTREF(CallableSignatureObj, parameters);
-
+    // Pair of signature and callback
     ADD_REF(SassFnPair, function);
 
   public:
@@ -37,7 +41,7 @@ namespace Sass {
     // Throws a [SassFormatException] if parsing fails.
     BuiltInCallable(
       const EnvKey& fname,
-      CallableSignature* parameters,
+      CallableSignature* signature,
       const SassFnSig& callback);
 
     // Return callback with matching signature
@@ -53,15 +57,16 @@ namespace Sass {
     // Equality comparator (needed for `get-function` value)
     bool operator==(const Callable& rhs) const override final;
 
+    // Define isaBuiltInCallable up-cast function
     IMPLEMENT_ISA_CASTER(BuiltInCallable);
   };
 
   /////////////////////////////////////////////////////////////////////////
+  // Internal callable with multiple signatures to choose from.
   /////////////////////////////////////////////////////////////////////////
-
   class BuiltInCallables final : public Callable {
 
-    // The function name
+    // Name of this callable/function
     ADD_CONSTREF(EnvKey, envkey);
 
     // The overloads declared for this callable.
@@ -91,12 +96,13 @@ namespace Sass {
     // Equality comparator (needed for `get-function` value)
     bool operator==(const Callable& rhs) const override final;
 
+    // Define isaBuiltInCallables up-cast function
     IMPLEMENT_ISA_CASTER(BuiltInCallables);
   };
 
   /////////////////////////////////////////////////////////////////////////
+  // User defined callable from sass code (functions and mixins)
   /////////////////////////////////////////////////////////////////////////
-
   class UserDefinedCallable final : public Callable
   {
   private:
@@ -126,13 +132,13 @@ namespace Sass {
     // Equality comparator (needed for `get-function` value)
     bool operator==(const Callable& rhs) const override final;
 
+    // Define isaUserDefinedCallable up-cast function
     IMPLEMENT_ISA_CASTER(UserDefinedCallable);
   };
 
   /////////////////////////////////////////////////////////////////////////
+  // External callable defined on the C-API side.
   /////////////////////////////////////////////////////////////////////////
-
-  // ToDo: this could be struct SassFunction!?
   class ExternalCallable final : public Callable
   {
   private:
@@ -154,11 +160,6 @@ namespace Sass {
       CallableSignature* parameters,
       SassFunctionLambda function);
 
-    // Destructor
-    ~ExternalCallable() override final {
-      // sass_delete_function(function_);
-    }
-
     // The main entry point to execute the function (implemented in each specialization)
     Value* execute(Eval& eval, CallableArguments* arguments, const SourceSpan& pstate) override final;
 
@@ -168,6 +169,7 @@ namespace Sass {
     // Equality comparator (needed for `get-function` value)
     bool operator==(const Callable& rhs) const override final;
 
+    // Define isaExternalCallable up-cast function
     IMPLEMENT_ISA_CASTER(ExternalCallable);
   };
 

src/ast_containers.hpp

@@ -52,25 +52,34 @@ namespace Sass {
       if (!childless) elements_ = std::move(vec);
     }
 
-    // Allow destructor overloading
-    // virtual ~Vectorized() {};
-
     // Some simple method delegations
-    T& last() { return elements_.back(); }
-    T& first() { return elements_.front(); }
-    const T& last() const { return elements_.back(); }
-    const T& first() const { return elements_.front(); }
-    bool empty() const { return elements_.empty(); }
     void clear() { return elements_.clear(); }
     size_t size() const { return elements_.size(); }
+    bool empty() const { return elements_.empty(); }
+    const T& at(size_t i) const { return elements_.at(i); }
+    const T& get(size_t i) const { return elements_[i]; }
+    const T& last() const { return elements_.back(); }
+    const T& first() const { return elements_.front(); }
+
+    // Setter to ensure we reset hash value
+    void set(size_t i, const T& value) {
+      hash_ = 0; // reset hash
+      elements_.at(i) = value;
+    }
+
+    // Setter to ensure we reset hash value
+    void set_last(const T& value) {
+      hash_ = 0; // reset hash
+      elements_.back() = value;
+    }
 
     // Check underlying containers for equality
-    // Note: maybe we could gain some speed by checking
-    // the computed hash first, before doing full test?
     bool operator== (const Vectorized<V>& rhs) const
     {
       // Abort early if sizes do not match
       if (size() != rhs.size()) return false;
+      // Abort early if hashes exist and don't match
+      if (hash_ && rhs.hash_ && hash_ != rhs.hash_) return false;
       // Otherwise test each node for object equality in order
       return std::equal(begin(), end(), rhs.begin(), ObjEqualityFn<T>);
     }
@@ -81,15 +90,6 @@ namespace Sass {
       return !(*this == rhs);
     }
 
-    T& at(size_t i) { return elements_.at(i); }
-    T& get(size_t i) { return elements_[i]; }
-    T& operator[](size_t i) { return elements_[i]; }
-
-    const T& at(size_t i) const { return elements_.at(i); }
-    const T& get(size_t i) const { return elements_[i]; }
-    // ToDo: might insert am item? (update ordered list)
-    const T& operator[](size_t i) const { return elements_[i]; }
-
     // Implicitly get the sass::vector from our object
     // Makes the Vector directly assignable to sass::vector
     // You are responsible to make a copy if needed
@@ -102,13 +102,14 @@ namespace Sass {
     // You are responsible to make a copy if needed
     // Note: since this returns the real object, we can't
     // Note: guarantee that the hash will not get out of sync
-    sass::vector<T>& elements() { return elements_; }
+    sass::vector<T>& elements43() { return elements_; }
     const sass::vector<T>& elements() const { return elements_; }
 
     // Insert all items from compatible vector
     void concat(const sass::vector<T>& v)
     {
       if (v.empty()) return;
+      hash_ = 0; // reset hash
       elements_.insert(end(),
         v.begin(), v.end());
     }
@@ -117,6 +118,7 @@ namespace Sass {
     void concat(sass::vector<T>&& v)
     {
       if (v.empty()) return;
+      hash_ = 0; // reset hash
       elements_.insert(elements_.end(),
         std::make_move_iterator(v.begin()),
         std::make_move_iterator(v.end()));
@@ -133,27 +135,31 @@ namespace Sass {
     // Insert one item on the front
     void unshift(const T& element)
     {
+      hash_ = 0; // reset hash
       elements_.insert(begin(),
         std::copy(element));
     }
 
     // Insert one item on the front
     void unshift(T&& element)
     {
+      hash_ = 0; // reset hash
       elements_.insert(begin(),
         std::move(element));
     }
 
     // Remove and return item on the front
     T shift() {
       T head = first();
+      hash_ = 0; // reset hash
       elements_.erase(begin());
       return head;
     }
 
     // Remove and return item on the back
     T pop() {
       T tail = last();
+      hash_ = 0; // reset hash
       elements_.pop_back();
       return tail;
     }
@@ -162,13 +168,15 @@ namespace Sass {
     // ToDo: rename this to push?
     void append(const T& element)
     {
+      hash_ = 0; // reset hash
       elements_.emplace_back(element);
     }
 
     // Insert one item on the back
     // ToDo: rename this to push?
     void append(T&& element)
     {
+      hash_ = 0; // reset hash
       elements_.emplace_back(std::move(element));
     }
 
@@ -202,18 +210,6 @@ namespace Sass {
       return false;
     }
 
-    // This might be better implemented as `operator=`?
-    void elementsM(const sass::vector<T>& e)
-    {
-      elements_ = e;
-    }
-
-    // This might be better implemented as `operator=`?
-    void elementsM(sass::vector<T>&& e)
-    {
-      elements_ = std::move(e);
-    }
-
     template <typename P>
     typename sass::vector<T>::iterator insert(P position, const T& val) {
       return elements_.insert(position, val);
@@ -374,14 +370,14 @@ namespace Sass {
       return elements_;
     }
 
-    const sass::vector<K> keys() const {
+    sass::vector<K> keys() const {
       sass::vector<T> list;
       for (auto kv : elements_) {
         list.emplace_back(kv.first);
       }
       return list;
     }
-    const sass::vector<T> values() const {
+    sass::vector<T> values() const {
       sass::vector<T> list;
       for (auto kv : elements_) {
         list.emplace_back(kv.second);

src/ast_css.hpp

@@ -308,6 +308,7 @@ namespace Sass {
       return SASS_MEMORY_NEW_DBG(CssAtRule, this);
     }
 
+    // Define isaCssAtRule up-cast function
     IMPLEMENT_ISA_CASTER(CssAtRule);
   };
 
@@ -390,6 +391,7 @@ namespace Sass {
       return SASS_MEMORY_NEW_DBG(CssStyleRule, this, childless);
     }
 
+    // Define isaCssStyleRule up-cast function
     IMPLEMENT_ISA_CASTER(CssStyleRule);
   };
 
@@ -429,6 +431,7 @@ namespace Sass {
       return SASS_MEMORY_NEW_DBG(CssSupportsRule, this, childless);
     }
 
+    // Define isaCssSupportsRule up-cast function
     IMPLEMENT_ISA_CASTER(CssSupportsRule);
   };
 
@@ -539,6 +542,7 @@ namespace Sass {
       return SASS_MEMORY_NEW_DBG(CssMediaRule, this, childless);
     }
 
+    // Define isaCssMediaRule up-cast function
     IMPLEMENT_ISA_CASTER(CssMediaRule);
   };
   // EO CssMediaRule

src/ast_expressions.cpp

@@ -114,7 +114,7 @@ namespace Sass {
     return containsDoubleQuote ? $apos : $quote;
   }
 
-  PlainCssCallable2::PlainCssCallable2(
+  PlainCssFunction::PlainCssFunction(
     SourceSpan pstate,
     Interpolation* itpl,
     CallableArguments* args,