Fix and improve the last bits

sass · Jun 17, 2019 · 62484b1 · 62484b1
1 parent 3e8e941
commit 62484b1
Show file tree

Hide file tree

Showing 7 changed files with 40 additions and 36 deletions.
diff --git a/src/ast_helpers.hpp b/src/ast_helpers.hpp
@@ -5,6 +5,7 @@
 // __EXTENSIONS__ fix on Solaris.
 #include "sass.hpp"
 #include <algorithm>
+#include <functional>
 #include "util_string.hpp"
 
 namespace Sass {

diff --git a/src/ast_sel_weave.cpp b/src/ast_sel_weave.cpp
@@ -587,7 +587,7 @@ namespace Sass {
         groups1, groups2, {}, cmpChunkForEmptySequence);
 
     // Append chunks with inner arrays flattened
-    choices.emplace_back(std::move(flattenInner(chunks)));
+    choices.emplace_back(flattenInner(chunks));
 
     // append all trailing selectors to choices
     std::move(std::begin(trails), std::end(trails),

diff --git a/src/expand.cpp b/src/expand.cpp
@@ -689,13 +689,13 @@ namespace Sass {
             // Make this an error once deprecation is over
             for (SimpleSelectorObj simple : compound->elements()) {
               // Pass every selector we ever see to extender (to make them findable for extend)
-              ctx.extender.addExtension(selector(), simple, e, mediaStack.back());
+              ctx.extender.addExtension(selector(), simple, mediaStack.back(), e->isOptional());
             }
 
           }
           else {
             // Pass every selector we ever see to extender (to make them findable for extend)
-            ctx.extender.addExtension(selector(), compound->first(), e, mediaStack.back());
+            ctx.extender.addExtension(selector(), compound->first(), mediaStack.back(), e->isOptional());
           }
 
         }

diff --git a/src/extender.cpp b/src/extender.cpp
@@ -289,18 +289,17 @@ namespace Sass {
   void Extender::addExtension(
     SelectorListObj& extender,
     SimpleSelectorObj& target,
-    // get get passed a pointer
-    ExtendRuleObj extend,
-    CssMediaRuleObj& mediaQueryContext)
+    CssMediaRuleObj& mediaQueryContext,
+    bool is_optional)
   {
 
     auto rules = selectors.find(target);
     bool hasRule = rules != selectors.end();
 
     ExtSelExtMapEntry newExtensions;
 
-    auto existingExtensions = extensionsByExtender.find(target);
-    bool hasExistingExtensions = existingExtensions != extensionsByExtender.end();
+    // ToDo: we check this here first and fetch the same? item again after the loop!?
+    bool hasExistingExtensions = extensionsByExtender.find(target) != extensionsByExtender.end();
 
     ExtSelExtMapEntry& sources = extensions[target];
 
@@ -309,7 +308,7 @@ namespace Sass {
       Extension state(complex);
       // ToDo: fine-tune public API
       state.target = target;
-      state.isOptional = extend->isOptional();
+      state.isOptional = is_optional;
       state.mediaContext = mediaQueryContext;
 
       if (sources.hasKey(complex)) {
@@ -349,12 +348,15 @@ namespace Sass {
 
     ExtSelExtMap newExtensionsByTarget;
     newExtensionsByTarget.insert(std::make_pair(target, newExtensions));
-    existingExtensions = extensionsByExtender.find(target);
-    if (hasExistingExtensions && !existingExtensions->second.empty()) {
-      auto additionalExtensions =
-        extendExistingExtensions(existingExtensions->second, newExtensionsByTarget);
-      if (!additionalExtensions.empty()) {
-        mapCopyExts(newExtensionsByTarget, additionalExtensions);
+    // ToDo: do we really need to fetch again (see top off fn) 
+    auto existingExtensions = extensionsByExtender.find(target);
+    if (existingExtensions != extensionsByExtender.end()) {
+      if (hasExistingExtensions && !existingExtensions->second.empty()) {
+        auto additionalExtensions =
+          extendExistingExtensions(existingExtensions->second, newExtensionsByTarget);
+        if (!additionalExtensions.empty()) {
+          mapCopyExts(newExtensionsByTarget, additionalExtensions);
+        }
       }
     }
 
@@ -410,30 +412,32 @@ namespace Sass {
   // ##########################################################################
   ExtSelExtMap Extender::extendExistingExtensions(
     // Taking in a reference here makes MSVC debug stuck!?
-    const std::vector<Extension> oldExtensions,
+    const std::vector<Extension>& oldExtensions,
     ExtSelExtMap& newExtensions)
   {
 
     ExtSelExtMap additionalExtensions;
 
-    for (Extension extension : oldExtensions) {
+    // During the loop `oldExtensions` vector might be changed.
+    // Callers normally pass this from `extensionsByExtender` and
+    // that points pack to the `sources` vector from `extensions`.
+    for (size_t i = 0, iL = oldExtensions.size(); i < iL; i += 1) {
+      Extension extension = oldExtensions[i];
       ExtSelExtMapEntry& sources = extensions[extension.target];
-      std::vector<ComplexSelectorObj> selectors;
-
-      selectors = extendComplex(
+      std::vector<ComplexSelectorObj> selectors(extendComplex(
         extension.extender,
         newExtensions,
         extension.mediaContext
-      );
+      ));
 
       if (selectors.empty()) {
         continue;
       }
 
       // ToDo: "catch" error from extend
 
-      bool first = false;
-      bool containsExtension = ObjEqualityFn(selectors.front(), extension.extender);
+      bool first = false, containsExtension =
+        ObjEqualityFn(selectors.front(), extension.extender);
       for (const ComplexSelectorObj& complex : selectors) {
         // If the output contains the original complex 
         // selector, there's no need to recreate it.
@@ -442,11 +446,11 @@ namespace Sass {
           continue;
         }
 
-        Extension withExtender = extension.withExtender(complex);
+        const Extension withExtender =
+          extension.withExtender(complex);
         if (sources.hasKey(complex)) {
-          Extension source = sources.get(complex);
           sources.insert(complex, mergeExtension(
-            source, withExtender));
+            sources.get(complex), withExtender));
         }
         else {
           sources.insert(complex, withExtender);
@@ -527,7 +531,7 @@ namespace Sass {
   // ##########################################################################
   std::vector<ComplexSelectorObj> Extender::extendComplex(
     // Taking in a reference here makes MSVC debug stuck!?
-    const ComplexSelectorObj complex,
+    const ComplexSelectorObj& complex,
     const ExtSelExtMap& extensions,
     const CssMediaRuleObj& mediaQueryContext)
   {
@@ -656,7 +660,7 @@ namespace Sass {
   // ##########################################################################
   Extension Extender::extensionForCompound(
     // Taking in a reference here makes MSVC debug stuck!?
-    const std::vector<SimpleSelectorObj> simples) const
+    const std::vector<SimpleSelectorObj>& simples) const
   {
     CompoundSelectorObj compound = SASS_MEMORY_NEW(CompoundSelector, ParserState("[ext]"));
     compound->concat(simples);

diff --git a/src/extender.hpp b/src/extender.hpp
@@ -208,9 +208,8 @@ namespace Sass {
     void addExtension(
       SelectorListObj& extender,
       SimpleSelectorObj& target,
-      // gets passed by pointer
-      ExtendRuleObj extend,
-      CssMediaRuleObj& mediaQueryContext);
+      CssMediaRuleObj& mediaQueryContext,
+      bool is_optional = false);
 
     // ##########################################################################
     // The set of all simple selectors in style rules handled
@@ -275,7 +274,7 @@ namespace Sass {
     // ##########################################################################
     ExtSelExtMap extendExistingExtensions(
       // Taking in a reference here makes MSVC debug stuck!?
-      const std::vector<Extension> extensions,
+      const std::vector<Extension>& extensions,
       ExtSelExtMap& newExtensions);
 
     // ##########################################################################
@@ -292,7 +291,7 @@ namespace Sass {
     // ##########################################################################
     std::vector<ComplexSelectorObj> extendComplex(
       // Taking in a reference here makes MSVC debug stuck!?
-      const ComplexSelectorObj list,
+      const ComplexSelectorObj& list,
       const ExtSelExtMap& extensions,
       const CssMediaRuleObj& mediaQueryContext);
 
@@ -309,7 +308,7 @@ namespace Sass {
     // ##########################################################################
     Extension extensionForCompound(
       // Taking in a reference here makes MSVC debug stuck!?
-      const std::vector<SimpleSelectorObj> simples) const;
+      const std::vector<SimpleSelectorObj>& simples) const;
 
     // ##########################################################################
     // Extends [compound] using [extensions], and returns the

diff --git a/src/extension.cpp b/src/extension.cpp
@@ -11,7 +11,7 @@ namespace Sass {
   // ##########################################################################
   // Static function to create a copy with a new extender
   // ##########################################################################
-  Extension Extension::withExtender(ComplexSelectorObj newExtender)
+  Extension Extension::withExtender(const ComplexSelectorObj& newExtender) const
   {
     Extension extension(newExtender);
     extension.specificity = specificity;

diff --git a/src/extension.hpp b/src/extension.hpp
@@ -80,7 +80,7 @@ namespace Sass {
     // compatible with the query context for this extender.
     void assertCompatibleMediaContext(CssMediaRuleObj mediaContext, Backtraces& traces) const;
 
-    Extension withExtender(ComplexSelectorObj newExtender);
+    Extension withExtender(const ComplexSelectorObj& newExtender) const;
 
   };