Move Selector_Schema beneath Selector_List

This greatly simplifies the handling with selectors. We
always store a `Selector_List` and handle interpolation
as needed. Selectors are evaluated in expand, which might
be in a for loop. Therefore we either eval the list or
the schema to get the result (caching might be possible).

Author: mgreter

src/ast.cpp

@@ -2,6 +2,7 @@
 #include "ast.hpp"
 #include "context.hpp"
 #include "node.hpp"
+#include "eval.hpp"
 #include "extend.hpp"
 #include "emitter.hpp"
 #include "color_maps.hpp"
@@ -1185,7 +1186,14 @@ namespace Sass {
       }
     }
 
+  }
 
+  Selector_List_Obj Selector_List::eval(Eval& eval)
+  {
+    Selector_List_Obj list = schema() ?
+      eval(schema()) : eval(this);
+    list->schema(schema());
+    return list;
   }
 
   Selector_List_Ptr Selector_List::resolve_parent_refs(Context& ctx, std::vector<Selector_List_Obj>& pstack, bool implicit_parent)
@@ -1472,6 +1480,30 @@ namespace Sass {
     }
   }
 
+  size_t Wrapped_Selector::hash()
+  {
+    if (hash_ == 0) {
+      hash_combine(hash_, Simple_Selector::hash());
+      if (selector_) hash_combine(hash_, selector_->hash());
+    }
+    return hash_;
+  }
+  bool Wrapped_Selector::has_parent_ref() {
+    // if (has_reference()) return true;
+    if (!selector()) return false;
+    return selector()->has_parent_ref();
+  }
+  bool Wrapped_Selector::has_real_parent_ref() {
+    // if (has_reference()) return true;
+    if (!selector()) return false;
+    return selector()->has_real_parent_ref();
+  }
+  unsigned long Wrapped_Selector::specificity() const
+  {
+    return selector_ ? selector_->specificity() : 0;
+  }
+
+
   bool Selector_List::has_parent_ref()
   {
     for (Complex_Selector_Obj s : elements()) {

src/ast.hpp

@@ -521,10 +521,10 @@ namespace Sass {
   // of style declarations.
   /////////////////////////////////////////////////////////////////////////////
   class Ruleset : public Has_Block {
-    ADD_PROPERTY(Selector_Obj, selector)
+    ADD_PROPERTY(Selector_List_Obj, selector)
     ADD_PROPERTY(bool, is_root);
   public:
-    Ruleset(ParserState pstate, Selector_Obj s = 0, Block_Obj b = 0)
+    Ruleset(ParserState pstate, Selector_List_Obj s = 0, Block_Obj b = 0)
     : Has_Block(pstate, b), selector_(s), is_root_(false)
     { statement_type(RULESET); }
     Ruleset(const Ruleset* ptr)
@@ -598,10 +598,10 @@ namespace Sass {
   ///////////////////////////////////////////////////////////////////////
   class Directive : public Has_Block {
     ADD_CONSTREF(std::string, keyword)
-    ADD_PROPERTY(Selector_Obj, selector)
+    ADD_PROPERTY(Selector_List_Obj, selector)
     ADD_PROPERTY(Expression_Obj, value)
   public:
-    Directive(ParserState pstate, std::string kwd, Selector_Obj sel = 0, Block_Obj b = 0, Expression_Obj val = 0)
+    Directive(ParserState pstate, std::string kwd, Selector_List_Obj sel = 0, Block_Obj b = 0, Expression_Obj val = 0)
     : Has_Block(pstate, b), keyword_(kwd), selector_(sel), value_(val) // set value manually if needed
     { statement_type(DIRECTIVE); }
     Directive(const Directive* ptr)
@@ -633,7 +633,7 @@ namespace Sass {
   class Keyframe_Rule : public Has_Block {
     // according to css spec, this should be <keyframes-name>
     // <keyframes-name> = <custom-ident> | <string>
-    ADD_PROPERTY(Selector_Obj, name)
+    ADD_PROPERTY(Selector_List_Obj, name)
   public:
     Keyframe_Rule(ParserState pstate, Block_Obj b)
     : Has_Block(pstate, b), name_()
@@ -911,9 +911,9 @@ namespace Sass {
   // The Sass `@extend` directive.
   ////////////////////////////////
   class Extension : public Statement {
-    ADD_PROPERTY(Selector_Obj, selector)
+    ADD_PROPERTY(Selector_List_Obj, selector)
   public:
-    Extension(ParserState pstate, Selector_Obj s)
+    Extension(ParserState pstate, Selector_List_Obj s)
     : Statement(pstate), selector_(s)
     { statement_type(EXTEND); }
     Extension(const Extension* ptr)
@@ -2313,19 +2313,26 @@ namespace Sass {
   // Interpolated selectors -- the interpolated String will be expanded and
   // re-parsed into a normal selector class.
   /////////////////////////////////////////////////////////////////////////
-  class Selector_Schema : public Selector {
+  class Selector_Schema : public AST_Node {
     ADD_PROPERTY(String_Obj, contents)
     ADD_PROPERTY(bool, connect_parent);
+    // must not be a reference counted object
+    // otherwise we create circular references
+    ADD_PROPERTY(Media_Block_Ptr, media_block)
+    // store computed hash
+    size_t hash_;
   public:
     Selector_Schema(ParserState pstate, String_Obj c)
-    : Selector(pstate),
+    : AST_Node(pstate),
       contents_(c),
-      connect_parent_(true)
+      connect_parent_(true),
+      media_block_(NULL)
     { }
     Selector_Schema(const Selector_Schema* ptr)
-    : Selector(ptr),
+    : AST_Node(ptr),
       contents_(ptr->contents_),
-      connect_parent_(ptr->connect_parent_)
+      connect_parent_(ptr->connect_parent_),
+      media_block_(ptr->media_block_)
     { }
     virtual bool has_parent_ref();
     virtual bool has_real_parent_ref();
@@ -2657,9 +2664,9 @@ namespace Sass {
   // Wrapped selector -- pseudo selector that takes a list of selectors as argument(s) e.g., :not(:first-of-type), :-moz-any(ol p.blah, ul, menu, dir)
   /////////////////////////////////////////////////
   class Wrapped_Selector : public Simple_Selector {
-    ADD_PROPERTY(Selector_Obj, selector)
+    ADD_PROPERTY(Selector_List_Obj, selector)
   public:
-    Wrapped_Selector(ParserState pstate, std::string n, Selector_Obj sel)
+    Wrapped_Selector(ParserState pstate, std::string n, Selector_List_Obj sel)
     : Simple_Selector(pstate, n), selector_(sel)
     { simple_type(WRAPPED_SEL); }
     Wrapped_Selector(const Wrapped_Selector* ptr)
@@ -2668,28 +2675,10 @@ namespace Sass {
     virtual bool is_superselector_of(Wrapped_Selector_Obj sub);
     // Selectors inside the negation pseudo-class are counted like any
     // other, but the negation itself does not count as a pseudo-class.
-    virtual size_t hash()
-    {
-      if (hash_ == 0) {
-        hash_combine(hash_, Simple_Selector::hash());
-        if (selector_) hash_combine(hash_, selector_->hash());
-      }
-      return hash_;
-    }
-    virtual bool has_parent_ref() {
-      // if (has_reference()) return true;
-      if (!selector()) return false;
-      return selector()->has_parent_ref();
-    }
-    virtual bool has_real_parent_ref() {
-      // if (has_reference()) return true;
-      if (!selector()) return false;
-      return selector()->has_real_parent_ref();
-    }
-    virtual unsigned long specificity() const
-    {
-      return selector_ ? selector_->specificity() : 0;
-    }
+    virtual size_t hash();
+    virtual bool has_parent_ref();
+    virtual bool has_real_parent_ref();
+    virtual unsigned long specificity() const;
     virtual bool operator==(const Simple_Selector& rhs) const;
     virtual bool operator==(const Wrapped_Selector& rhs) const;
     virtual bool operator<(const Simple_Selector& rhs) const;
@@ -2971,16 +2960,21 @@ namespace Sass {
   // Comma-separated selector groups.
   ///////////////////////////////////
   class Selector_List : public Selector, public Vectorized<Complex_Selector_Obj> {
+    ADD_PROPERTY(Selector_Schema_Obj, schema)
     ADD_CONSTREF(std::vector<std::string>, wspace)
   protected:
     void adjust_after_pushing(Complex_Selector_Obj c);
   public:
     Selector_List(ParserState pstate, size_t s = 0)
-    : Selector(pstate), Vectorized<Complex_Selector_Obj>(s), wspace_(0)
+    : Selector(pstate),
+      Vectorized<Complex_Selector_Obj>(s),
+      schema_(NULL),
+      wspace_(0)
     { }
     Selector_List(const Selector_List* ptr)
     : Selector(ptr),
       Vectorized<Complex_Selector_Obj>(*ptr),
+      schema_(ptr->schema_),
       wspace_(ptr->wspace_)
     { }
     std::string type() { return "list"; }
@@ -2995,6 +2989,7 @@ namespace Sass {
     virtual bool is_superselector_of(Selector_List_Obj sub, std::string wrapping = "");
     Selector_List_Ptr unify_with(Selector_List_Ptr, Context&);
     void populate_extends(Selector_List_Obj, Context&, Subset_Map&);
+    Selector_List_Obj eval(Eval& eval);
     virtual size_t hash()
     {
       if (Selector::hash_ == 0) {

src/debugger.hpp

@@ -103,6 +103,7 @@ inline void debug_ast(AST_Node_Ptr node, std::string ind, Env* env)
     std::cerr << (selector->has_line_break() ? " [line-break]": " -");
     std::cerr << (selector->has_line_feed() ? " [line-feed]": " -");
     std::cerr << std::endl;
+    debug_ast(selector->schema(), "#{} ");
 
     for(const Complex_Selector_Obj& i : selector->elements()) { debug_ast(i, ind + " ", env); }
 
@@ -261,8 +262,6 @@ inline void debug_ast(AST_Node_Ptr node, std::string ind, Env* env)
     std::cerr << " (" << pstate_source_position(node) << ")"
       << " [@media:" << selector->media_block() << "]"
       << (selector->connect_parent() ? " [connect-parent]": " -")
-      << (selector->has_line_break() ? " [line-break]": " -")
-      << (selector->has_line_feed() ? " [line-feed]": " -")
     << std::endl;
 
     debug_ast(selector->contents(), ind + " ");

src/expand.cpp

@@ -94,9 +94,11 @@ namespace Sass {
       Block_Ptr bb = operator()(r->block());
       Keyframe_Rule_Obj k = SASS_MEMORY_NEW(Keyframe_Rule, r->pstate(), bb);
       if (r->selector()) {
-        selector_stack.push_back(0);
-        k->name(Cast<Selector_List>(r->selector()->perform(&eval)));
-        selector_stack.pop_back();
+        if (Selector_List_Ptr s = r->selector()) {
+          selector_stack.push_back(0);
+          k->name(s->eval(eval));
+          selector_stack.pop_back();
+        }
       }
       return k.detach();
     }
@@ -111,10 +113,8 @@ namespace Sass {
       has_parent_selector = ll != 0 && ll->length() > 0;
     }
 
-    Expression_Obj ex = 0;
-    if (r->selector()) ex = r->selector()->perform(&eval);
-    Selector_List_Obj sel = Cast<Selector_List>(ex);
-    if (sel == 0) throw std::runtime_error("Expanded null selector");
+    Selector_List_Obj sel = r->selector();
+    if (sel) sel = sel->eval(eval);
 
     // check for parent selectors in base level rules
     if (r->is_root()) {
@@ -219,11 +219,11 @@ namespace Sass {
   {
     LOCAL_FLAG(in_keyframes, a->is_keyframes());
     Block_Ptr ab = a->block();
-    Selector_Ptr as = a->selector();
+    Selector_List_Ptr as = a->selector();
     Expression_Ptr av = a->value();
     selector_stack.push_back(0);
     if (av) av = av->perform(&eval);
-    if (as) as = Cast<Selector>(as->perform(&eval));
+    if (as) as = eval(as);
     selector_stack.pop_back();
     Block_Ptr bb = ab ? operator()(ab) : NULL;
     Directive_Ptr aa = SASS_MEMORY_NEW(Directive,
@@ -626,28 +626,23 @@ namespace Sass {
 
   Statement* Expand::operator()(Extension_Ptr e)
   {
-    if (Selector_List_Obj extender = Cast<Selector_List>(selector())) {
-      Selector_Obj s = e->selector();
-      Selector_List_Obj sl = NULL;
-      // check if we already have a valid selector list
-      if ((sl = Cast<Selector_List>(s))) {}
-      // convert selector schema to a selector list
-      else if (Selector_Schema_Obj schema = Cast<Selector_Schema>(s)) {
+    if (Selector_List_Ptr extender = selector()) {
+      Selector_List_Ptr sl = e->selector();
+      // abort on invalid selector
+      if (sl == NULL) return NULL;
+      if (Selector_Schema_Ptr schema = sl->schema()) {
         if (schema->has_real_parent_ref()) {
           // put root block on stack again (ignore parents)
           // selector schema must not connect in eval!
           block_stack.push_back(block_stack.at(1));
-          sl = eval(schema);
+          sl = eval(sl->schema());
           block_stack.pop_back();
         } else {
           selector_stack.push_back(0);
-          sl = eval(schema);
-          sl->remove_parent_selectors();
+          sl = eval(sl->schema());
           selector_stack.pop_back();
         }
       }
-      // abort on invalid selector
-      if (sl.isNull()) return NULL;
       for (Complex_Selector_Obj cs : sl->elements()) {
         if (!cs.isNull() && !cs->head().isNull()) {
           cs->head()->media_block(media_block_stack.back());

src/parser.cpp

@@ -248,9 +248,15 @@ namespace Sass {
     else if (lex < kwd_extend >(true)) {
       Lookahead lookahead = lookahead_for_include(position);
       if (!lookahead.found) css_error("Invalid CSS", " after ", ": expected selector, was ");
-      Selector_Obj target;
-      if (lookahead.has_interpolants) target = parse_selector_schema(lookahead.found, true);
-      else                            target = parse_selector_list(true);
+      Selector_List_Obj target;
+      if (!lookahead.has_interpolants) {
+        target = parse_selector_list(true);
+      }
+      else {
+        target = SASS_MEMORY_NEW(Selector_List, pstate);
+        target->schema(parse_selector_schema(lookahead.found, true));
+      }
+
       block->append(SASS_MEMORY_NEW(Extension, pstate, target));
     }
 
@@ -496,7 +502,11 @@ namespace Sass {
     Ruleset_Obj ruleset = SASS_MEMORY_NEW(Ruleset, pstate);
     // parse selector static or as schema to be evaluated later
     if (lookahead.parsable) ruleset->selector(parse_selector_list(false));
-    else ruleset->selector(parse_selector_schema(lookahead.found, false));
+    else {
+      Selector_List_Obj list = SASS_MEMORY_NEW(Selector_List, pstate);
+      list->schema(parse_selector_schema(lookahead.found, false));
+      ruleset->selector(list);
+    }
     // then parse the inner block
     stack.push_back(Scope::Rules);
     ruleset->block(parse_block());