Skip to content

Commit 20b91ef

Browse files
glebmxzyfer
glebm
authored and
xzyfer
committed
Avoid modifying Expression in ast_node_to_sass_value
Previously, the argument to `ast_node_to_sass_value` was a `const Expression_Ptr`. The `const` there was useless, as it didn't mean the constness of Expression but merely that `val` pointer could not be reassigned within the function (due to how pointer `typedef` and `const` interacts). Assuming the intended argument type was `const Expression_Ptr *`, I've changed it to `Expression_Ptr_Const`. This required splitting `to{RGBA,HSLA}(bool)` into separate functions, so that the one that always copies could be marked as `const`.
1 parent bc55151 commit 20b91ef

File tree

6 files changed

+50
-32
lines changed

6 files changed

+50
-32
lines changed

‎src/ast2c.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ namespace Sass {
2323
{ return sass_make_color(c->r(), c->g(), c->b(), c->a()); }
2424

2525
union Sass_Value* AST2C::operator()(Color_HSLA_Ptr c)
26-
{ return operator()(c->toRGBA()); }
26+
{
27+
Color_RGBA_Obj rgba = c->copyAsRGBA();
28+
return operator()(rgba.ptr());
29+
}
2730

2831
union Sass_Value* AST2C::operator()(String_Constant_Ptr s)
2932
{

‎src/ast_values.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,7 @@ namespace Sass {
554554
return hash_;
555555
}
556556

557-
Color_HSLA_Ptr Color_RGBA::toHSLA(bool copy)
557+
Color_HSLA_Ptr Color_RGBA::copyAsHSLA() const
558558
{
559559

560560
// Algorithm from http://en.wikipedia.org/wiki/wHSL_and_HSV#Conversion_from_RGB_to_HSL_or_HSV
@@ -592,9 +592,9 @@ namespace Sass {
592592
);
593593
}
594594

595-
Color_RGBA_Ptr Color_RGBA::toRGBA(bool copy)
595+
Color_RGBA_Ptr Color_RGBA::copyAsRGBA() const
596596
{
597-
return copy ? SASS_MEMORY_COPY(this) : this;
597+
return SASS_MEMORY_COPY(this);
598598
}
599599

600600
/////////////////////////////////////////////////////////////////////////
@@ -649,7 +649,7 @@ namespace Sass {
649649
return m1;
650650
}
651651

652-
Color_RGBA_Ptr Color_HSLA::toRGBA(bool copy)
652+
Color_RGBA_Ptr Color_HSLA::copyAsRGBA() const
653653
{
654654

655655
double h = absmod(h_ / 360.0, 1.0);
@@ -671,9 +671,9 @@ namespace Sass {
671671
);
672672
}
673673

674-
Color_HSLA_Ptr Color_HSLA::toHSLA(bool copy)
674+
Color_HSLA_Ptr Color_HSLA::copyAsHSLA() const
675675
{
676-
return copy ? SASS_MEMORY_COPY(this) : this;
676+
return SASS_MEMORY_COPY(this);
677677
}
678678

679679
/////////////////////////////////////////////////////////////////////////

‎src/ast_values.hpp

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,11 @@ namespace Sass {
253253

254254
bool operator== (const Expression& rhs) const override;
255255

256-
virtual Color_RGBA_Ptr toRGBA(bool copy = false) = 0;
257-
virtual Color_HSLA_Ptr toHSLA(bool copy = false) = 0;
256+
virtual Color_RGBA_Ptr copyAsRGBA() const = 0;
257+
virtual Color_RGBA_Ptr toRGBA() = 0;
258+
259+
virtual Color_HSLA_Ptr copyAsHSLA() const = 0;
260+
virtual Color_HSLA_Ptr toHSLA() = 0;
258261

259262
ATTACH_VIRTUAL_AST_OPERATIONS(Color)
260263
};
@@ -273,8 +276,12 @@ namespace Sass {
273276
static std::string type_name() { return "color"; }
274277

275278
size_t hash() const override;
276-
Color_RGBA_Ptr toRGBA(bool copy = false) override;
277-
Color_HSLA_Ptr toHSLA(bool copy = false) override;
279+
280+
Color_RGBA_Ptr copyAsRGBA() const override;
281+
Color_RGBA_Ptr toRGBA() override { return this; }
282+
283+
Color_HSLA_Ptr copyAsHSLA() const override;
284+
Color_HSLA_Ptr toHSLA() override { return copyAsHSLA(); }
278285

279286
bool operator== (const Expression& rhs) const override;
280287

@@ -297,8 +304,12 @@ namespace Sass {
297304
static std::string type_name() { return "color"; }
298305

299306
size_t hash() const override;
300-
Color_RGBA_Ptr toRGBA(bool copy = false) override;
301-
Color_HSLA_Ptr toHSLA(bool copy = false) override;
307+
308+
Color_RGBA_Ptr copyAsRGBA() const override;
309+
Color_RGBA_Ptr toRGBA() override { return copyAsRGBA(); }
310+
311+
Color_HSLA_Ptr copyAsHSLA() const override;
312+
Color_HSLA_Ptr toHSLA() override { return this; }
302313

303314
bool operator== (const Expression& rhs) const override;
304315

‎src/fn_colors.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ namespace Sass {
277277
{
278278
Color_Ptr col = ARG("$color", Color);
279279
double degrees = ARGVAL("$degrees");
280-
Color_HSLA_Obj copy = col->toHSLA(true);
280+
Color_HSLA_Obj copy = col->copyAsHSLA();
281281
copy->h(absmod(copy->h() + degrees, 360.0));
282282
return copy.detach();
283283
}
@@ -287,7 +287,7 @@ namespace Sass {
287287
{
288288
Color_Ptr col = ARG("$color", Color);
289289
double amount = DARG_U_PRCT("$amount");
290-
Color_HSLA_Obj copy = col->toHSLA(true);
290+
Color_HSLA_Obj copy = col->copyAsHSLA();
291291
copy->l(clip(copy->l() + amount, 0.0, 100.0));
292292
return copy.detach();
293293

@@ -298,7 +298,7 @@ namespace Sass {
298298
{
299299
Color_Ptr col = ARG("$color", Color);
300300
double amount = DARG_U_PRCT("$amount");
301-
Color_HSLA_Obj copy = col->toHSLA(true);
301+
Color_HSLA_Obj copy = col->copyAsHSLA();
302302
copy->l(clip(copy->l() - amount, 0.0, 100.0));
303303
return copy.detach();
304304
}
@@ -313,7 +313,7 @@ namespace Sass {
313313

314314
Color_Ptr col = ARG("$color", Color);
315315
double amount = DARG_U_PRCT("$amount");
316-
Color_HSLA_Obj copy = col->toHSLA(true);
316+
Color_HSLA_Obj copy = col->copyAsHSLA();
317317
copy->s(clip(copy->s() + amount, 0.0, 100.0));
318318
return copy.detach();
319319
}
@@ -323,7 +323,7 @@ namespace Sass {
323323
{
324324
Color_Ptr col = ARG("$color", Color);
325325
double amount = DARG_U_PRCT("$amount");
326-
Color_HSLA_Obj copy = col->toHSLA(true);
326+
Color_HSLA_Obj copy = col->copyAsHSLA();
327327
copy->s(clip(copy->s() - amount, 0.0, 100.0));
328328
return copy.detach();
329329
}
@@ -338,7 +338,7 @@ namespace Sass {
338338
}
339339

340340
Color_Ptr col = ARG("$color", Color);
341-
Color_HSLA_Obj copy = col->toHSLA(true);
341+
Color_HSLA_Obj copy = col->copyAsHSLA();
342342
copy->s(0.0); // just reset saturation
343343
return copy.detach();
344344
}
@@ -351,7 +351,7 @@ namespace Sass {
351351
BUILT_IN(complement)
352352
{
353353
Color_Ptr col = ARG("$color", Color);
354-
Color_HSLA_Obj copy = col->toHSLA(true);
354+
Color_HSLA_Obj copy = col->copyAsHSLA();
355355
copy->h(absmod(copy->h() - 180.0, 360.0));
356356
return copy.detach();
357357
}
@@ -367,7 +367,7 @@ namespace Sass {
367367

368368
Color_Ptr col = ARG("$color", Color);
369369
double weight = DARG_U_PRCT("$weight");
370-
Color_RGBA_Obj inv = col->toRGBA(true);
370+
Color_RGBA_Obj inv = col->copyAsRGBA();
371371
inv->r(clip(255.0 - inv->r(), 0.0, 255.0));
372372
inv->g(clip(255.0 - inv->g(), 0.0, 255.0));
373373
inv->b(clip(255.0 - inv->b(), 0.0, 255.0));
@@ -441,15 +441,15 @@ namespace Sass {
441441
error("Cannot specify HSL and RGB values for a color at the same time for `adjust-color'", pstate, traces);
442442
}
443443
else if (rgb) {
444-
Color_RGBA_Obj c = col->toRGBA(true);
444+
Color_RGBA_Obj c = col->copyAsRGBA();
445445
if (r) c->r(c->r() + DARG_R_BYTE("$red"));
446446
if (g) c->g(c->g() + DARG_R_BYTE("$green"));
447447
if (b) c->b(c->b() + DARG_R_BYTE("$blue"));
448448
if (a) c->a(c->a() + DARG_R_FACT("$alpha"));
449449
return c.detach();
450450
}
451451
else if (hsl) {
452-
Color_HSLA_Obj c = col->toHSLA(true);
452+
Color_HSLA_Obj c = col->copyAsHSLA();
453453
if (h) c->h(c->h() + absmod(h->value(), 360.0));
454454
if (s) c->s(c->s() + DARG_R_PRCT("$saturation"));
455455
if (l) c->l(c->l() + DARG_R_PRCT("$lightness"));
@@ -486,7 +486,7 @@ namespace Sass {
486486
error("Cannot specify HSL and RGB values for a color at the same time for `scale-color'", pstate, traces);
487487
}
488488
else if (rgb) {
489-
Color_RGBA_Obj c = col->toRGBA(true);
489+
Color_RGBA_Obj c = col->copyAsRGBA();
490490
double rscale = (r ? DARG_R_PRCT("$red") : 0.0) / 100.0;
491491
double gscale = (g ? DARG_R_PRCT("$green") : 0.0) / 100.0;
492492
double bscale = (b ? DARG_R_PRCT("$blue") : 0.0) / 100.0;
@@ -498,7 +498,7 @@ namespace Sass {
498498
return c.detach();
499499
}
500500
else if (hsl) {
501-
Color_HSLA_Obj c = col->toHSLA(true);
501+
Color_HSLA_Obj c = col->copyAsHSLA();
502502
double hscale = (h ? DARG_R_PRCT("$hue") : 0.0) / 100.0;
503503
double sscale = (s ? DARG_R_PRCT("$saturation") : 0.0) / 100.0;
504504
double lscale = (l ? DARG_R_PRCT("$lightness") : 0.0) / 100.0;
@@ -540,15 +540,15 @@ namespace Sass {
540540
error("Cannot specify HSL and RGB values for a color at the same time for `change-color'", pstate, traces);
541541
}
542542
else if (rgb) {
543-
Color_RGBA_Obj c = col->toRGBA(true);
543+
Color_RGBA_Obj c = col->copyAsRGBA();
544544
if (r) c->r(DARG_U_BYTE("$red"));
545545
if (g) c->g(DARG_U_BYTE("$green"));
546546
if (b) c->b(DARG_U_BYTE("$blue"));
547547
if (a) c->a(DARG_U_FACT("$alpha"));
548548
return c.detach();
549549
}
550550
else if (hsl) {
551-
Color_HSLA_Obj c = col->toHSLA(true);
551+
Color_HSLA_Obj c = col->copyAsHSLA();
552552
if (h) c->h(absmod(h->value(), 360.0));
553553
if (s) c->s(DARG_U_PRCT("$saturation"));
554554
if (l) c->l(DARG_U_PRCT("$lightness"));

‎src/values.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
namespace Sass {
1111

1212
// convert value from C++ side to C-API
13-
union Sass_Value* ast_node_to_sass_value (const Expression_Ptr val)
13+
union Sass_Value* ast_node_to_sass_value (Expression_Ptr_Const val)
1414
{
1515
if (val->concrete_type() == Expression::NUMBER)
1616
{
@@ -19,9 +19,13 @@ namespace Sass {
1919
}
2020
else if (val->concrete_type() == Expression::COLOR)
2121
{
22-
// ToDo: allow to also use HSLA colors!!
23-
Color_RGBA_Obj col = Cast<Color>(val)->toRGBA();
24-
return sass_make_color(col->r(), col->g(), col->b(), col->a());
22+
if (Color_RGBA_Ptr_Const rgba = Cast<Color_RGBA>(val)) {
23+
return sass_make_color(rgba->r(), rgba->g(), rgba->b(), rgba->a());
24+
} else {
25+
// ToDo: allow to also use HSLA colors!!
26+
Color_RGBA_Obj col = Cast<Color>(val)->copyAsRGBA();
27+
return sass_make_color(col->r(), col->g(), col->b(), col->a());
28+
}
2529
}
2630
else if (val->concrete_type() == Expression::LIST)
2731
{

‎src/values.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace Sass {
77

8-
union Sass_Value* ast_node_to_sass_value (const Expression_Ptr val);
8+
union Sass_Value* ast_node_to_sass_value (Expression_Ptr_Const val);
99
Value_Ptr sass_value_to_ast_node (const union Sass_Value* val);
1010

1111
}

0 commit comments

Comments
 (0)