Skip to content

Commit

Permalink
Implement correct operator precedence handling
Browse files Browse the repository at this point in the history 
Starting point to improve and refactor interpolations
and binary expressions to the correct implementations.
  • Loading branch information
@mgreter
mgreter committed Jan 5, 2016
1 parent f2ca46c commit f178af2
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 79 deletions.
9 changes: 8 additions & 1 deletion src/ast.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,14 @@ namespace Sass {
Binary_Expression(ParserState pstate,
Operand op, Expression* lhs, Expression* rhs)
: Expression(pstate), op_(op), left_(lhs), right_(rhs), hash_(0)
{ }
{
/*
is_interpolant(
(lhs && lhs->is_interpolant()) ||
(rhs && rhs->is_interpolant())
);
*/
}
const std::string type_name() {
switch (type()) {
case AND: return "and"; break;
Expand Down
2 changes: 2 additions & 0 deletions src/debugger.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,8 @@ inline void debug_ast(AST_Node* node, std::string ind, Env* env)
std::cerr << ind << "Binary_Expression " << expression;
std::cerr << " [interpolant: " << expression->is_interpolant() << "] ";
std::cerr << " [delayed: " << expression->is_delayed() << "] ";
std::cerr << " [ws_before: " << expression->op().ws_before << "] ";
std::cerr << " [ws_after: " << expression->op().ws_after << "] ";
std::cerr << " (" << pstate_source_position(node) << ")";
std::cerr << " [" << expression->type_name() << "]" << std::endl;
debug_ast(expression->left(), ind + " left: ", env);
Expand Down
146 changes: 104 additions & 42 deletions src/eval.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,33 +483,19 @@ namespace Sass {

Expression* Eval::operator()(Binary_Expression* b)
{
// debug_ast(b);
enum Sass_OP op_type = b->type();
// don't eval delayed expressions (the '/' when used as a separator)
if (op_type == Sass_OP::DIV && b->is_delayed()) return b;
b->is_delayed(false);
// if one of the operands is a '/' then make sure it's evaluated
Expression* lhs = b->left()->perform(this);
lhs->is_delayed(false);
while (typeid(*lhs) == typeid(Binary_Expression)) {
Binary_Expression* lhs_ex = static_cast<Binary_Expression*>(lhs);
if (lhs_ex->type() == Sass_OP::DIV && lhs_ex->is_delayed()) break;
lhs = Eval::operator()(lhs_ex);
if (op_type == Sass_OP::DIV && b->is_delayed()) {
b->right(b->right()->perform(this));
b->left(b->left()->perform(this));
return b;
}

switch (op_type) {
case Sass_OP::AND:
return *lhs ? b->right()->perform(this) : lhs;
break;

case Sass_OP::OR:
return *lhs ? lhs : b->right()->perform(this);
break;
// b->is_delayed(false);
Expression* lhs = b->left();
Expression* rhs = b->right();

default:
break;
}
// not a logical connective, so go ahead and eval the rhs
Expression* rhs = b->right()->perform(this);
// maybe fully evaluate structure
if (op_type == Sass_OP::EQ ||
op_type == Sass_OP::NEQ ||
Expand All @@ -518,19 +504,64 @@ namespace Sass {
op_type == Sass_OP::LT ||
op_type == Sass_OP::LTE)
{
if (String_Schema* schema = dynamic_cast<String_Schema*>(lhs)) {
if (schema->has_interpolants()) {
b->is_delayed(true);
}
}
if (String_Schema* schema = dynamic_cast<String_Schema*>(rhs)) {
if (schema->has_interpolants()) {
b->is_delayed(true);
}
}
//

lhs->is_expanded(false);
lhs->set_delayed(false);
lhs = lhs->perform(this);
lhs->is_expanded(false);
lhs->set_delayed(false);
lhs = lhs->perform(this);

rhs->is_expanded(false);
rhs->set_delayed(false);
rhs = rhs->perform(this);
rhs->is_expanded(false);
rhs->set_delayed(false);
rhs = rhs->perform(this);
/*
*/
}
else
{
// rhs->set_delayed(false);
// rhs = rhs->perform(this);
}

// if one of the operands is a '/' then make sure it's evaluated
lhs = lhs->perform(this);
lhs->is_delayed(false);
while (typeid(*lhs) == typeid(Binary_Expression)) {
Binary_Expression* lhs_ex = static_cast<Binary_Expression*>(lhs);
if (lhs_ex->type() == Sass_OP::DIV && lhs_ex->is_delayed()) break;
lhs = Eval::operator()(lhs_ex);
}

switch (op_type) {
case Sass_OP::AND:
return *lhs ? b->right()->perform(this) : lhs;
break;

case Sass_OP::OR:
return *lhs ? lhs : b->right()->perform(this);
break;

default:
break;
}
// not a logical connective, so go ahead and eval the rhs
rhs = rhs->perform(this);

// upgrade string to number if possible (issue #948)
if (op_type == Sass_OP::DIV || op_type == Sass_OP::MUL) {
if (String_Constant* str = dynamic_cast<String_Constant*>(rhs)) {
Expand All @@ -544,15 +575,16 @@ namespace Sass {
}

// see if it's a relational expression
switch(op_type) {
case Sass_OP::EQ: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), eq(lhs, rhs));
case Sass_OP::NEQ: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), !eq(lhs, rhs));
case Sass_OP::GT: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), !lt(lhs, rhs) && !eq(lhs, rhs));
case Sass_OP::GTE: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), !lt(lhs, rhs));
case Sass_OP::LT: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), lt(lhs, rhs));
case Sass_OP::LTE: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), lt(lhs, rhs) || eq(lhs, rhs));

