Skip to content

Commit 2c4b0c3

Browse files
mgretermgreter
committed
Fix and improve last extend refactoring bits
1 parent 3e8e941 commit 2c4b0c3

File tree

8 files changed

+69
-64
lines changed

8 files changed

+69
-64
lines changed

‎src/ast.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ namespace Sass {
228228
void clear() { return elements_.clear(); }
229229
T& last() { return elements_.back(); }
230230
T& first() { return elements_.front(); }
231-
const T last() const { return elements_.back(); }
232-
const T first() const { return elements_.front(); }
231+
const T& last() const { return elements_.back(); }
232+
const T& first() const { return elements_.front(); }
233233

234234
bool operator== (const Vectorized<T>& rhs) const {
235235
// Abort early if sizes do not match

‎src/ast_helpers.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// __EXTENSIONS__ fix on Solaris.
66
#include "sass.hpp"
77
#include <algorithm>
8+
#include <functional>
89
#include "util_string.hpp"
910

1011
namespace Sass {

‎src/ast_sel_weave.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ namespace Sass {
587587
groups1, groups2, {}, cmpChunkForEmptySequence);
588588

589589
// Append chunks with inner arrays flattened
590-
choices.emplace_back(std::move(flattenInner(chunks)));
590+
choices.emplace_back(flattenInner(chunks));
591591

592592
// append all trailing selectors to choices
593593
std::move(std::begin(trails), std::end(trails),

‎src/expand.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,7 @@ namespace Sass {
677677
error("complex selectors may not be extended.", complex->pstate(), traces);
678678
}
679679

680-
if (auto compound = complex->first()->getCompound()) {
680+
if (const CompoundSelector* compound = complex->first()->getCompound()) {
681681

682682
if (compound->length() != 1) {
683683

@@ -689,13 +689,13 @@ namespace Sass {
689689
// Make this an error once deprecation is over
690690
for (SimpleSelectorObj simple : compound->elements()) {
691691
// Pass every selector we ever see to extender (to make them findable for extend)
692-
ctx.extender.addExtension(selector(), simple, e, mediaStack.back());
692+
ctx.extender.addExtension(selector(), simple, mediaStack.back(), e->isOptional());
693693
}
694694

695695
}
696696
else {
697697
// Pass every selector we ever see to extender (to make them findable for extend)
698-
ctx.extender.addExtension(selector(), compound->first(), e, mediaStack.back());
698+
ctx.extender.addExtension(selector(), compound->first(), mediaStack.back(), e->isOptional());
699699
}
700700

701701
}

‎src/extender.cpp

Lines changed: 45 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ namespace Sass {
4747
// ##########################################################################
4848
SelectorListObj Extender::extend(
4949
SelectorListObj& selector,
50-
SelectorListObj& source,
51-
SelectorListObj& targets,
50+
const SelectorListObj& source,
51+
const SelectorListObj& targets,
5252
Backtraces& traces)
5353
{
5454
return extendOrReplace(selector, source, targets, ExtendMode::TARGETS, traces);
@@ -60,8 +60,8 @@ namespace Sass {
6060
// ##########################################################################
6161
SelectorListObj Extender::replace(
6262
SelectorListObj& selector,
63-
SelectorListObj& source,
64-
SelectorListObj& targets,
63+
const SelectorListObj& source,
64+
const SelectorListObj& targets,
6565
Backtraces& traces)
6666
{
6767
return extendOrReplace(selector, source, targets, ExtendMode::REPLACE, traces);
@@ -73,9 +73,9 @@ namespace Sass {
7373
// ##########################################################################
7474
SelectorListObj Extender::extendOrReplace(
7575
SelectorListObj& selector,
76-
SelectorListObj& source,
77-
SelectorListObj& targets,
78-
ExtendMode mode,
76+
const SelectorListObj& source,
77+
const SelectorListObj& targets,
78+
const ExtendMode mode,
7979
Backtraces& traces)
8080
{
8181
ExtSelExtMapEntry extenders;
@@ -87,15 +87,16 @@ namespace Sass {
8787

8888
for (auto complex : targets->elements()) {
8989

90-
if (complex->length() != 1) {
91-
// throw "can't extend complex selector $complex."
92-
}
90+
// This seems superfluous, check is done before!?
91+
// if (complex->length() != 1) {
92+
// error("complex selectors may not be extended.", complex->pstate(), traces);
93+
// }
9394

94-
if (auto compound = complex->first()->getCompound()) {
95+
if (const CompoundSelector* compound = complex->first()->getCompound()) {
9596

9697
ExtSelExtMap extensions;
9798

98-
for (auto simple : compound->elements()) {
99+
for (const SimpleSelectorObj& simple : compound->elements()) {
99100
extensions.insert(std::make_pair(simple, extenders));
100101
}
101102

@@ -287,20 +288,19 @@ namespace Sass {
287288
// Note: this function could need some logic cleanup
288289
// ##########################################################################
289290
void Extender::addExtension(
290-
SelectorListObj& extender,
291-
SimpleSelectorObj& target,
292-
// get get passed a pointer
293-
ExtendRuleObj extend,
294-
CssMediaRuleObj& mediaQueryContext)
291+
const SelectorListObj& extender,
292+
const SimpleSelectorObj& target,
293+
const CssMediaRuleObj& mediaQueryContext,
294+
bool is_optional)
295295
{
296296

297297
auto rules = selectors.find(target);
298298
bool hasRule = rules != selectors.end();
299299

300300
ExtSelExtMapEntry newExtensions;
301301

302-
auto existingExtensions = extensionsByExtender.find(target);
303-
bool hasExistingExtensions = existingExtensions != extensionsByExtender.end();
302+
// ToDo: we check this here first and fetch the same? item again after the loop!?
303+
bool hasExistingExtensions = extensionsByExtender.find(target) != extensionsByExtender.end();
304304

305305
ExtSelExtMapEntry& sources = extensions[target];
306306

@@ -309,7 +309,7 @@ namespace Sass {
309309
Extension state(complex);
310310
// ToDo: fine-tune public API
311311
state.target = target;
312-
state.isOptional = extend->isOptional();
312+
state.isOptional = is_optional;
313313
state.mediaContext = mediaQueryContext;
314314

315315
if (sources.hasKey(complex)) {
@@ -349,12 +349,15 @@ namespace Sass {
349349

350350
ExtSelExtMap newExtensionsByTarget;
351351
newExtensionsByTarget.insert(std::make_pair(target, newExtensions));
352-
existingExtensions = extensionsByExtender.find(target);
353-
if (hasExistingExtensions && !existingExtensions->second.empty()) {
354-
auto additionalExtensions =
355-
extendExistingExtensions(existingExtensions->second, newExtensionsByTarget);
356-
if (!additionalExtensions.empty()) {
357-
mapCopyExts(newExtensionsByTarget, additionalExtensions);
352+
// ToDo: do we really need to fetch again (see top off fn)
353+
auto existingExtensions = extensionsByExtender.find(target);
354+
if (existingExtensions != extensionsByExtender.end()) {
355+
if (hasExistingExtensions && !existingExtensions->second.empty()) {
356+
auto additionalExtensions =
357+
extendExistingExtensions(existingExtensions->second, newExtensionsByTarget);
358+
if (!additionalExtensions.empty()) {
359+
mapCopyExts(newExtensionsByTarget, additionalExtensions);
360+
}
358361
}
359362
}
360363

@@ -410,30 +413,32 @@ namespace Sass {
410413
// ##########################################################################
411414
ExtSelExtMap Extender::extendExistingExtensions(
412415
// Taking in a reference here makes MSVC debug stuck!?
413-
const std::vector<Extension> oldExtensions,
414-
ExtSelExtMap& newExtensions)
416+
const std::vector<Extension>& oldExtensions,
417+
const ExtSelExtMap& newExtensions)
415418
{
416419

417420
ExtSelExtMap additionalExtensions;
418421

419-
for (Extension extension : oldExtensions) {
422+
// During the loop `oldExtensions` vector might be changed.
423+
// Callers normally pass this from `extensionsByExtender` and
424+
// that points back to the `sources` vector from `extensions`.
425+
for (size_t i = 0, iL = oldExtensions.size(); i < iL; i += 1) {
426+
const Extension& extension = oldExtensions[i];
420427
ExtSelExtMapEntry& sources = extensions[extension.target];
421-
std::vector<ComplexSelectorObj> selectors;
422-
423-
selectors = extendComplex(
428+
std::vector<ComplexSelectorObj> selectors(extendComplex(
424429
extension.extender,
425430
newExtensions,
426431
extension.mediaContext
427-
);
432+
));
428433

429434
if (selectors.empty()) {
430435
continue;
431436
}
432437

433438
// ToDo: "catch" error from extend
434439

435-
bool first = false;
436-
bool containsExtension = ObjEqualityFn(selectors.front(), extension.extender);
440+
bool first = false, containsExtension =
441+
ObjEqualityFn(selectors.front(), extension.extender);
437442
for (const ComplexSelectorObj& complex : selectors) {
438443
// If the output contains the original complex
439444
// selector, there's no need to recreate it.
@@ -442,11 +447,11 @@ namespace Sass {
442447
continue;
443448
}
444449

445-
Extension withExtender = extension.withExtender(complex);
450+
const Extension withExtender =
451+
extension.withExtender(complex);
446452
if (sources.hasKey(complex)) {
447-
Extension source = sources.get(complex);
448453
sources.insert(complex, mergeExtension(
449-
source, withExtender));
454+
sources.get(complex), withExtender));
450455
}
451456
else {
452457
sources.insert(complex, withExtender);
@@ -527,7 +532,7 @@ namespace Sass {
527532
// ##########################################################################
528533
std::vector<ComplexSelectorObj> Extender::extendComplex(
529534
// Taking in a reference here makes MSVC debug stuck!?
530-
const ComplexSelectorObj complex,
535+
const ComplexSelectorObj& complex,
531536
const ExtSelExtMap& extensions,
532537
const CssMediaRuleObj& mediaQueryContext)
533538
{
@@ -656,7 +661,7 @@ namespace Sass {
656661
// ##########################################################################
657662
Extension Extender::extensionForCompound(
658663
// Taking in a reference here makes MSVC debug stuck!?
659-
const std::vector<SimpleSelectorObj> simples) const
664+
const std::vector<SimpleSelectorObj>& simples) const
660665
{
661666
CompoundSelectorObj compound = SASS_MEMORY_NEW(CompoundSelector, ParserState("[ext]"));
662667
compound->concat(simples);

‎src/extender.hpp

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -163,17 +163,17 @@ namespace Sass {
163163
// ##########################################################################
164164
static SelectorListObj extend(
165165
SelectorListObj& selector,
166-
SelectorListObj& source,
167-
SelectorListObj& target,
166+
const SelectorListObj& source,
167+
const SelectorListObj& target,
168168
Backtraces& traces);
169169

170170
// ##########################################################################
171171
// Returns a copy of [selector] with [targets] replaced by [source].
172172
// ##########################################################################
173173
static SelectorListObj replace(
174174
SelectorListObj& selector,
175-
SelectorListObj& source,
176-
SelectorListObj& target,
175+
const SelectorListObj& source,
176+
const SelectorListObj& target,
177177
Backtraces& traces);
178178

179179
// ##########################################################################
@@ -206,11 +206,10 @@ namespace Sass {
206206
// within the same context. A `null` context indicates no media queries.
207207
// ##########################################################################
208208
void addExtension(
209-
SelectorListObj& extender,
210-
SimpleSelectorObj& target,
211-
// gets passed by pointer
212-
ExtendRuleObj extend,
213-
CssMediaRuleObj& mediaQueryContext);
209+
const SelectorListObj& extender,
210+
const SimpleSelectorObj& target,
211+
const CssMediaRuleObj& mediaQueryContext,
212+
bool is_optional = false);
214213

215214
// ##########################################################################
216215
// The set of all simple selectors in style rules handled
@@ -235,9 +234,9 @@ namespace Sass {
235234
// ##########################################################################
236235
static SelectorListObj extendOrReplace(
237236
SelectorListObj& selector,
238-
SelectorListObj& source,
239-
SelectorListObj& target,
240-
ExtendMode mode,
237+
const SelectorListObj& source,
238+
const SelectorListObj& target,
239+
const ExtendMode mode,
241240
Backtraces& traces);
242241

243242
// ##########################################################################
@@ -275,8 +274,8 @@ namespace Sass {
275274
// ##########################################################################
276275
ExtSelExtMap extendExistingExtensions(
277276
// Taking in a reference here makes MSVC debug stuck!?
278-
const std::vector<Extension> extensions,
279-
ExtSelExtMap& newExtensions);
277+
const std::vector<Extension>& extensions,
278+
const ExtSelExtMap& newExtensions);
280279

281280
// ##########################################################################
282281
// Extends [list] using [extensions].
@@ -292,7 +291,7 @@ namespace Sass {
292291
// ##########################################################################
293292
std::vector<ComplexSelectorObj> extendComplex(
294293
// Taking in a reference here makes MSVC debug stuck!?
295-
const ComplexSelectorObj list,
294+
const ComplexSelectorObj& list,
296295
const ExtSelExtMap& extensions,
297296
const CssMediaRuleObj& mediaQueryContext);
298297

@@ -309,7 +308,7 @@ namespace Sass {
309308
// ##########################################################################
310309
Extension extensionForCompound(
311310
// Taking in a reference here makes MSVC debug stuck!?
312-
const std::vector<SimpleSelectorObj> simples) const;
311+
const std::vector<SimpleSelectorObj>& simples) const;
313312

314313
// ##########################################################################
315314
// Extends [compound] using [extensions], and returns the

‎src/extension.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace Sass {
1111
// ##########################################################################
1212
// Static function to create a copy with a new extender
1313
// ##########################################################################
14-
Extension Extension::withExtender(ComplexSelectorObj newExtender)
14+
Extension Extension::withExtender(const ComplexSelectorObj& newExtender) const
1515
{
1616
Extension extension(newExtender);
1717
extension.specificity = specificity;

‎src/extension.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ namespace Sass {
8080
// compatible with the query context for this extender.
8181
void assertCompatibleMediaContext(CssMediaRuleObj mediaContext, Backtraces& traces) const;
8282

83-
Extension withExtender(ComplexSelectorObj newExtender);
83+
Extension withExtender(const ComplexSelectorObj& newExtender) const;
8484

8585
};
8686

0 commit comments

Comments
 (0)