Skip to content

Commit 663f94e

Browse files
glebmxzyfer
authored andcommitted
Simplify string concatenation
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.
1 parent 4f9f662 commit 663f94e

File tree

7 files changed

+83
-120
lines changed

7 files changed

+83
-120
lines changed

src/ast.cpp

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,48 @@ namespace Sass {
1919

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

22+
const char* sass_op_to_name(enum Sass_OP op) {
23+
switch (op) {
24+
case AND: return "and";
25+
case OR: return "or";
26+
case EQ: return "eq";
27+
case NEQ: return "neq";
28+
case GT: return "gt";
29+
case GTE: return "gte";
30+
case LT: return "lt";
31+
case LTE: return "lte";
32+
case ADD: return "plus";
33+
case SUB: return "minus";
34+
case MUL: return "times";
35+
case DIV: return "div";
36+
case MOD: return "mod";
37+
// this is only used internally!
38+
case NUM_OPS: return "[OPS]";
39+
default: return "invalid";
40+
}
41+
}
42+
43+
const char* sass_op_separator(enum Sass_OP op) {
44+
switch (op) {
45+
case AND: return "&&";
46+
case OR: return "||";
47+
case EQ: return "==";
48+
case NEQ: return "!=";
49+
case GT: return ">";
50+
case GTE: return ">=";
51+
case LT: return "<";
52+
case LTE: return "<=";
53+
case ADD: return "+";
54+
case SUB: return "-";
55+
case MUL: return "*";
56+
case DIV: return "/";
57+
case MOD: return "%";
58+
// this is only used internally!
59+
case NUM_OPS: return "[OPS]";
60+
default: return "invalid";
61+
}
62+
}
63+
2264
/////////////////////////////////////////////////////////////////////////
2365
/////////////////////////////////////////////////////////////////////////
2466

@@ -80,7 +122,7 @@ namespace Sass {
80122
{
81123
return false;
82124
}
83-
125+
84126
/////////////////////////////////////////////////////////////////////////
85127
/////////////////////////////////////////////////////////////////////////
86128

src/ast.hpp

Lines changed: 2 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -69,47 +69,9 @@ namespace Sass {
6969
}
7070
//////////////////////////////////////////////////////////
7171

72-
inline static const std::string sass_op_to_name(enum Sass_OP op) {
73-
switch (op) {
74-
case AND: return "and";
75-
case OR: return "or";
76-
case EQ: return "eq";
77-
case NEQ: return "neq";
78-
case GT: return "gt";
79-
case GTE: return "gte";
80-
case LT: return "lt";
81-
case LTE: return "lte";
82-
case ADD: return "plus";
83-
case SUB: return "minus";
84-
case MUL: return "times";
85-
case DIV: return "div";
86-
case MOD: return "mod";
87-
// this is only used internally!
88-
case NUM_OPS: return "[OPS]";
89-
default: return "invalid";
90-
}
91-
}
72+
const char* sass_op_to_name(enum Sass_OP op);
9273

93-
inline static const std::string sass_op_separator(enum Sass_OP op) {
94-
switch (op) {
95-
case AND: return "&&";
96-
case OR: return "||";
97-
case EQ: return "==";
98-
case NEQ: return "!=";
99-
case GT: return ">";
100-
case GTE: return ">=";
101-
case LT: return "<";
102-
case LTE: return "<=";
103-
case ADD: return "+";
104-
case SUB: return "-";
105-
case MUL: return "*";
106-
case DIV: return "/";
107-
case MOD: return "%";
108-
// this is only used internally!
109-
case NUM_OPS: return "[OPS]";
110-
default: return "invalid";
111-
}
112-
}
74+
const char* sass_op_separator(enum Sass_OP op);
11375

11476
//////////////////////////////////////////////////////////
11577
// Abstract base class for all abstract syntax tree nodes.

src/error_handling.cpp

Lines changed: 25 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -23,35 +23,30 @@ namespace Sass {
2323
InvalidParent::InvalidParent(Selector_Ptr parent, Backtraces traces, Selector_Ptr selector)
2424
: Base(selector->pstate(), def_msg, traces), parent(parent), selector(selector)
2525
{
26-
msg = "Invalid parent selector for \"";
27-
msg += selector->to_string(Sass_Inspect_Options());
28-
msg += "\": \"";
29-
msg += parent->to_string(Sass_Inspect_Options());
30-
msg += "\"";
26+
msg = "Invalid parent selector for "
27+
"\"" + selector->to_string(Sass_Inspect_Options()) + "\": "
28+
"\"" + parent->to_string(Sass_Inspect_Options()) + "\"";
3129
}
3230

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

4038
InvalidArgumentType::InvalidArgumentType(ParserState pstate, Backtraces traces, std::string fn, std::string arg, std::string type, const Value_Ptr value)
4139
: Base(pstate, def_msg, traces), fn(fn), arg(arg), type(type), value(value)
4240
{
43-
msg = arg + ": \"";
41+
msg = arg + ": \"";
4442
if (value) msg += value->to_string(Sass_Inspect_Options());
45-
msg += "\" is not a " + type;
46-
msg += " for `" + fn + "'";
43+
msg += "\" is not a " + type + " for `" + fn + "'";
4744
}
4845

4946
MissingArgument::MissingArgument(ParserState pstate, Backtraces traces, std::string fn, std::string arg, std::string fntype)
5047
: Base(pstate, def_msg, traces), fn(fn), arg(arg), fntype(fntype)
5148
{
52-
msg = fntype + " " + fn;
53-
msg += " is missing argument ";
54-
msg += arg + ".";
49+
msg = fntype + " " + fn + " is missing argument " + arg + ".";
5550
}
5651

5752
InvalidSyntax::InvalidSyntax(ParserState pstate, Backtraces traces, std::string msg)
@@ -65,87 +60,66 @@ namespace Sass {
6560
DuplicateKeyError::DuplicateKeyError(Backtraces traces, const Map& dup, const Expression& org)
6661
: Base(org.pstate(), def_msg, traces), dup(dup), org(org)
6762
{
68-
msg = "Duplicate key ";
69-
msg += dup.get_duplicate_key()->inspect();
70-
msg += " in map (";
71-
msg += org.inspect();
72-
msg += ").";
63+
msg = "Duplicate key " + dup.get_duplicate_key()->inspect() + " in map (" + org.inspect() + ").";
7364
}
7465

7566
TypeMismatch::TypeMismatch(Backtraces traces, const Expression& var, const std::string type)
7667
: Base(var.pstate(), def_msg, traces), var(var), type(type)
7768
{
78-
msg = var.to_string();
79-
msg += " is not an ";
80-
msg += type;
81-
msg += ".";
69+
msg = var.to_string() + " is not an " + type + ".";
8270
}
8371

8472
InvalidValue::InvalidValue(Backtraces traces, const Expression& val)
8573
: Base(val.pstate(), def_msg, traces), val(val)
8674
{
87-
msg = val.to_string();
88-
msg += " isn't a valid CSS value.";
75+
msg = val.to_string() + " isn't a valid CSS value.";
8976
}
9077

9178
StackError::StackError(Backtraces traces, const AST_Node& node)
9279
: Base(node.pstate(), def_msg, traces), node(node)
9380
{
94-
msg = "stack level too deep";
81+
msg = "stack level too deep";
9582
}
9683

9784
IncompatibleUnits::IncompatibleUnits(const Units& lhs, const Units& rhs)
9885
{
99-
msg = "Incompatible units: '";
100-
msg += rhs.unit();
101-
msg += "' and '";
102-
msg += lhs.unit();
103-
msg += "'.";
86+
msg = "Incompatible units: '" + rhs.unit() + "' and '" + lhs.unit() + "'.";
10487
}
10588

10689
IncompatibleUnits::IncompatibleUnits(const UnitType lhs, const UnitType rhs)
10790
{
108-
msg = "Incompatible units: '";
109-
msg += unit_to_string(rhs);
110-
msg += "' and '";
111-
msg += unit_to_string(lhs);
112-
msg += "'.";
91+
msg = std::string("Incompatible units: '") + unit_to_string(rhs) + "' and '" + unit_to_string(lhs) + "'.";
11392
}
11493

11594
AlphaChannelsNotEqual::AlphaChannelsNotEqual(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, enum Sass_OP op)
11695
: OperationError(), lhs(lhs), rhs(rhs), op(op)
11796
{
118-
msg = "Alpha channels must be equal: ";
119-
msg += lhs->to_string({ NESTED, 5 });
120-
msg += " " + sass_op_to_name(op) + " ";
121-
msg += rhs->to_string({ NESTED, 5 });
122-
msg += ".";
97+
msg = "Alpha channels must be equal: " +
98+
lhs->to_string({ NESTED, 5 }) +
99+
" " + sass_op_to_name(op) + " " +
100+
rhs->to_string({ NESTED, 5 }) + ".";
123101
}
124102

125103
ZeroDivisionError::ZeroDivisionError(const Expression& lhs, const Expression& rhs)
126104
: OperationError(), lhs(lhs), rhs(rhs)
127105
{
128-
msg = "divided by 0";
106+
msg = "divided by 0";
129107
}
130108

131109
UndefinedOperation::UndefinedOperation(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, enum Sass_OP op)
132110
: OperationError(), lhs(lhs), rhs(rhs), op(op)
133111
{
134-
msg = def_op_msg + ": \"";
135-
msg += lhs->to_string({ NESTED, 5 });
136-
msg += " " + sass_op_to_name(op) + " ";
137-
msg += rhs->to_string({ TO_SASS, 5 });
138-
msg += "\".";
112+
msg = def_op_msg + ": \"" +
113+
lhs->to_string({ NESTED, 5 }) +
114+
" " + sass_op_to_name(op) + " " +
115+
rhs->to_string({ TO_SASS, 5 }) +
116+
"\".";
139117
}
140118

141119
InvalidNullOperation::InvalidNullOperation(Expression_Ptr_Const lhs, Expression_Ptr_Const rhs, enum Sass_OP op)
142120
: UndefinedOperation(lhs, rhs, op)
143121
{
144-
msg = def_op_null_msg + ": \"";
145-
msg += lhs->inspect();
146-
msg += " " + sass_op_to_name(op) + " ";
147-
msg += rhs->inspect();
148-
msg += "\".";
122+
msg = def_op_null_msg + ": \"" + lhs->inspect() + " " + sass_op_to_name(op) + " " + rhs->inspect() + "\".";
149123
}
150124

151125
SassValueError::SassValueError(Backtraces traces, ParserState pstate, OperationError& err)

src/fn_selectors.cpp

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,23 +119,15 @@ namespace Sass {
119119

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

130126
// Cannot be a Universal selector
131127
Type_Selector_Obj pType = Cast<Type_Selector>(childSeq->head()->first());
132128
if(pType && pType->name() == "*") {
133-
std::string msg("Can't append \"");
134-
msg += childSeq->to_string();
135-
msg += "\" to \"";
136-
msg += parentSeqClone->to_string();
137-
msg += "\" for `selector-append'";
138-
error(msg, pstate, traces);
129+
error("Can't append \"" + childSeq->to_string() + "\" to \"" +
130+
parentSeqClone->to_string() + "\" for `selector-append'", pstate, traces);
139131
}
140132

141133
// TODO: Add check for namespace stuff

src/fn_utils.hpp

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,7 @@ namespace Sass {
3535
{
3636
T* val = Cast<T>(env[argname]);
3737
if (!val) {
38-
std::string msg("argument `");
39-
msg += argname;
40-
msg += "` of `";
41-
msg += sig;
42-
msg += "` must be a ";
43-
msg += T::type_name();
44-
error(msg, pstate, traces);
38+
error("argument `" + argname + "` of `" + sig + "` must be a " + T::type_name(), pstate, traces);
4539
}
4640
return val;
4741
}

src/operation.hpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,8 @@ namespace Sass {
202202
// will be called if not overloaded
203203
template <typename U> T fallback(U x)
204204
{
205-
std::string msg(typeid(*this).name());
206-
msg += ": CRTP not implemented for ";
207-
throw std::runtime_error(msg + typeid(x).name());
205+
throw std::runtime_error(
206+
std::string(typeid(*this).name()) + ": CRTP not implemented for " + typeid(x).name());
208207
}
209208

210209
};

src/operators.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ namespace Sass {
5757
/* colour math deprecation warning */
5858
void op_color_deprecation(enum Sass_OP op, std::string lsh, std::string rhs, const ParserState& pstate)
5959
{
60-
std::string op_str(sass_op_to_name(op));
61-
62-
std::string msg("The operation `" + lsh + " " + op_str + " " + rhs + "` is deprecated and will be an error in future versions.");
63-
std::string tail("Consider using Sass's color functions instead.\nhttp://sass-lang.com/documentation/Sass/Script/Functions.html#other_color_functions");
64-
65-
deprecated(msg, tail, false, pstate);
60+
deprecated(
61+
"The operation `" + lsh + " " + sass_op_to_name(op) + " " + rhs +
62+
"` is deprecated and will be an error in future versions.",
63+
"Consider using Sass's color functions instead.\n"
64+
"http://sass-lang.com/documentation/Sass/Script/Functions.html#other_color_functions",
65+
/*with_column=*/false, pstate);
6666
}
6767

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

0 commit comments

Comments
 (0)