Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Commit

Permalink
Simplify string concatenation
Browse files Browse the repository at this point in the history 
Simply do `a + b + c`.

In C++ 11, if at least one of the operands is an rvalue reference, it'll
be basically as good as `a += b; b += c;`. Besides, all of these strings
are short.

Ideally we'd use string views and something like `absl::StrCat`
throughout, but we don't have anything like that here.
  • Loading branch information
@glebm @xzyfer
glebm authored and xzyfer committed Dec 20, 2018
1 parent 4f9f662 commit 663f94e
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 120 deletions.
44 changes: 43 additions & 1 deletion src/ast.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,48 @@ namespace Sass {

static Null sass_null(ParserState("null"));

const char* sass_op_to_name(enum Sass_OP op) {
switch (op) {
case AND: return "and";
case OR: return "or";
case EQ: return "eq";
case NEQ: return "neq";
case GT: return "gt";
case GTE: return "gte";
case LT: return "lt";
case LTE: return "lte";
case ADD: return "plus";
case SUB: return "minus";
case MUL: return "times";
case DIV: return "div";
case MOD: return "mod";
// this is only used internally!
case NUM_OPS: return "[OPS]";
default: return "invalid";
}
}

const char* sass_op_separator(enum Sass_OP op) {
switch (op) {
case AND: return "&&";
case OR: return "||";
case EQ: return "==";
case NEQ: return "!=";
case GT: return ">";
case GTE: return ">=";
case LT: return "<";
case LTE: return "<=";
case ADD: return "+";
case SUB: return "-";
case MUL: return "*";
case DIV: return "/";
case MOD: return "%";
// this is only used internally!
case NUM_OPS: return "[OPS]";
default: return "invalid";
}
}

/////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////

Expand Down Expand Up @@ -80,7 +122,7 @@ namespace Sass {
{
return false;
}

/////////////////////////////////////////////////////////////////////////
/////////////////////////////////////////////////////////////////////////

Expand Down
42 changes: 2 additions & 40 deletions src/ast.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,47 +69,9 @@ namespace Sass {
}
//////////////////////////////////////////////////////////

inline static const std::string sass_op_to_name(enum Sass_OP op) {
switch (op) {
case AND: return "and";
case OR: return "or";
case EQ: return "eq";
case NEQ: return "neq";
case GT: return "gt";
case GTE: return "gte";
case LT: return "lt";
case LTE: return "lte";
case ADD: return "plus";
case SUB: return "minus";
case MUL: return "times";
case DIV: return "div";
case MOD: return "mod";
// this is only used internally!
case NUM_OPS: return "[OPS]";
default: return "invalid";
}
}
const char* sass_op_to_name(enum Sass_OP op);

inline static const std::string sass_op_separator(enum Sass_OP op) {
switch (op) {
case AND: return "&&";
case OR: return "||";
case EQ: return "==";
case NEQ: return "!=";
case GT: return ">";
case GTE: return ">=";
case LT: return "<";
case LTE: return "<=";
case ADD: return "+";
case SUB: return "-";
case MUL: return "*";
case DIV: return "/";
case MOD: return "%";
// this is only used internally!
case NUM_OPS: return "[OPS]";
default: return "invalid";
}
}
const char* sass_op_separator(enum Sass_OP op);

//////////////////////////////////////////////////////////
// Abstract base class for all abstract syntax tree nodes.
Expand Down
76 changes: 25 additions & 51 deletions src/error_handling.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,35 +23,30 @@ namespace Sass {
InvalidParent::InvalidParent(Selector_Ptr parent, Backtraces traces, Selector_Ptr selector)
: Base(selector->pstate(), def_msg, traces), parent(parent), selector(selector)
{
msg = "Invalid parent selector for \"";
msg += selector->to_string(Sass_Inspect_Options());
msg += "\": \"";
msg += parent->to_string(Sass_Inspect_Options());
msg += "\"";
msg = "Invalid parent selector for "
"\"" + selector->to_string(Sass_Inspect_Options()) + "\": "
"\"" + parent->to_string(Sass_Inspect_Options()) + "\"";
}

InvalidVarKwdType::InvalidVarKwdType(ParserState pstate, Backtraces traces, std::string name, const Argument_Ptr arg)
: Base(pstate, def_msg, traces), name(name), arg(arg)
{
msg = "Variable keyword argument map must have string keys.\n";
msg += name + " is not a string in " + arg->to_string() + ".";
msg = "Variable keyword argument map must have string keys.\n" +
name + " is not a string in " + arg->to_string() + ".";
}

InvalidArgumentType::InvalidArgumentType(ParserState pstate, Backtraces traces, std::string fn, std::string arg, std::string type, const Value_Ptr value)
: Base(pstate, def_msg, traces), fn(fn), arg(arg), type(type), value(value)
{
msg = arg + ": \"";
msg = arg + ": \"";
if (value) msg += value->to_string(Sass_Inspect_Options());
msg += "\" is not a " + type;
msg += " for `" + fn + "'";
msg += "\" is not a " + type + " for `" + fn + "'";
}

MissingArgument::MissingArgument(ParserState pstate, Backtraces traces, std::string fn, std::string arg, std::string fntype)
: Base(pstate, def_msg, traces), fn(fn), arg(arg), fntype(fntype)
{
msg = fntype + " " + fn;
msg += " is missing argument ";
msg += arg + ".";
msg = fntype + " " + fn + " is missing argument " + arg + ".";
}

InvalidSyntax::InvalidSyntax(ParserState pstate, Backtraces traces, std::string msg)
Expand All @@ -65,87 +60,66 @@ namespace Sass {
DuplicateKeyError::DuplicateKeyError(Backtraces traces, const Map& dup, const Expression& org)
: Base(org.pstate(), def_msg, traces), dup(dup), org(org)
{
msg = "Duplicate key ";
msg += dup.get_duplicate_key()->inspect();
msg += " in map (";
msg += org.inspect();
msg += ").";
msg = "Duplicate key " + dup.get_duplicate_key()->inspect() + " in map (" + org.inspect() + ").";
}

TypeMismatch::TypeMismatch(Backtraces traces, const Expression& var, const std::string type)
: Base(var.pstate(), def_msg, traces), var(var), type(type)
{
msg = var.to_string();
msg += " is not an ";
msg += type;
msg += ".";
msg = var.to_string() + " is not an " + type + ".";
}

InvalidValue::InvalidValue(Backtraces traces, const Expression& val)
: Base(val.pstate(), def_msg, traces), val(val)
{
msg = val.to_string();
msg += " isn't a valid CSS value.";
msg = val.to_string() + " isn't a valid CSS value.";
}

StackError::StackError(Backtraces traces, const AST_Node& node)
: Base(node.pstate(), def_msg, traces), node(node)
{
msg = "stack level too deep";
msg = "stack level too deep";
}

IncompatibleUnits::IncompatibleUnits(const Units& lhs, const Units& rhs)
{
msg = "Incompatible units: '";
msg += rhs.unit();
msg += "' and '";
msg += lhs.unit();
msg += "'.";
msg = "Incompatible units: '" + rhs.unit() + "' and '" + lhs.unit() + "'.";
}

IncompatibleUnits::IncompatibleUnits(const UnitType lhs, const UnitType rhs)
{
msg = "Incompatible units: '";
msg += unit_to_string(rhs);
msg += "' and '";
msg += unit_to_string(lhs);
msg += "'.";
msg = std::string("Incompatible units: '") + unit_to_string(rhs) + "' and '" + unit_to_string(lhs) + "'.";
}

AlphaChannelsNotEqual::AlphaChannelsNotEqual(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, enum Sass_OP op)
: OperationError(), lhs(lhs), rhs(rhs), op(op)
{
msg = "Alpha channels must be equal: ";
msg += lhs->to_string({ NESTED, 5 });
msg += " " + sass_op_to_name(op) + " ";
msg += rhs->to_string({ NESTED, 5 });
msg += ".";
msg = "Alpha channels must be equal: " +
lhs->to_string({ NESTED, 5 }) +
" " + sass_op_to_name(op) + " " +
rhs->to_string({ NESTED, 5 }) + ".";
}

ZeroDivisionError::ZeroDivisionError(const Expression& lhs, const Expression& rhs)
: OperationError(), lhs(lhs), rhs(rhs)
{
msg = "divided by 0";
msg = "divided by 0";
}

UndefinedOperation::UndefinedOperation(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, enum Sass_OP op)
: OperationError(), lhs(lhs), rhs(rhs), op(op)
{
msg = def_op_msg + ": \"";
msg += lhs->to_string({ NESTED, 5 });
msg += " " + sass_op_to_name(op) + " ";
msg += rhs->to_string({ TO_SASS, 5 });
msg += "\".";
msg = def_op_msg + ": \"" +
lhs->to_string({ NESTED, 5 }) +
" " + sass_op_to_name(op) + " " +
rhs->to_string({ TO_SASS, 5 }) +
"\".";
}

InvalidNullOperation::InvalidNullOperation(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, enum Sass_OP op)
: UndefinedOperation(lhs, rhs, op)
{
msg = def_op_null_msg + ": \"";
msg += lhs->inspect();
msg += " " + sass_op_to_name(op) + " ";
msg += rhs->inspect();
msg += "\".";
msg = def_op_null_msg + ": \"" + lhs->inspect() + " " + sass_op_to_name(op) + " " + rhs->inspect() + "\".";
}

SassValueError::SassValueError(Backtraces traces, ParserState pstate, OperationError& err)
Expand Down
16 changes: 4 additions & 12 deletions src/fn_selectors.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,23 +119,15 @@ namespace Sass {

// Must be a simple sequence
if( childSeq->combinator() != Complex_Selector::Combinator::ANCESTOR_OF ) {
std::string msg("Can't append \"");
msg += childSeq->to_string();
msg += "\" to \"";
msg += parentSeqClone->to_string();
msg += "\" for `selector-append'";
error(msg, pstate, traces);
error("Can't append \"" + childSeq->to_string() + "\" to \"" +
parentSeqClone->to_string() + "\" for `selector-append'", pstate, traces);
}

// Cannot be a Universal selector
Type_Selector_Obj pType = Cast<Type_Selector>(childSeq->head()->first());
if(pType && pType->name() == "*") {
std::string msg("Can't append \"");
msg += childSeq->to_string();
msg += "\" to \"";
msg += parentSeqClone->to_string();
msg += "\" for `selector-append'";
error(msg, pstate, traces);
error("Can't append \"" + childSeq->to_string() + "\" to \"" +
parentSeqClone->to_string() + "\" for `selector-append'", pstate, traces);
}

// TODO: Add check for namespace stuff
Expand Down
8 changes: 1 addition & 7 deletions src/fn_utils.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,7 @@ namespace Sass {
{
T* val = Cast<T>(env[argname]);
if (!val) {
std::string msg("argument `");
msg += argname;
msg += "` of `";
msg += sig;
msg += "` must be a ";
msg += T::type_name();
error(msg, pstate, traces);
error("argument `" + argname + "` of `" + sig + "` must be a " + T::type_name(), pstate, traces);
}
return val;
}
Expand Down
5 changes: 2 additions & 3 deletions src/operation.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,8 @@ namespace Sass {
// will be called if not overloaded
template <typename U> T fallback(U x)
{
std::string msg(typeid(*this).name());
msg += ": CRTP not implemented for ";
throw std::runtime_error(msg + typeid(x).name());
throw std::runtime_error(
std::string(typeid(*this).name()) + ": CRTP not implemented for " + typeid(x).name());
}

};
Expand Down
12 changes: 6 additions & 6 deletions src/operators.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ namespace Sass {
/* colour math deprecation warning */
void op_color_deprecation(enum Sass_OP op, std::string lsh, std::string rhs, const ParserState& pstate)
{
std::string op_str(sass_op_to_name(op));

std::string msg("The operation `" + lsh + " " + op_str + " " + rhs + "` is deprecated and will be an error in future versions.");
std::string tail("Consider using Sass's color functions instead.\nhttp://sass-lang.com/documentation/Sass/Script/Functions.html#other_color_functions");

deprecated(msg, tail, false, pstate);
deprecated(
"The operation `" + lsh + " " + sass_op_to_name(op) + " " + rhs +
"` is deprecated and will be an error in future versions.",
"Consider using Sass's color functions instead.\n"
"http://sass-lang.com/documentation/Sass/Script/Functions.html#other_color_functions",
/*with_column=*/false, pstate);
}

/* static function, throws OperationError, has no traces but optional pstate for returned value */
Expand Down

0 comments on commit 663f94e

Please sign in to comment.