default: break;
if (!b->is_delayed()) {
switch(op_type) {
case Sass_OP::EQ: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), eq(lhs, rhs));
case Sass_OP::NEQ: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), !eq(lhs, rhs));
case Sass_OP::GT: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), !lt(lhs, rhs) && !eq(lhs, rhs));
case Sass_OP::GTE: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), !lt(lhs, rhs));
case Sass_OP::LT: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), lt(lhs, rhs));
case Sass_OP::LTE: return SASS_MEMORY_NEW(ctx.mem, Boolean, b->pstate(), lt(lhs, rhs) || eq(lhs, rhs));
default: break;
}
}


Expand All @@ -571,7 +603,8 @@ namespace Sass {
if ((s1 && s1->has_interpolants()) || (s2 && s2->has_interpolants()))
{
// If possible upgrade LHS to a number
if (op_type == Sass_OP::DIV || op_type == Sass_OP::MUL || op_type == Sass_OP::MOD || op_type == Sass_OP::ADD || op_type == Sass_OP::SUB) {
if (op_type == Sass_OP::DIV || op_type == Sass_OP::MUL || op_type == Sass_OP::MOD || op_type == Sass_OP::ADD || op_type == Sass_OP::SUB ||
op_type == Sass_OP::EQ) {
if (String_Constant* str = dynamic_cast<String_Constant*>(lhs)) {
std::string value(str->value());
const char* start = value.c_str();
Expand Down Expand Up @@ -609,7 +642,7 @@ namespace Sass {
}
}

if ((!schema_op || delay_anyway) && l_type == Expression::NUMBER && r_type == Expression::NUMBER) {
if ((!schema_op || delay_anyway) && (l_type == Expression::NUMBER || r_type == Expression::NUMBER)) {
std::string str("");
str += v_l->to_string(compressed, precision);
if (b->op().ws_before && !delay_anyway) str += " ";
Expand Down Expand Up @@ -651,7 +684,7 @@ namespace Sass {
To_Value to_value(ctx, ctx.mem);
Value* v_l = dynamic_cast<Value*>(lhs->perform(&to_value));
Value* v_r = dynamic_cast<Value*>(rhs->perform(&to_value));
Value* ex = op_strings(ctx.mem, op_type, *v_l, *v_r, compressed, precision, &pstate);
Value* ex = op_strings(ctx.mem, b->op(), *v_l, *v_r, compressed, precision, &pstate);
if (String_Constant* str = dynamic_cast<String_Constant*>(ex))
{
if (str->concrete_type() == Expression::STRING)
Expand Down Expand Up @@ -773,7 +806,12 @@ namespace Sass {
// if it's user-defined, eval the body
if (body) result = body->perform(this);
// if it's native, invoke the underlying CPP function
else result = func(fn_env, *env, ctx, def->signature(), c->pstate(), backtrace());
else {
Sass_Output_Style style = ctx.c_options->output_style;
ctx.c_options->output_style = SASS_STYLE_COMPACT;
result = func(fn_env, *env, ctx, def->signature(), c->pstate(), backtrace());
ctx.c_options->output_style = style;
}
if (!result) error(std::string("Function ") + c->name() + " did not return a value", c->pstate());
exp.backtrace_stack.pop_back();
}
Expand Down Expand Up @@ -884,7 +922,10 @@ namespace Sass {

// std::cerr << "\ttype is now: " << typeid(*value).name() << std::endl << std::endl;
value->is_interpolant(v->is_interpolant());
return value;
// return value;
value->is_expanded(false);
// value->set_delayed(false);
return value->perform(this);
}

Expression* Eval::operator()(Textual* t)
Expand Down Expand Up @@ -1478,10 +1519,11 @@ namespace Sass {
l.a());
}

Value* Eval::op_strings(Memory_Manager& mem, enum Sass_OP op, Value& lhs, Value& rhs, bool compressed, int precision, ParserState* pstate)
Value* Eval::op_strings(Memory_Manager& mem, Sass::Operand operand, Value& lhs, Value& rhs, bool compressed, int precision, ParserState* pstate)
{
Expression::Concrete_Type ltype = lhs.concrete_type();
Expression::Concrete_Type rtype = rhs.concrete_type();
enum Sass_OP op = operand.operand;

String_Quoted* lqstr = dynamic_cast<String_Quoted*>(&lhs);
String_Quoted* rqstr = dynamic_cast<String_Quoted*>(&rhs);
Expand Down Expand Up @@ -1519,12 +1561,21 @@ namespace Sass {
const Color* c_r = name_to_color(rstr);
return op_number_color(mem, op, *n_l, *c_r, compressed, precision);
}
if (op == Sass_OP::MUL) error("invalid operands for multiplication", lhs.pstate());
if (op == Sass_OP::MOD) error("invalid operands for modulo", lhs.pstate());

// if (op == Sass_OP::MUL) error("invalid operands for multiplication", lhs.pstate());
// if (op == Sass_OP::MOD) error("invalid operands for modulo", lhs.pstate());
std::string sep;
switch (op) {
case Sass_OP::SUB: sep = "-"; break;
case Sass_OP::DIV: sep = "/"; break;
case Sass_OP::MUL: sep = "*"; break;
case Sass_OP::MOD: sep = "%"; break;
case Sass_OP::EQ: sep = "=="; break;
case Sass_OP::NEQ: sep = "!="; break;
case Sass_OP::LT: sep = "<"; break;
case Sass_OP::GT: sep = ">"; break;
case Sass_OP::LTE: sep = "<="; break;
case Sass_OP::GTE: sep = ">="; break;
default: break;
}
if (ltype == Expression::NULL_VAL) error("Invalid null operation: \"null plus "+quote(unquote(rstr), '"')+"\".", lhs.pstate());
Expand All @@ -1536,8 +1587,8 @@ namespace Sass {
lhs.to_string() + sep + rhs.to_string());
}

if ( (ltype == Expression::STRING || sep == "") &&
(sep != "/" || !rqstr || !rqstr->quote_mark())
if ( (sep == "") /* &&
(sep != "/" || !rqstr || !rqstr->quote_mark()) */
) {
char quote_mark = 0;
std::string unq(unquote(lstr + sep + rstr, &quote_mark, true));
Expand All @@ -1546,7 +1597,18 @@ namespace Sass {
}
return SASS_MEMORY_NEW(mem, String_Quoted, lhs.pstate(), lstr + sep + rstr);
}
return SASS_MEMORY_NEW(mem, String_Constant, lhs.pstate(), (lstr) + sep + quote(rstr));

if (op == Sass_OP::SUB || op == Sass_OP::DIV) {
if (rqstr && rqstr->quote_mark()) rstr = quote(rstr);
return SASS_MEMORY_NEW(mem, String_Constant, lhs.pstate(), lstr + sep + rstr);
}

if (sep != "") {
if (operand.ws_before) sep = " " + sep;
if (operand.ws_after) sep = sep + " ";
}

return SASS_MEMORY_NEW(mem, String_Constant, lhs.pstate(), (lstr) + sep + (rstr));
}

Expression* cval_to_astnode(Memory_Manager& mem, union Sass_Value* v, Context& ctx, Backtrace* backtrace, ParserState pstate)
Expand Down
2 changes: 1 addition & 1 deletion src/eval.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ namespace Sass {
static Value* op_number_color(Memory_Manager&, enum Sass_OP, const Number&, const Color&, bool compressed = false, int precision = 5, ParserState* pstate = 0);
static Value* op_color_number(Memory_Manager&, enum Sass_OP, const Color&, const Number&, bool compressed = false, int precision = 5, ParserState* pstate = 0);
static Value* op_colors(Memory_Manager&, enum Sass_OP, const Color&, const Color&, bool compressed = false, int precision = 5, ParserState* pstate = 0);
static Value* op_strings(Memory_Manager&, enum Sass_OP, Value&, Value&, bool compressed = false, int precision = 5, ParserState* pstate = 0);
static Value* op_strings(Memory_Manager&, Sass::Operand, Value&, Value&, bool compressed = false, int precision = 5, ParserState* pstate = 0);

private:
void interpolation(Context& ctx, std::string& res, Expression* ex, bool into_quotes, bool was_itpl = false);
Expand Down
Loading

0 comments on commit f178af2

Please sign in to comment.