Add comparison operators for SharedImpl, fix bugs

Based on: https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp All of the `0` comparisons here were bugs and never worked as intended.
PolyPik · Jun 15, 2019 · dfe23ac · dfe23ac
1 parent 0328de2
commit dfe23ac
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 10 deletions.
diff --git a/src/ast.cpp b/src/ast.cpp
@@ -184,7 +184,7 @@ namespace Sass {
   /////////////////////////////////////////////////////////////////////////
 
   Bubble::Bubble(ParserState pstate, Statement_Obj n, Statement_Obj g, size_t t)
-  : Statement(pstate, Statement::BUBBLE, t), node_(n), group_end_(g == 0)
+  : Statement(pstate, Statement::BUBBLE, t), node_(n), group_end_(g == nullptr)
   { }
   Bubble::Bubble(const Bubble* ptr)
   : Statement(ptr),
@@ -834,7 +834,7 @@ namespace Sass {
   }
 
   bool At_Root_Block::exclude_node(Statement_Obj s) {
-    if (expression() == 0)
+    if (expression() == nullptr)
     {
       return s->statement_type() == Statement::RULESET;
     }

diff --git a/src/ast.hpp b/src/ast.hpp
@@ -312,7 +312,7 @@ namespace Sass {
     bool empty() const                     { return list_.empty(); }
     bool has(Expression_Obj k) const          { return elements_.count(k) == 1; }
     Expression_Obj at(Expression_Obj k) const;
-    bool has_duplicate_key() const         { return duplicate_key_ != 0; }
+    bool has_duplicate_key() const         { return duplicate_key_ != nullptr; }
     Expression_Obj get_duplicate_key() const  { return duplicate_key_; }
     const ExpressionMap elements() { return elements_; }
     Hashed& operator<<(std::pair<Expression_Obj, Expression_Obj> p)

diff --git a/src/expand.cpp b/src/expand.cpp
@@ -104,7 +104,7 @@ namespace Sass {
     bool has_parent_selector = false;
     for (size_t i = 0, L = selector_stack.size(); i < L && !has_parent_selector; i++) {
       Selector_List_Obj ll = selector_stack.at(i);
-      has_parent_selector = ll != 0 && ll->length() > 0;
+      has_parent_selector = ll != nullptr && ll->length() > 0;
     }
 
     Selector_List_Obj sel = r->selector();
@@ -614,7 +614,7 @@ namespace Sass {
 
 
     Selector_List_Obj contextualized = Cast<Selector_List>(s->perform(&eval));
-    if (contextualized == false) return;
+    if (contextualized == nullptr) return;
     for (auto complex_sel : contextualized->elements()) {
       Complex_Selector_Obj c = complex_sel;
       if (!c->head() || c->tail()) {

diff --git a/src/inspect.cpp b/src/inspect.cpp
@@ -1060,7 +1060,7 @@ namespace Sass {
 
     for (size_t i = 0, L = g->length(); i < L; ++i) {
       if (!in_wrapped && i == 0) append_indentation();
-      if ((*g)[i] == 0) continue;
+      if ((*g)[i] == nullptr) continue;
       schedule_mapping(g->at(i)->last());
       // add_open_mapping((*g)[i]->last());
       (*g)[i]->perform(this);

diff --git a/src/memory/SharedPtr.hpp b/src/memory/SharedPtr.hpp
@@ -3,8 +3,10 @@
 
 #include "sass/base.h"
 
+#include <cstddef>
 #include <iostream>
 #include <string>
+#include <type_traits>
 #include <vector>
 
 namespace Sass {
@@ -185,6 +187,100 @@ namespace Sass {
     T* detach() { return static_cast<T*>(SharedPtr::detach()); }
   };
 
-}
+  // Comparison operators, based on:
+  // https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp
+
+  template<class T1, class T2>
+  bool operator==(const SharedImpl<T1>& x, const SharedImpl<T2>& y) {
+    return x.ptr() == y.ptr();
+  }
+
+  template<class T1, class T2>
+  bool operator!=(const SharedImpl<T1>& x, const SharedImpl<T2>& y) {
+    return x.ptr() != y.ptr();
+  }
+
+  template<class T1, class T2>
+  bool operator<(const SharedImpl<T1>& x, const SharedImpl<T2>& y) {
+    using CT = typename std::common_type<T1*, T2*>::type;
+    return std::less<CT>()(x.get(), y.get());
+  }
+
+  template<class T1, class T2>
+  bool operator<=(const SharedImpl<T1>& x, const SharedImpl<T2>& y) {
+    return !(y < x);
+  }
+
+  template<class T1, class T2>
+  bool operator>(const SharedImpl<T1>& x, const SharedImpl<T2>& y) {
+    return y < x;
+  }
+
+  template<class T1, class T2>
+  bool operator>=(const SharedImpl<T1>& x, const SharedImpl<T2>& y) {
+    return !(x < y);
+  }
+
+  template <class T>
+  bool operator==(const SharedImpl<T>& x, std::nullptr_t) noexcept {
+    return x.isNull();
+  }
+
+  template <class T>
+  bool operator==(std::nullptr_t, const SharedImpl<T>& x) noexcept {
+    return x.isNull();
+  }
+
+  template <class T>
+  bool operator!=(const SharedImpl<T>& x, std::nullptr_t) noexcept {
+    return !x.isNull();
+  }
+
+  template <class T>
+  bool operator!=(std::nullptr_t, const SharedImpl<T>& x) noexcept {
+    return !x.isNull();
+  }
+
+  template <class T>
+  bool operator<(const SharedImpl<T>& x, std::nullptr_t) {
+    return std::less<T*>()(x.get(), nullptr);
+  }
+
+  template <class T>
+  bool operator<(std::nullptr_t, const SharedImpl<T>& y) {
+    return std::less<T*>()(nullptr, y.get());
+  }
+
+  template <class T>
+  bool operator<=(const SharedImpl<T>& x, std::nullptr_t) {
+    return !(nullptr < x);
+  }
+
+  template <class T>
+  bool operator<=(std::nullptr_t, const SharedImpl<T>& y) {
+    return !(y < nullptr);
+  }
+
+  template <class T>
+  bool operator>(const SharedImpl<T>& x, std::nullptr_t) {
+    return nullptr < x;
+  }
+
+  template <class T>
+  bool operator>(std::nullptr_t, const SharedImpl<T>& y) {
+    return y < nullptr;
+  }
+
+  template <class T>
+  bool operator>=(const SharedImpl<T>& x, std::nullptr_t) {
+    return !(x < nullptr);
+  }
+
+  template <class T>
+  bool operator>=(std::nullptr_t, const SharedImpl<T>& y) {
+    return !(nullptr < y);
+  }
+
+}  // namespace Sass
 
 #endif
diff --git a/src/parser.cpp b/src/parser.cpp
@@ -2488,7 +2488,7 @@ namespace Sass {
   Supports_Condition_Obj Parser::parse_supports_condition_in_parens(bool parens_required)
   {
     Supports_Condition_Obj interp = parse_supports_interpolation();
-    if (interp != 0) return interp;
+    if (interp != nullptr) return interp;
 
     if (!lex < exactly <'('> >()) {
       if (parens_required) {

diff --git a/src/util.cpp b/src/util.cpp
@@ -627,9 +627,9 @@ namespace Sass {
 
     bool isPrintable(Media_Block* m, Sass_Output_Style style)
     {
-      if (m == 0) return false;
+      if (m == nullptr) return false;
       Block_Obj b = m->block();
-      if (b == 0) return false;
+      if (b == nullptr) return false;
       for (size_t i = 0, L = b->length(); i < L; ++i) {
         Statement_Obj stm = b->at(i);
         if (Cast<Directive>(stm)) return true;

diff --git a/test/test_shared_ptr.cpp b/test/test_shared_ptr.cpp
@@ -135,6 +135,32 @@ bool TestDetachNull() {
   return true;
 }
 
+class EmptyTestObj : public Sass::SharedObj {
+  public:
+    const std::string to_string() const { return ""; }
+};
+
+bool TestComparisonWithSharedPtr() {
+  Sass::SharedImpl<EmptyTestObj> a = new EmptyTestObj();
+  ASSERT(a == a);
+  Sass::SharedImpl<EmptyTestObj> b = a;
+  ASSERT(a == b);
+  Sass::SharedImpl<EmptyTestObj> c = new EmptyTestObj();
+  ASSERT(a != c);
+  Sass::SharedImpl<EmptyTestObj> nullobj;
+  ASSERT(a != nullobj);
+  ASSERT(nullobj == nullobj);
+  return true;
+}
+
+bool TestComparisonWithNullptr() {
+  Sass::SharedImpl<EmptyTestObj> a = new EmptyTestObj();
+  ASSERT(a != nullptr);
+  Sass::SharedImpl<EmptyTestObj> nullobj;
+  ASSERT(nullobj == nullptr);
+  return true;
+}
+
 #define TEST(fn) \
   if (fn()) { \
     passed.push_back(#fn); \
@@ -155,6 +181,8 @@ int main(int argc, char **argv) {
   TEST(TestSelfAssignDetach);
   TEST(TestDetachedPtrIsNotDestroyedUntilAssignment);
   TEST(TestDetachNull);
+  TEST(TestComparisonWithSharedPtr);
+  TEST(TestComparisonWithNullptr);
   std::cerr << argv[0] << ": Passed: " << passed.size()
             << ", failed: " << failed.size()
             << "." << std::endl;