From e4a09c8bc901d6526765a09fe26975031778d35e Mon Sep 17 00:00:00 2001 From: Marcel Greter Date: Sun, 25 Sep 2016 01:51:02 +0200 Subject: [PATCH] Do not cache `has_placeholder` flag Change implementation to always query the AST tree. Fixes https://github.com/sass/libsass/issues/2150 --- src/ast.hpp | 50 +++++++++++++++++++++++++++++++++++------------- src/debugger.hpp | 6 ++++++ src/parser.cpp | 5 ----- 3 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/ast.hpp b/src/ast.hpp index 0fdb927026..820d159fc3 100644 --- a/src/ast.hpp +++ b/src/ast.hpp @@ -1860,7 +1860,6 @@ namespace Sass { ///////////////////////////////////////// class Selector : public Expression { // ADD_PROPERTY(bool, has_reference) - ADD_PROPERTY(bool, has_placeholder) // line break before list separator ADD_PROPERTY(bool, has_line_feed) // line break after list separator @@ -1875,7 +1874,6 @@ namespace Sass { Selector(ParserState pstate, bool r = false, bool h = false) : Expression(pstate), // has_reference_(r), - has_placeholder_(h), has_line_feed_(false), has_line_break_(false), is_optional_(false), @@ -1884,9 +1882,6 @@ namespace Sass { { concrete_type(SELECTOR); } virtual ~Selector() = 0; virtual size_t hash() = 0; - virtual bool has_parent_ref() { - return false; - } virtual unsigned long specificity() { return 0; } @@ -1896,6 +1891,12 @@ namespace Sass { virtual bool has_wrapped_selector() { return false; } + virtual bool has_placeholder() { + return false; + } + virtual bool has_parent_ref() { + return false; + } }; inline Selector::~Selector() { } @@ -2032,11 +2033,14 @@ namespace Sass { public: Placeholder_Selector(ParserState pstate, std::string n) : Simple_Selector(pstate, n) - { has_placeholder(true); } + { } virtual unsigned long specificity() { return Constants::Specificity_Base; } + virtual bool has_placeholder() { + return true; + } // virtual Placeholder_Selector* find_placeholder(); virtual ~Placeholder_Selector() {}; ATTACH_OPERATIONS() @@ -2205,11 +2209,6 @@ namespace Sass { Wrapped_Selector(ParserState pstate, std::string n, Selector* sel) : Simple_Selector(pstate, n), selector_(sel) { } - virtual bool has_parent_ref() { - // if (has_reference()) return true; - if (!selector()) return false; - return selector()->has_parent_ref(); - } virtual bool is_superselector_of(Wrapped_Selector* sub); // Selectors inside the negation pseudo-class are counted like any // other, but the negation itself does not count as a pseudo-class. @@ -2221,6 +2220,11 @@ namespace Sass { } return hash_; } + virtual bool has_parent_ref() { + // if (has_reference()) return true; + if (!selector()) return false; + return selector()->has_parent_ref(); + } virtual bool has_wrapped_selector() { return true; @@ -2254,7 +2258,7 @@ namespace Sass { void adjust_after_pushing(Simple_Selector* s) { // if (s->has_reference()) has_reference(true); - if (s->has_placeholder()) has_placeholder(true); + // if (s->has_placeholder()) has_placeholder(true); } public: SimpleSequence_Selector(ParserState pstate, size_t s = 0) @@ -2319,6 +2323,15 @@ namespace Sass { return false; } + virtual bool has_placeholder() + { + if (length() == 0) return false; + if (Simple_Selector* ss = elements().front()) { + if (ss->has_placeholder()) return true; + } + return false; + } + bool is_empty_reference() { return length() == 1 && @@ -2371,7 +2384,7 @@ namespace Sass { reference_(r) { // if ((h && h->has_reference()) || (t && t->has_reference())) has_reference(true); - if ((h && h->has_placeholder()) || (t && t->has_placeholder())) has_placeholder(true); + // if ((h && h->has_placeholder()) || (t && t->has_placeholder())) has_placeholder(true); } virtual bool has_parent_ref(); @@ -2452,6 +2465,11 @@ namespace Sass { if (tail_ && tail_->has_wrapped_selector()) return true; return false; } + virtual bool has_placeholder() { + if (head_ && head_->has_placeholder()) return true; + if (tail_ && tail_->has_placeholder()) return true; + return false; + } bool operator<(const Sequence_Selector& rhs) const; bool operator==(const Sequence_Selector& rhs) const; inline bool operator!=(const Sequence_Selector& rhs) const { return !(*this == rhs); } @@ -2566,6 +2584,12 @@ namespace Sass { } return false; } + virtual bool has_placeholder() { + for (Sequence_Selector* cs : elements()) { + if (cs->has_placeholder()) return true; + } + return false; + } CommaSequence_Selector* clone(Context&) const; // does not clone SimpleSequence_Selector*s CommaSequence_Selector* cloneFully(Context&) const; // clones SimpleSequence_Selector*s virtual bool operator==(const Selector& rhs) const; diff --git a/src/debugger.hpp b/src/debugger.hpp index 4c71482ef8..6d10fd32ed 100644 --- a/src/debugger.hpp +++ b/src/debugger.hpp @@ -87,6 +87,8 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env) std::cerr << " (" << pstate_source_position(node) << ")"; std::cerr << " <" << selector->hash() << ">"; std::cerr << " [@media:" << selector->media_block() << "]"; + std::cerr << (selector->is_invisible() ? " [INVISIBLE]": " -"); + std::cerr << (selector->has_placeholder() ? " [PLACEHOLDER]": " -"); std::cerr << (selector->is_optional() ? " [is_optional]": " -"); std::cerr << (selector->has_parent_ref() ? " [has-parent]": " -"); std::cerr << (selector->has_line_break() ? " [line-break]": " -"); @@ -115,6 +117,8 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env) << " <" << selector->hash() << ">" << " [weight:" << longToHex(selector->specificity()) << "]" << " [@media:" << selector->media_block() << "]" + << (selector->is_invisible() ? " [INVISIBLE]": " -") + << (selector->has_placeholder() ? " [PLACEHOLDER]": " -") << (selector->is_optional() ? " [is_optional]": " -") << (selector->has_parent_ref() ? " [has parent]": " -") << (selector->has_line_feed() ? " [line-feed]": " -") @@ -460,6 +464,7 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env) std::cerr << ind << "Ruleset " << ruleset; std::cerr << " (" << pstate_source_position(node) << ")"; std::cerr << " [indent: " << ruleset->tabs() << "]"; + std::cerr << (ruleset->is_invisible() ? " [INVISIBLE]" : ""); std::cerr << (ruleset->at_root() ? " [@ROOT]" : ""); std::cerr << (ruleset->is_root() ? " [root]" : ""); std::cerr << std::endl; @@ -469,6 +474,7 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env) Block* block = dynamic_cast(node); std::cerr << ind << "Block " << block; std::cerr << " (" << pstate_source_position(node) << ")"; + std::cerr << (block->is_invisible() ? " [INVISIBLE]" : ""); std::cerr << " [indent: " << block->tabs() << "]" << std::endl; for(auto i : block->elements()) { debug_ast(i, ind + " ", env); } } else if (dynamic_cast(node)) { diff --git a/src/parser.cpp b/src/parser.cpp index 91049bb047..aae4b3e478 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -697,11 +697,6 @@ namespace Sass { if (!peek_css< class_char < complex_selector_delims > >()) { // parse next selector in sequence sel->tail(parse_complex_selector(true)); - if (sel->tail()) { - // ToDo: move this logic below into tail setter - // if (sel->tail()->has_reference()) sel->has_reference(true); - if (sel->tail()->has_placeholder()) sel->has_placeholder(true); - } } // add a parent selector if we are not in a root