Skip to content

Commit

Permalink
Do not cache has_placeholder flag
Browse files Browse the repository at this point in the history 
Change implementation to always query the AST tree.
Fixes #2150
  • Loading branch information
@mgreter
mgreter committed Sep 24, 2016
1 parent b5ac616 commit e4a09c8
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 18 deletions.
50 changes: 37 additions & 13 deletions src/ast.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand All @@ -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;
}
Expand All @@ -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() { }

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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); }
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/debugger.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]": " -");
Expand Down Expand Up @@ -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]": " -")
Expand Down Expand Up @@ -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;
Expand All @@ -469,6 +474,7 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env)
Block* block = dynamic_cast<Block*>(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<Textual*>(node)) {
Expand Down
5 changes: 0 additions & 5 deletions src/parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e4a09c8

Please sign in to comment.