From 0ba687602ee8eba15e46ca97f21b34c0c33637e5 Mon Sep 17 00:00:00 2001
From: Marcel Greter <marcel.greter@ocbnet.ch>
Date: Sat, 15 Jun 2019 18:46:43 +0200
Subject: [PATCH] Improve and polish ported extender code

- Added some more documentation from dart sass
- Use references where possible to avoid copies
- Move extend check to extender for less API surface
---
 src/ast.hpp       |   2 +
 src/context.cpp   |  18 +--
 src/expand.cpp    |  28 ++--
 src/expand.hpp    |   4 +-
 src/extender.cpp  | 314 +++++++++++++++++++++++++----------------
 src/extender.hpp  | 353 +++++++++++++++++++++++++++++++++++-----------
 src/extension.cpp |   2 +-
 src/extension.hpp |   2 +-
 8 files changed, 495 insertions(+), 228 deletions(-)

diff --git a/src/ast.hpp b/src/ast.hpp
index b56f0fe838..7df31e83a0 100644
--- a/src/ast.hpp
+++ b/src/ast.hpp
@@ -228,6 +228,8 @@ namespace Sass {
     void clear()            { return elements_.clear(); }
     T last() const          { return elements_.back(); }
     T first() const         { return elements_.front(); }
+    T& last() { return elements_.back(); }
+    T& first() { return elements_.front(); }
 
     bool operator== (const Vectorized<T>& rhs) const {
       // Abort early if sizes do not match
diff --git a/src/context.cpp b/src/context.cpp
index 8a1235ed69..df5f4a34d9 100644
--- a/src/context.cpp
+++ b/src/context.cpp
@@ -654,22 +654,18 @@ namespace Sass {
     }
     // expand and eval the tree
     root = expand(root);
+
+    Extension unsatisfied;
+    // check that all extends were used
+    if (extender.checkForUnsatisfiedExtends(unsatisfied)) {
+      throw Exception::UnsatisfiedExtend(traces, unsatisfied);
+    }
+
     // check nesting
     check_nesting(root);
     // merge and bubble certain rules
     root = cssize(root);
 
-    ExtSmplSelSet originals = extender.getSimpleSelectors();
-    for (auto target : extender.extensions) {
-      SimpleSelector* key = target.first;
-      ExtSelExtMapEntry& val = target.second;
-      if (originals.find(key) == originals.end()) {
-        const Extension& extension = val.front().second;
-        if (extension.isOptional) continue;
-        throw Exception::UnsatisfiedExtend(traces, extension);
-      }
-    }
-
     // clean up by removing empty placeholders
     // ToDo: maybe we can do this somewhere else?
     Remove_Placeholders remove_placeholders;
diff --git a/src/expand.cpp b/src/expand.cpp
index eaf26cb5c1..27038f07db 100644
--- a/src/expand.cpp
+++ b/src/expand.cpp
@@ -67,24 +67,30 @@ namespace Sass {
     return selector_stack;
   }
 
-  SelectorListObj Expand::selector()
+  SelectorListObj& Expand::selector()
   {
     if (selector_stack.size() > 0) {
-      auto asd = selector_stack.back();
-      if (asd.isNull()) return {};
-      return asd;
+      auto& sel = selector_stack.back();
+      if (sel.isNull()) return sel;
+      return sel;
     }
-    return {};
+    // Avoid the need to return copies
+    // We always want an empty first item
+    selector_stack.push_back({});
+    return selector_stack.back();;
   }
 
-  SelectorListObj Expand::original()
+  SelectorListObj& Expand::original()
   {
     if (originalStack.size() > 0) {
-      auto asd = originalStack.back();
-      if (asd.isNull()) return {};
-      return asd;
-    }
-    return {};
+      auto& sel = originalStack.back();
+      if (sel.isNull()) return sel;
+      return sel;
+    }
+    // Avoid the need to return copies
+    // We always want an empty first item
+    originalStack.push_back({});
+    return originalStack.back();
   }
 
   SelectorListObj Expand::popFromSelectorStack()
diff --git a/src/expand.hpp b/src/expand.hpp
index 07e13e0394..db5852ce49 100644
--- a/src/expand.hpp
+++ b/src/expand.hpp
@@ -19,8 +19,8 @@ namespace Sass {
   public:
 
     Env* environment();
-    SelectorListObj selector();
-    SelectorListObj original();
+    SelectorListObj& selector();
+    SelectorListObj& original();
     SelectorListObj popFromSelectorStack();
     SelectorStack getSelectorStack();
     void pushToSelectorStack(SelectorListObj selector);
diff --git a/src/extender.cpp b/src/extender.cpp
index a76e4600a6..51cb4ad60b 100644
--- a/src/extender.cpp
+++ b/src/extender.cpp
@@ -9,16 +9,10 @@
 
 namespace Sass {
 
-  bool hasExactlyOne(ComplexSelectorObj vec)
-  {
-    return vec->length() == 1;
-  }
-
-  bool hasMoreThanOne(ComplexSelectorObj vec)
-  {
-    return vec->length() > 1;
-  }
-
+  // ##########################################################################
+  // Constructor without default [mode].
+  // [traces] are needed to throw errors.
+  // ##########################################################################
   Extender::Extender(Backtraces& traces) :
     mode(NORMAL),
     traces(traces),
@@ -30,6 +24,10 @@ namespace Sass {
     originals()
   {}
 
+  // ##########################################################################
+  // Constructor with specific [mode].
+  // [traces] are needed to throw errors.
+  // ##########################################################################
   Extender::Extender(ExtendMode mode, Backtraces& traces) :
     mode(mode),
     traces(traces),
@@ -41,15 +39,6 @@ namespace Sass {
     originals()
   {}
 
-  ExtSmplSelSet Extender::getSimpleSelectors() const
-  {
-    ExtSmplSelSet set;
-    for (auto entry : selectors) {
-      set.insert(entry.first);
-    }
-    return set;
-  }
-
   // ##########################################################################
   // Extends [selector] with [source] extender and [targets] extendees.
   // This works as though `source {@extend target}` were written in the
@@ -57,9 +46,9 @@ namespace Sass {
   // selectors which must be extended as a unit.
   // ##########################################################################
   SelectorListObj Extender::extend(
-    SelectorListObj selector,
-    SelectorListObj source,
-    SelectorListObj targets,
+    SelectorListObj& selector,
+    SelectorListObj& source,
+    SelectorListObj& targets,
     Backtraces& traces)
   {
     return extendOrReplace(selector, source, targets, ExtendMode::TARGETS, traces);
@@ -70,9 +59,9 @@ namespace Sass {
   // Returns a copy of [selector] with [targets] replaced by [source].
   // ##########################################################################
   SelectorListObj Extender::replace(
-    SelectorListObj selector,
-    SelectorListObj source,
-    SelectorListObj targets,
+    SelectorListObj& selector,
+    SelectorListObj& source,
+    SelectorListObj& targets,
     Backtraces& traces)
   {
     return extendOrReplace(selector, source, targets, ExtendMode::REPLACE, traces);
@@ -83,9 +72,9 @@ namespace Sass {
   // A helper function for [extend] and [replace].
   // ##########################################################################
   SelectorListObj Extender::extendOrReplace(
-    SelectorListObj selector,
-    SelectorListObj source,
-    SelectorListObj targets,
+    SelectorListObj& selector,
+    SelectorListObj& source,
+    SelectorListObj& targets,
     ExtendMode mode,
     Backtraces& traces)
   {
@@ -129,6 +118,44 @@ namespace Sass {
   }
   // EO extendOrReplace
 
+  // ##########################################################################
+  // The set of all simple selectors in style rules handled
+  // by this extender. This includes simple selectors that
+  // were added because of downstream extensions.
+  // ##########################################################################
+  ExtSmplSelSet Extender::getSimpleSelectors() const
+  {
+    ExtSmplSelSet set;
+    for (auto& entry : selectors) {
+      set.insert(entry.first);
+    }
+    return set;
+  }
+  // EO getSimpleSelectors
+
+  // ##########################################################################
+  // Check for extends that have not been satisfied.
+  // Returns true if any non-optional extension did not
+  // extend any selector. Updates the passed reference
+  // to point to that Extension for further analysis.
+  // ##########################################################################
+  bool Extender::checkForUnsatisfiedExtends(Extension& unsatisfied) const
+  {
+    ExtSmplSelSet originals = getSimpleSelectors();
+    for (auto target : extensions) {
+      SimpleSelector* key = target.first;
+      ExtSelExtMapEntry& val = target.second;
+      if (originals.find(key) == originals.end()) {
+        const Extension& extension = val.front().second;
+        if (extension.isOptional) continue;
+        unsatisfied = extension;
+        return true;
+      }
+    }
+    return false;
+  }
+  // EO checkUnsatisfiedExtends
+
   // ##########################################################################
   // Adds [selector] to this extender, with [selectorSpan] as the span covering
   // the selector and [ruleSpan] as the span covering the entire style rule.
@@ -138,11 +165,16 @@ namespace Sass {
   // The [mediaContext] is the media query context in which the selector was
   // defined, or `null` if it was defined at the top level of the document.
   // ##########################################################################
-  void Extender::addSelector(SelectorListObj selector, CssMediaRule_Obj mediaContext)
+  void Extender::addSelector(
+    const SelectorListObj& selector,
+    const CssMediaRuleObj& mediaContext)
   {
 
-    SelectorListObj original = selector;
-    if (!original->isInvisible()) {
+    // Note: dart-sass makes a copy here AFAICT
+    // Note: probably why we have originalStack
+    // SelectorListObj original = selector;
+
+    if (!selector->isInvisible()) {
       for (auto complex : selector->elements()) {
         originals.insert(complex);
       }
@@ -150,7 +182,7 @@ namespace Sass {
 
     if (!extensions.empty()) {
 
-      SelectorListObj res = extendList(original, extensions, mediaContext);
+      SelectorListObj res = extendList(selector, extensions, mediaContext);
 
       selector->elements(res->elements());
 
@@ -169,7 +201,9 @@ namespace Sass {
   // Registers the [SimpleSelector]s in [list]
   // to point to [rule] in [selectors].
   // ##########################################################################
-  void Extender::registerSelector(SelectorListObj list, SelectorListObj rule)
+  void Extender::registerSelector(
+    const SelectorListObj& list,
+    const SelectorListObj& rule)
   {
     if (list.isNull() || list->empty()) return;
     for (auto complex : list->elements()) {
@@ -179,7 +213,8 @@ namespace Sass {
             selectors[simple].insert(rule);
             if (auto pseudo = simple->getPseudoSelector()) {
               if (pseudo->selector()) {
-                registerSelector(pseudo->selector(), rule);
+                auto sel = pseudo->selector();
+                registerSelector(sel, rule);
               }
             }
           }
@@ -195,7 +230,9 @@ namespace Sass {
   // media contexts. Throws an [ArgumentError] if [left]
   // and [right] don't have the same extender and target.
   // ##########################################################################
-  Extension mergeExtension(Extension lhs, Extension rhs)
+  Extension Extender::mergeExtension(
+    const Extension& lhs,
+    const Extension& rhs)
   {
     // If one extension is optional and doesn't add a
     // special media context, it doesn't need to be merged.
@@ -213,7 +250,9 @@ namespace Sass {
   // ##########################################################################
   // Helper function to copy extension between maps
   // ##########################################################################
-  void mapCopyExts(ExtSelExtMap& dest, ExtSelExtMap& source)
+  void Extender::mapCopyExts(
+    ExtSelExtMap& dest,
+    const ExtSelExtMap& source)
   {
     for (auto it : source) {
       SimpleSelectorObj key = it.first;
@@ -223,14 +262,16 @@ namespace Sass {
         dest.insert(std::make_pair(key, inner));
       }
       else {
+        ExtSelExtMapEntry& imap = dmap->second;
         // ToDo: optimize ordered_map API!
-        for (ComplexSelectorObj it2 : inner) {
-          ExtSelExtMapEntry& imap = dest[key];
+        // ToDo: we iterate and fetch the value
+        for (ComplexSelectorObj& it2 : inner) {
           imap.insert(it2, inner.get(it2));
         }
       }
     }
   }
+  // EO mapCopyExts
 
   // ##########################################################################
   // Adds an extension to this extender. The [extender] is the selector for the
@@ -246,10 +287,11 @@ namespace Sass {
   // Note: this function could need some logic cleanup
   // ##########################################################################
   void Extender::addExtension(
-    SelectorListObj extender,
-    SimpleSelectorObj target,
-    ExtendRule_Obj extend,
-    CssMediaRule_Obj mediaQueryContext)
+    SelectorListObj& extender,
+    SimpleSelectorObj& target,
+    // get get passed a pointer
+    ExtendRuleObj extend,
+    CssMediaRule_Obj& mediaQueryContext)
   {
 
     auto rules = selectors.find(target);
@@ -262,7 +304,7 @@ namespace Sass {
 
     ExtSelExtMapEntry& sources = extensions[target];
 
-    for (auto complex : extender->elements()) {
+    for (auto& complex : extender->elements()) {
 
       Extension state(complex);
       // ToDo: fine-tune public API
@@ -281,9 +323,9 @@ namespace Sass {
 
       sources.insert(complex, state);
 
-      for (auto component : complex->elements()) {
+      for (auto& component : complex->elements()) {
         if (auto compound = component->getCompound()) {
-          for (auto simple : compound->elements()) {
+          for (auto& simple : compound->elements()) {
             extensionsByExtender[simple].push_back(state);
             if (sourceSpecificity.find(simple) == sourceSpecificity.end()) {
               // Only source specificity for the original selector is relevant.
@@ -329,12 +371,12 @@ namespace Sass {
   // Note: dart-sass throws an error in here
   // ##########################################################################
   void Extender::extendExistingStyleRules(
-    ExtListSelSet& rules,
-    ExtSelExtMap& newExtensions)
+    const ExtListSelSet& rules,
+    const ExtSelExtMap& newExtensions)
   {
     // Is a modifyableCssStyleRUle in dart sass
-    for (SelectorListObj rule : rules) {
-      SelectorListObj oldValue = SASS_MEMORY_COPY(rule);
+    for (const SelectorListObj& rule : rules) {
+      const SelectorListObj& oldValue = SASS_MEMORY_COPY(rule);
       CssMediaRule_Obj mediaContext;
       if (mediaContexts.hasKey(rule)) mediaContext = mediaContexts.get(rule);
       SelectorListObj ext = extendList(rule, newExtensions, mediaContext);
@@ -367,7 +409,7 @@ namespace Sass {
   // Note: dart-sass throws an error in here
   // ##########################################################################
   ExtSelExtMap Extender::extendExistingExtensions(
-    std::vector<Extension> oldExtensions,
+    const std::vector<Extension>& oldExtensions,
     ExtSelExtMap& newExtensions)
   {
 
@@ -391,7 +433,7 @@ namespace Sass {
 
       bool first = false;
       bool containsExtension = ObjEqualityFn(selectors.front(), extension.extender);
-      for (ComplexSelectorObj complex : selectors) {
+      for (const ComplexSelectorObj& complex : selectors) {
         // If the output contains the original complex 
         // selector, there's no need to recreate it.
         if (containsExtension && first) {
@@ -401,14 +443,15 @@ namespace Sass {
 
         Extension withExtender = extension.withExtender(complex);
         if (sources.hasKey(complex)) {
+          Extension source = sources.get(complex);
           sources.insert(complex, mergeExtension(
-            sources.get(complex), withExtender));
+            source, withExtender));
         }
         else {
           sources.insert(complex, withExtender);
-          for (auto component : complex->elements()) {
+          for (auto& component : complex->elements()) {
             if (auto compound = component->getCompound()) {
-              for (auto simple : compound->elements()) {
+              for (auto& simple : compound->elements()) {
                 extensionsByExtender[simple].push_back(withExtender);
               }
             }
@@ -437,16 +480,16 @@ namespace Sass {
   // Extends [list] using [extensions].
   // ##########################################################################
   SelectorListObj Extender::extendList(
-    SelectorListObj list,
-    ExtSelExtMap& extensions,
-    CssMediaRule_Obj mediaQueryContext)
+    const SelectorListObj& list,
+    const ExtSelExtMap& extensions,
+    const CssMediaRuleObj& mediaQueryContext)
   {
 
     // This could be written more simply using [List.map], but we want to
     // avoid any allocations in the common case where no extends apply.
     std::vector<ComplexSelectorObj> extended;
     for (size_t i = 0; i < list->length(); i++) {
-      ComplexSelectorObj complex = list->get(i);
+      const ComplexSelectorObj& complex = list->get(i);
       std::vector<ComplexSelectorObj> result =
         extendComplex(complex, extensions, mediaQueryContext);
       if (result.empty()) {
@@ -482,9 +525,9 @@ namespace Sass {
   // returns the contents of a [SelectorList].
   // ##########################################################################
   std::vector<ComplexSelectorObj> Extender::extendComplex(
-    ComplexSelectorObj complex,
-    ExtSelExtMap& extensions,
-    CssMediaRule_Obj mediaQueryContext)
+    const ComplexSelectorObj& complex,
+    const ExtSelExtMap& extensions,
+    const CssMediaRule_Obj& mediaQueryContext)
   {
 
     // The complex selectors that each compound selector in [complex.components]
@@ -509,9 +552,10 @@ namespace Sass {
     std::vector<std::vector<ComplexSelectorObj>> extendedNotExpanded;
     bool isOriginal = originals.find(complex) != originals.end();
     for (size_t i = 0; i < complex->length(); i += 1) {
-      SelectorComponentObj component = complex->get(i);
-      if (CompoundSelectorObj compound = Cast<CompoundSelector>(component)) {
-        std::vector<ComplexSelectorObj> extended = extendCompound(compound, extensions, mediaQueryContext, isOriginal);
+      const SelectorComponentObj& component = complex->get(i);
+      if (CompoundSelector* compound = Cast<CompoundSelector>(component)) {
+        std::vector<ComplexSelectorObj> extended = extendCompound(
+          compound, extensions, mediaQueryContext, isOriginal);
         if (extended.empty()) {
           if (!extendedNotExpanded.empty()) {
             extendedNotExpanded.push_back({
@@ -552,20 +596,20 @@ namespace Sass {
     std::vector<std::vector<ComplexSelectorObj>>
       paths = permutate(extendedNotExpanded);
     
-    for (std::vector<ComplexSelectorObj> path : paths) {
+    for (const std::vector<ComplexSelectorObj>& path : paths) {
       // Unpack the inner complex selector to component list
       std::vector<std::vector<SelectorComponentObj>> _paths;
-      for (ComplexSelectorObj sel : path) {
+      for (const ComplexSelectorObj& sel : path) {
         _paths.insert(_paths.end(), sel->elements());
       }
 
       std::vector<std::vector<SelectorComponentObj>> weaved = weave(_paths);
 
-      for (std::vector<SelectorComponentObj> components : weaved) {
+      for (std::vector<SelectorComponentObj>& components : weaved) {
 
         ComplexSelectorObj cplx = SASS_MEMORY_NEW(ComplexSelector, "[phony]");
         cplx->hasPreLineFeed(complex->hasPreLineFeed());
-        for (auto pp : path) {
+        for (auto& pp : path) {
           if (pp->hasPreLineFeed()) {
             cplx->hasPreLineFeed(true);
           }
@@ -591,10 +635,11 @@ namespace Sass {
   // EO extendComplex
 
   // ##########################################################################
-  // Returns a one-off [Extension] whose extender
-  // is composed solely of [simple].
+  // Returns a one-off [Extension] whose
+  // extender is composed solely of [simple].
   // ##########################################################################
-  Extension Extender::extensionForSimple(SimpleSelectorObj simple)
+  Extension Extender::extensionForSimple(
+    const SimpleSelectorObj& simple)
   {
     Extension extension(simple->wrapInComplex());
     extension.specificity = maxSourceSpecificity(simple);
@@ -607,7 +652,8 @@ namespace Sass {
   // Returns a one-off [Extension] whose extender is composed
   // solely of a compound selector containing [simples].
   // ##########################################################################
-  Extension Extender::extensionForCompound(std::vector<SimpleSelectorObj> simples)
+  Extension Extender::extensionForCompound(
+    const std::vector<SimpleSelectorObj>& simples)
   {
     CompoundSelectorObj compound = SASS_MEMORY_NEW(CompoundSelector, ParserState("[ext]"));
     compound->concat(simples);
@@ -625,9 +671,9 @@ namespace Sass {
   // meaning that [compound] should not be trimmed out.
   // ##########################################################################
   std::vector<ComplexSelectorObj> Extender::extendCompound(
-    CompoundSelectorObj compound,
-    ExtSelExtMap& extensions,
-    CssMediaRule_Obj mediaQueryContext,
+    const CompoundSelectorObj& compound,
+    const ExtSelExtMap& extensions,
+    const CssMediaRule_Obj& mediaQueryContext,
     bool inOriginal)
   {
 
@@ -646,7 +692,7 @@ namespace Sass {
     std::vector<std::vector<Extension>> options;
 
     for (size_t i = 0; i < compound->length(); i++) {
-      SimpleSelectorObj simple = compound->get(i);
+      const SimpleSelectorObj& simple = compound->get(i);
       auto extended = extendSimple(simple, extensions, mediaQueryContext, targetsUsed);
       if (extended.empty()) {
         if (!options.empty()) {
@@ -725,7 +771,7 @@ namespace Sass {
 
     for (size_t i = 0; i < prePaths.size(); i += 1) {
       std::vector<std::vector<SelectorComponentObj>> complexes;
-      std::vector<Extension> path = prePaths.at(i);
+      const std::vector<Extension>& path = prePaths[i];
       if (first) {
         // The first path is always the original selector. We can't just
         // return [compound] directly because pseudo selectors may be
@@ -734,7 +780,7 @@ namespace Sass {
         CompoundSelectorObj mergedSelector =
           SASS_MEMORY_NEW(CompoundSelector, "[ext]");
         for (size_t n = 0; n < path.size(); n += 1) {
-          ComplexSelectorObj sel = path[n].extender;
+          const ComplexSelectorObj& sel = path[n].extender;
           if (CompoundSelectorObj compound = Cast<CompoundSelector>(sel->last())) {
             mergedSelector->concat(compound->elements());
           }
@@ -745,10 +791,10 @@ namespace Sass {
         std::vector<SimpleSelectorObj> originals;
         std::vector<std::vector<SelectorComponentObj>> toUnify;
 
-        for (auto state : path) {
+        for (auto& state : path) {
           if (state.isOriginal) {
-            ComplexSelectorObj sel = state.extender;
-            if (CompoundSelectorObj compound = Cast<CompoundSelector>(sel->last())) {
+            const ComplexSelectorObj& sel = state.extender;
+            if (const CompoundSelector* compound = Cast<CompoundSelector>(sel->last())) {
               originals.insert(originals.end(), compound->last());
             }
           }
@@ -771,13 +817,13 @@ namespace Sass {
 
       bool lineBreak = false;
       // var specificity = _sourceSpecificityFor(compound);
-      for (auto state : path) {
+      for (const Extension& state : path) {
         state.assertCompatibleMediaContext(mediaQueryContext, traces);
         lineBreak = lineBreak || state.extender->hasPreLineFeed();
         // specificity = math.max(specificity, state.specificity);
       }
 
-      for (std::vector<SelectorComponentObj> components : complexes) {
+      for (std::vector<SelectorComponentObj>& components : complexes) {
         auto sel = SASS_MEMORY_NEW(ComplexSelector, "[ext]");
         sel->hasPreLineFeed(lineBreak);
         sel->elements(components);
@@ -795,14 +841,14 @@ namespace Sass {
   // contents of any selector pseudos it contains.
   // ##########################################################################
   std::vector<Extension> Extender::extendWithoutPseudo(
-    SimpleSelectorObj simple,
-    ExtSelExtMap& extensions,
+    const SimpleSelectorObj& simple,
+    const ExtSelExtMap& extensions,
     ExtSmplSelSet* targetsUsed)
   {
 
     auto extension = extensions.find(simple);
     if (extension == extensions.end()) return {};
-    ExtSelExtMapEntry& extenders = extension->second;
+    const ExtSelExtMapEntry& extenders = extension->second;
 
     if (targetsUsed != nullptr) {
       targetsUsed->insert(simple);
@@ -811,7 +857,6 @@ namespace Sass {
       return extenders.values();
     }
 
-    extenders = extensions[simple];
     const std::vector<Extension>&
       values = extenders.values();
     std::vector<Extension> result;
@@ -827,19 +872,20 @@ namespace Sass {
   // contents of any selector pseudos it contains.
   // ##########################################################################
   std::vector<std::vector<Extension>> Extender::extendSimple(
-    SimpleSelectorObj simple,
-    ExtSelExtMap& extensions,
-    CssMediaRule_Obj mediaQueryContext,
+    const SimpleSelectorObj& simple,
+    const ExtSelExtMap& extensions,
+    const CssMediaRuleObj& mediaQueryContext,
     ExtSmplSelSet* targetsUsed)
   {
-    if (Pseudo_Selector_Obj pseudo = Cast<Pseudo_Selector>(simple)) {
+    if (Pseudo_Selector* pseudo = Cast<Pseudo_Selector>(simple)) {
       if (pseudo->selector()) {
         std::vector<std::vector<Extension>> merged;
         std::vector<Pseudo_Selector_Obj> extended =
           extendPseudo(pseudo, extensions, mediaQueryContext);
-        for (auto extend : extended) {
+        for (Pseudo_Selector_Obj& extend : extended) {
+          SimpleSelectorObj simple = extend;
           std::vector<Extension> result =
-            extendWithoutPseudo(extend, extensions, targetsUsed);
+            extendWithoutPseudo(simple, extensions, targetsUsed);
           if (result.empty()) result = { extensionForSimple(extend) };
           merged.push_back(result);
         }
@@ -858,10 +904,10 @@ namespace Sass {
   // ##########################################################################
   // Inner loop helper for [extendPseudo] function
   // ##########################################################################
-  std::vector<ComplexSelectorObj> extendPseudoComplex(
-    ComplexSelectorObj complex,
-    Pseudo_Selector_Obj pseudo,
-    CssMediaRule_Obj mediaQueryContext)
+  std::vector<ComplexSelectorObj> Extender::extendPseudoComplex(
+    const ComplexSelectorObj& complex,
+    const Pseudo_Selector_Obj& pseudo,
+    const CssMediaRule_Obj& mediaQueryContext)
   {
 
     if (complex->length() != 1) { return { complex }; }
@@ -909,9 +955,9 @@ namespace Sass {
   // a list of resulting pseudo selectors.
   // ##########################################################################
   std::vector<Pseudo_Selector_Obj> Extender::extendPseudo(
-    Pseudo_Selector_Obj pseudo,
-    ExtSelExtMap& extensions,
-    CssMediaRule_Obj mediaQueryContext)
+    const Pseudo_Selector_Obj& pseudo,
+    const ExtSelExtMap& extensions,
+    const CssMediaRule_Obj& mediaQueryContext)
   {
     auto selector = pseudo->selector();
     SelectorListObj extended = extendList(
@@ -930,7 +976,7 @@ namespace Sass {
       if (!hasAny(pseudo->selector()->elements(), hasMoreThanOne)) {
         if (hasAny(extended->elements(), hasExactlyOne)) {
           complexes.clear();
-          for (auto complex : extended->elements()) {
+          for (auto& complex : extended->elements()) {
             if (complex->length() <= 1) {
               complexes.push_back(complex);
             }
@@ -964,22 +1010,14 @@ namespace Sass {
   }
   // EO extendPseudo
 
-  // ##########################################################################
-  // ##########################################################################
-  bool dontTrimComplex(
-    const ComplexSelector* complex2,
-    const ComplexSelector* complex1,
-    const size_t maxSpecificity)
-  {
-    if (complex2->minSpecificity() < maxSpecificity) return false;
-    return complex2->isSuperselectorOf(complex1);
-  }
-
   // ##########################################################################
   // Rotates the element in list from [start] (inclusive) to [end] (exclusive)
   // one index higher, looping the final element back to [start].
   // ##########################################################################
-  void rotateSlice(std::vector<ComplexSelectorObj>& list, size_t start, size_t end) {
+  void Extender::rotateSlice(
+    std::vector<ComplexSelectorObj>& list,
+    size_t start, size_t end)
+  {
     auto element = list[end - 1];
     for (size_t i = start; i < end; i++) {
       auto next = list[i];
@@ -987,6 +1025,7 @@ namespace Sass {
       element = next;
     }
   }
+  // EO rotateSlice
 
   // ##########################################################################
   // Removes elements from [selectors] if they're subselectors of other
@@ -997,8 +1036,8 @@ namespace Sass {
   // code path in selector-trim that might need this special callback
   // ##########################################################################
   std::vector<ComplexSelectorObj> Extender::trim(
-    std::vector<ComplexSelectorObj> selectors,
-    ExtCplxSelSet& existing)
+    const std::vector<ComplexSelectorObj>& selectors,
+    const ExtCplxSelSet& existing)
   {
 
     // Avoid truly horrific quadratic behavior.
@@ -1039,8 +1078,8 @@ namespace Sass {
       // must be another selector that's a superselector of it *and*
       // that has specificity greater or equal to this.
       size_t maxSpecificity = 0;
-      for (SelectorComponentObj component : complex1->elements()) {
-        if (CompoundSelectorObj compound = Cast<CompoundSelector>(component)) {
+      for (const SelectorComponentObj& component : complex1->elements()) {
+        if (const CompoundSelectorObj compound = Cast<CompoundSelector>(component)) {
           maxSpecificity = std::max(maxSpecificity, maxSourceSpecificity(compound));
         }
       }
@@ -1070,16 +1109,21 @@ namespace Sass {
   }
   // EO trim
 
+  // ##########################################################################
   // Returns the maximum specificity of the given [simple] source selector.
-  size_t Extender::maxSourceSpecificity(SimpleSelectorObj simple)
+  // ##########################################################################
+  size_t Extender::maxSourceSpecificity(const SimpleSelectorObj& simple) const
   {
     auto it = sourceSpecificity.find(simple);
     if (it == sourceSpecificity.end()) return 0;
     return it->second;
   }
+  // EO maxSourceSpecificity(SimpleSelectorObj)
 
+  // ##########################################################################
   // Returns the maximum specificity for sources that went into producing [compound].
-  size_t Extender::maxSourceSpecificity(CompoundSelectorObj compound)
+  // ##########################################################################
+  size_t Extender::maxSourceSpecificity(const CompoundSelectorObj& compound) const
   {
     size_t specificity = 0;
     for (auto simple : compound->elements()) {
@@ -1088,5 +1132,37 @@ namespace Sass {
     }
     return specificity;
   }
+  // EO maxSourceSpecificity(CompoundSelectorObj)
+
+  // ##########################################################################
+  // Helper function used as callbacks on lists
+  // ##########################################################################
+  bool Extender::dontTrimComplex(
+    const ComplexSelector* complex2,
+    const ComplexSelector* complex1,
+    const size_t maxSpecificity)
+  {
+    if (complex2->minSpecificity() < maxSpecificity) return false;
+    return complex2->isSuperselectorOf(complex1);
+  }
+  // EO dontTrimComplex
+
+  // ##########################################################################
+  // Helper function used as callbacks on lists
+  // ##########################################################################
+  bool Extender::hasExactlyOne(const ComplexSelectorObj& vec)
+  {
+    return vec->length() == 1;
+  }
+  // EO hasExactlyOne
+
+  // ##########################################################################
+  // Helper function used as callbacks on lists
+  // ##########################################################################
+  bool Extender::hasMoreThanOne(const ComplexSelectorObj& vec)
+  {
+    return vec->length() > 1;
+  }
+  // hasMoreThanOne
 
 }
diff --git a/src/extender.hpp b/src/extender.hpp
index b13d311d1a..28d91e6f39 100644
--- a/src/extender.hpp
+++ b/src/extender.hpp
@@ -14,11 +14,9 @@
 
 namespace Sass {
 
-
-  typedef std::tuple<
-    SelectorListObj, // modified
-    SelectorListObj // original
-  > ExtSelTuple;
+  // ##########################################################################
+  // Different hash map types used by extender
+  // ##########################################################################
 
   // This is special (ptrs!)
   typedef std::unordered_set<
@@ -69,45 +67,66 @@ namespace Sass {
     ObjEquality
   > ExtByExtMap;
 
-
   class Extender : public Operation_CRTP<void, Extender> {
 
   public:
 
     enum ExtendMode { TARGETS, REPLACE, NORMAL, };
 
-  public:
+  private:
 
+    // ##########################################################################
     // The mode that controls this extender's behavior.
+    // ##########################################################################
     ExtendMode mode;
 
+    // ##########################################################################
     // Shared backtraces with context and expander. Needed the throw
     // errors when e.g. extending accross media query boundaries.
+    // ##########################################################################
     Backtraces& traces;
 
+    // ##########################################################################
     // A map from all simple selectors in the stylesheet to the rules that
     // contain them.This is used to find which rules an `@extend` applies to.
+    // ##########################################################################
     ExtSelMap selectors;
 
+  public:
+
+    // ##########################################################################
     // A map from all extended simple selectors
     // to the sources of those extensions.
+    // ##########################################################################
     ExtSelExtMap extensions;
 
+  private:
+
+    // ##########################################################################
     // A map from all simple selectors in extenders to
     // the extensions that those extenders define.
+    // ##########################################################################
     ExtByExtMap extensionsByExtender;
 
+    // ##########################################################################
     // A map from CSS rules to the media query contexts they're defined in.
     // This tracks the contexts in which each style rule is defined.
     // If a rule is defined at the top level, it doesn't have an entry.
+    // ##########################################################################
     ordered_map<
       SelectorListObj,
       CssMediaRule_Obj,
       ObjPtrHash,
       ObjPtrEquality
     > mediaContexts;
-
-    
+ 
+    // ##########################################################################
+    // A map from [SimpleSelector]s to the specificity of their source selectors.
+    // This tracks the maximum specificity of the [ComplexSelector] that originally 
+    // contained each [SimpleSelector]. This allows us to ensure we don't trim any
+    // selectors that need to exist to satisfy the [second law that of extend][].
+    // [second law of extend]: https://github.com/sass/sass/issues/324#issuecomment-4607184
+    // ##########################################################################
     std::unordered_map<
       SimpleSelectorObj,
       size_t,
@@ -115,90 +134,53 @@ namespace Sass {
       ObjEquality
     > sourceSpecificity;
 
+    // ##########################################################################
     // A set of [ComplexSelector]s that were originally part of their
     // component [SelectorList]s, as opposed to being added by `@extend`.
     // This allows us to ensure that we don't trim any selectors
     // that need to exist to satisfy the [first law of extend][].
+    // ##########################################################################
     ExtCplxSelSet originals;
 
-
   public:
 
-    Extender(Backtraces& traces);;
-    ~Extender() {};
-
+    // Constructor without default [mode].
+    // [traces] are needed to throw errors.
+    Extender(Backtraces& traces);
 
+    // ##########################################################################
+    // Constructor with specific [mode].
+    // [traces] are needed to throw errors.
+    // ##########################################################################
     Extender(ExtendMode mode, Backtraces& traces);
 
-    ExtSmplSelSet getSimpleSelectors() const;
-
-  public:
-    static SelectorListObj extend(SelectorListObj selector, SelectorListObj source, SelectorListObj target, Backtraces& traces);
-    static SelectorListObj replace(SelectorListObj selector, SelectorListObj source, SelectorListObj target, Backtraces& traces);
-
-    // Extends [list] using [extensions].
-    /*, List<CssMediaQuery> mediaQueryContext*/
-    void addExtension(SelectorListObj extender, SimpleSelectorObj target, ExtendRule_Obj extend, CssMediaRule_Obj mediaQueryContext);
-    SelectorListObj extendList(SelectorListObj list, ExtSelExtMap& extensions, CssMediaRule_Obj mediaContext);
-
-    void extendExistingStyleRules(
-      ExtListSelSet& rules,
-      ExtSelExtMap& newExtensions);
-
-    ExtSelExtMap extendExistingExtensions(
-      std::vector<Extension> extensions,
-      ExtSelExtMap& newExtensions);
-
-
-    size_t maxSourceSpecificity(SimpleSelectorObj simple);
-    size_t maxSourceSpecificity(CompoundSelectorObj compound);
-    Extension extensionForSimple(SimpleSelectorObj simple);
-    Extension extensionForCompound(std::vector<SimpleSelectorObj> simples);
-
-
-    std::vector<ComplexSelectorObj> extendComplex(ComplexSelectorObj list, ExtSelExtMap& extensions, CssMediaRule_Obj mediaQueryContext);
-    std::vector<ComplexSelectorObj> extendCompound(CompoundSelectorObj compound, ExtSelExtMap& extensions, CssMediaRule_Obj mediaQueryContext, bool inOriginal = false);
-    std::vector<std::vector<Extension>> extendSimple(SimpleSelectorObj simple, ExtSelExtMap& extensions, CssMediaRule_Obj mediaQueryContext, ExtSmplSelSet* targetsUsed);
-
-    std::vector<Pseudo_Selector_Obj> extendPseudo(Pseudo_Selector_Obj pseudo, ExtSelExtMap& extensions, CssMediaRule_Obj mediaQueryContext);
-
-    std::vector<ComplexSelectorObj> trim(std::vector<ComplexSelectorObj> selectors, ExtCplxSelSet& set);
-    
-
-
-  private:
-    std::vector<Extension> extendWithoutPseudo(SimpleSelectorObj simple, ExtSelExtMap& extensions, ExtSmplSelSet* targetsUsed);
-    static SelectorListObj extendOrReplace(SelectorListObj selector, SelectorListObj source, SelectorListObj target, ExtendMode mode, Backtraces& traces);
-
-  public:
-
-    // An [Extender] that contains no extensions and can have no extensions added.
-    // static const empty = EmptyExtender();
-
-    // A map from all simple selectors in the
-    // stylesheet to the rules that contain them.
-    // This is used to find which rules an `@extend` applies to.
-    // std::map<SimpleSelectorObj, Set<ModifiableCssStyleRule>> _selectors;
-
-    // A map from all extended simple selectors to the sources of those extensions.
-    // std::map<SimpleSelectorObj, std::map<ComplexSelectorObj, Extension, OrderNodes>> extensions;
-
-    // A map from all simple selectors in extenders to the extensions that those extenders define.
-    // std::map<SimpleSelectorObj, std::vector<Extension>> extensionsByExtender;
-
-    /// A set of [ComplexSelector]s that were originally part of
-    /// their component [SelectorList]s, as opposed to being added by `@extend`.
-    ///
-    /// This allows us to ensure that we don't trim any selectors that need to
-    /// exist to satisfy the [first law of extend][].
-    ///
-    /// [first law of extend]: https://github.com/sass/sass/issues/324#issuecomment-4607184
-    // std::set<ComplexSelectorObj> originals;
-
-
-
-
+    // ##########################################################################
+    // Empty desctructor
+    // ##########################################################################
+    ~Extender() {};
 
+    // ##########################################################################
+    // Extends [selector] with [source] extender and [targets] extendees.
+    // This works as though `source {@extend target}` were written in the
+    // stylesheet, with the exception that [target] can contain compound
+    // selectors which must be extended as a unit.
+    // ##########################################################################
+    static SelectorListObj extend(
+      SelectorListObj& selector,
+      SelectorListObj& source,
+      SelectorListObj& target,
+      Backtraces& traces);
+
+    // ##########################################################################
+    // Returns a copy of [selector] with [targets] replaced by [source].
+    // ##########################################################################
+    static SelectorListObj replace(
+      SelectorListObj& selector,
+      SelectorListObj& source,
+      SelectorListObj& target,
+      Backtraces& traces);
+
+    // ##########################################################################
     // Adds [selector] to this extender, with [selectorSpan] as the span covering
     // the selector and [ruleSpan] as the span covering the entire style rule.
     // Extends [selector] using any registered extensions, then returns an empty
@@ -206,12 +188,217 @@ namespace Sass {
     // extensions are added, the returned rule is automatically updated.
     // The [mediaContext] is the media query context in which the selector was
     // defined, or `null` if it was defined at the top level of the document.
-    void addSelector(SelectorListObj selector, CssMediaRule_Obj mediaContext);
+    // ##########################################################################
+    void addSelector(
+      const SelectorListObj& selector,
+      const CssMediaRule_Obj& mediaContext);
 
+    // ##########################################################################
     // Registers the [SimpleSelector]s in [list]
     // to point to [rule] in [selectors].
-    void registerSelector(SelectorListObj list, SelectorListObj rule);
+    // ##########################################################################
+    void registerSelector(
+      const SelectorListObj& list,
+      const SelectorListObj& rule);
+
+    // ##########################################################################
+    // Adds an extension to this extender. The [extender] is the selector for the
+    // style rule in which the extension is defined, and [target] is the selector
+    // passed to `@extend`. The [extend] provides the extend span and indicates 
+    // whether the extension is optional. The [mediaContext] defines the media query
+    // context in which the extension is defined. It can only extend selectors
+    // within the same context. A `null` context indicates no media queries.
+    // ##########################################################################
+    void addExtension(
+      SelectorListObj& extender,
+      SimpleSelectorObj& target,
+      // gets passed by pointer
+      ExtendRuleObj extend,
+      CssMediaRule_Obj& mediaQueryContext);
+
+    // ##########################################################################
+    // The set of all simple selectors in style rules handled
+    // by this extender. This includes simple selectors that
+    // were added because of downstream extensions.
+    // ##########################################################################
+    ExtSmplSelSet getSimpleSelectors() const;
+
+    // ##########################################################################
+    // Check for extends that have not been satisfied.
+    // Returns true if any non-optional extension did not
+    // extend any selector. Updates the passed reference
+    // to point to that Extension for further analysis.
+    // ##########################################################################
+    bool checkForUnsatisfiedExtends(Extension& unsatisfied) const;
 
+  private:
+
+    // ##########################################################################
+    // A helper function for [extend] and [replace].
+    // ##########################################################################
+    static SelectorListObj extendOrReplace(
+      SelectorListObj& selector,
+      SelectorListObj& source,
+      SelectorListObj& target,
+      ExtendMode mode,
+      Backtraces& traces);
+
+    // ##########################################################################
+    // Returns an extension that combines [left] and [right]. Throws 
+    // a [SassException] if [left] and [right] have incompatible 
+    // media contexts. Throws an [ArgumentError] if [left]
+    // and [right] don't have the same extender and target.
+    // ##########################################################################
+    static Extension mergeExtension(
+      const Extension& lhs,
+      const Extension& rhs);
+
+    // ##########################################################################
+    // Helper function to copy extension between maps
+    // ##########################################################################
+    static void mapCopyExts(
+      ExtSelExtMap& dest,
+      const ExtSelExtMap& source);
+
+    // ##########################################################################
+    // Extend [extensions] using [newExtensions].
+    // ##########################################################################
+    // Note: dart-sass throws an error in here
+    // ##########################################################################
+    void extendExistingStyleRules(
+      const ExtListSelSet& rules,
+      const ExtSelExtMap& newExtensions);
+
+    // ##########################################################################
+    // Extend [extensions] using [newExtensions]. Note that this does duplicate
+    // some work done by [_extendExistingStyleRules],  but it's necessary to
+    // expand each extension's extender separately without reference to the full
+    // selector list, so that relevant results don't get trimmed too early.
+    // Returns `null` (Note: empty map) if there are no extensions to add.
+    // ##########################################################################
+    ExtSelExtMap extendExistingExtensions(
+      const std::vector<Extension>& extensions,
+      ExtSelExtMap& newExtensions);
+
+    // ##########################################################################
+    // Extends [list] using [extensions].
+    // ##########################################################################
+    SelectorListObj extendList(
+      const SelectorListObj& list,
+      const ExtSelExtMap& extensions,
+      const CssMediaRule_Obj& mediaContext);
+
+    // ##########################################################################
+    // Extends [complex] using [extensions], and
+    // returns the contents of a [SelectorList].
+    // ##########################################################################
+    std::vector<ComplexSelectorObj> extendComplex(
+      const ComplexSelectorObj& list,
+      const ExtSelExtMap& extensions,
+      const CssMediaRule_Obj& mediaQueryContext);
+
+    // ##########################################################################
+    // Returns a one-off [Extension] whose
+    // extender is composed solely of [simple].
+    // ##########################################################################
+    Extension extensionForSimple(
+      const SimpleSelectorObj& simple);
+
+    // ##########################################################################
+    // Returns a one-off [Extension] whose extender is composed
+    // solely of a compound selector containing [simples].
+    // ##########################################################################
+    Extension extensionForCompound(
+      const std::vector<SimpleSelectorObj>& simples);
+
+    // ##########################################################################
+    // Extends [compound] using [extensions], and returns the
+    // contents of a [SelectorList]. The [inOriginal] parameter
+    // indicates whether this is in an original complex selector,
+    // meaning that [compound] should not be trimmed out.
+    // ##########################################################################
+    std::vector<ComplexSelectorObj> extendCompound(
+      const CompoundSelectorObj& compound,
+      const ExtSelExtMap& extensions,
+      const CssMediaRule_Obj& mediaQueryContext,
+      bool inOriginal = false);
+
+    // ##########################################################################
+    // Extends [simple] without extending the
+    // contents of any selector pseudos it contains.
+    // ##########################################################################
+    std::vector<Extension> extendWithoutPseudo(
+      const SimpleSelectorObj& simple,
+      const ExtSelExtMap& extensions,
+      ExtSmplSelSet* targetsUsed);
+
+    // ##########################################################################
+    // Extends [simple] and also extending the
+    // contents of any selector pseudos it contains.
+    // ##########################################################################
+    std::vector<std::vector<Extension>> extendSimple(
+      const SimpleSelectorObj& simple,
+      const ExtSelExtMap& extensions,
+      const CssMediaRuleObj& mediaQueryContext,
+      ExtSmplSelSet* targetsUsed);
+
+    // ##########################################################################
+    // Inner loop helper for [extendPseudo] function
+    // ##########################################################################
+    static std::vector<ComplexSelectorObj> extendPseudoComplex(
+      const ComplexSelectorObj& complex,
+      const Pseudo_Selector_Obj& pseudo,
+      const CssMediaRule_Obj& mediaQueryContext);
+
+    // ##########################################################################
+    // Extends [pseudo] using [extensions], and returns
+    // a list of resulting pseudo selectors.
+    // ##########################################################################
+    std::vector<Pseudo_Selector_Obj> extendPseudo(
+      const Pseudo_Selector_Obj& pseudo,
+      const ExtSelExtMap& extensions,
+      const CssMediaRule_Obj& mediaQueryContext);
+
+    // ##########################################################################
+    // Rotates the element in list from [start] (inclusive) to [end] (exclusive)
+    // one index higher, looping the final element back to [start].
+    // ##########################################################################
+    static void rotateSlice(
+      std::vector<ComplexSelectorObj>& list,
+      size_t start, size_t end);
+
+    // ##########################################################################
+    // Removes elements from [selectors] if they're subselectors of other
+    // elements. The [isOriginal] callback indicates which selectors are
+    // original to the document, and thus should never be trimmed.
+    // ##########################################################################
+    std::vector<ComplexSelectorObj> trim(
+      const std::vector<ComplexSelectorObj>& selectors,
+      const ExtCplxSelSet& set);
+
+    // ##########################################################################
+    // Returns the maximum specificity of the given [simple] source selector.
+    // ##########################################################################
+    size_t maxSourceSpecificity(const SimpleSelectorObj& simple) const;
+
+    // ##########################################################################
+    // Returns the maximum specificity for sources that went into producing [compound].
+    // ##########################################################################
+    size_t maxSourceSpecificity(const CompoundSelectorObj& compound) const;
+
+    // ##########################################################################
+    // Helper function used as callbacks on lists
+    // ##########################################################################
+    static bool dontTrimComplex(
+      const ComplexSelector* complex2,
+      const ComplexSelector* complex1,
+      const size_t maxSpecificity);
+
+    // ##########################################################################
+    // Helper function used as callbacks on lists
+    // ##########################################################################
+    static bool hasExactlyOne(const ComplexSelectorObj& vec);
+    static bool hasMoreThanOne(const ComplexSelectorObj& vec);
 
   };
 
diff --git a/src/extension.cpp b/src/extension.cpp
index 70e506c972..e48a34c369 100644
--- a/src/extension.cpp
+++ b/src/extension.cpp
@@ -24,7 +24,7 @@ namespace Sass {
   // Asserts that the [mediaContext] for a selector is
   // compatible with the query context for this extender.
   // ##########################################################################
-  void Extension::assertCompatibleMediaContext(CssMediaRule_Obj mediaQueryContext, Backtraces& traces)
+  void Extension::assertCompatibleMediaContext(CssMediaRule_Obj mediaQueryContext, Backtraces& traces) const
   {
 
     if (this->mediaContext.isNull()) return;
diff --git a/src/extension.hpp b/src/extension.hpp
index 242b8810ec..f7bed60f5b 100644
--- a/src/extension.hpp
+++ b/src/extension.hpp
@@ -78,7 +78,7 @@ namespace Sass {
 
     // Asserts that the [mediaContext] for a selector is 
     // compatible with the query context for this extender.
-    void assertCompatibleMediaContext(CssMediaRule_Obj mediaContext, Backtraces& traces);
+    void assertCompatibleMediaContext(CssMediaRule_Obj mediaContext, Backtraces& traces) const;
 
     Extension withExtender(ComplexSelectorObj newExtender);