From dfe23ac550de902289e5e5d5f4ece794afbb68d3 Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Sat, 15 Jun 2019 12:48:27 +0100 Subject: [PATCH 1/2] 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. --- src/ast.cpp | 4 +- src/ast.hpp | 2 +- src/expand.cpp | 4 +- src/inspect.cpp | 2 +- src/memory/SharedPtr.hpp | 98 +++++++++++++++++++++++++++++++++++++++- src/parser.cpp | 2 +- src/util.cpp | 4 +- test/test_shared_ptr.cpp | 28 ++++++++++++ 8 files changed, 134 insertions(+), 10 deletions(-) diff --git a/src/ast.cpp b/src/ast.cpp index 09c65274e2..2ac93e8fd1 100644 --- 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 index c6af3ed62e..51d03caa24 100644 --- 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 p) diff --git a/src/expand.cpp b/src/expand.cpp index bb1fab4baa..ef56838262 100644 --- 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(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 index b0e3ab8cd0..3dd6b700fb 100644 --- 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 index 1c9d5653fc..da2b0d53ee 100644 --- a/src/memory/SharedPtr.hpp +++ b/src/memory/SharedPtr.hpp @@ -3,8 +3,10 @@ #include "sass/base.h" +#include #include #include +#include #include namespace Sass { @@ -185,6 +187,100 @@ namespace Sass { T* detach() { return static_cast(SharedPtr::detach()); } }; -} + // Comparison operators, based on: + // https://en.cppreference.com/w/cpp/memory/unique_ptr/operator_cmp + + template + bool operator==(const SharedImpl& x, const SharedImpl& y) { + return x.ptr() == y.ptr(); + } + + template + bool operator!=(const SharedImpl& x, const SharedImpl& y) { + return x.ptr() != y.ptr(); + } + + template + bool operator<(const SharedImpl& x, const SharedImpl& y) { + using CT = typename std::common_type::type; + return std::less()(x.get(), y.get()); + } + + template + bool operator<=(const SharedImpl& x, const SharedImpl& y) { + return !(y < x); + } + + template + bool operator>(const SharedImpl& x, const SharedImpl& y) { + return y < x; + } + + template + bool operator>=(const SharedImpl& x, const SharedImpl& y) { + return !(x < y); + } + + template + bool operator==(const SharedImpl& x, std::nullptr_t) noexcept { + return x.isNull(); + } + + template + bool operator==(std::nullptr_t, const SharedImpl& x) noexcept { + return x.isNull(); + } + + template + bool operator!=(const SharedImpl& x, std::nullptr_t) noexcept { + return !x.isNull(); + } + + template + bool operator!=(std::nullptr_t, const SharedImpl& x) noexcept { + return !x.isNull(); + } + + template + bool operator<(const SharedImpl& x, std::nullptr_t) { + return std::less()(x.get(), nullptr); + } + + template + bool operator<(std::nullptr_t, const SharedImpl& y) { + return std::less()(nullptr, y.get()); + } + + template + bool operator<=(const SharedImpl& x, std::nullptr_t) { + return !(nullptr < x); + } + + template + bool operator<=(std::nullptr_t, const SharedImpl& y) { + return !(y < nullptr); + } + + template + bool operator>(const SharedImpl& x, std::nullptr_t) { + return nullptr < x; + } + + template + bool operator>(std::nullptr_t, const SharedImpl& y) { + return y < nullptr; + } + + template + bool operator>=(const SharedImpl& x, std::nullptr_t) { + return !(x < nullptr); + } + + template + bool operator>=(std::nullptr_t, const SharedImpl& y) { + return !(nullptr < y); + } + +} // namespace Sass #endif diff --git a/src/parser.cpp b/src/parser.cpp index a697d20c01..411b052632 100644 --- 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 index 62f5d899a7..1ac9194f87 100644 --- 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(stm)) return true; diff --git a/test/test_shared_ptr.cpp b/test/test_shared_ptr.cpp index 6b1ccc2e93..b7b1d3d686 100644 --- 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 a = new EmptyTestObj(); + ASSERT(a == a); + Sass::SharedImpl b = a; + ASSERT(a == b); + Sass::SharedImpl c = new EmptyTestObj(); + ASSERT(a != c); + Sass::SharedImpl nullobj; + ASSERT(a != nullobj); + ASSERT(nullobj == nullobj); + return true; +} + +bool TestComparisonWithNullptr() { + Sass::SharedImpl a = new EmptyTestObj(); + ASSERT(a != nullptr); + Sass::SharedImpl 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; From 0bab610a893c5bf63f693bd6c9549e2368c1804f Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Sat, 15 Jun 2019 14:03:26 +0100 Subject: [PATCH 2/2] VS2013 noexcept workaround --- include/sass/base.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/sass/base.h b/include/sass/base.h index eee8327bed..a91dbc9537 100644 --- a/include/sass/base.h +++ b/include/sass/base.h @@ -17,6 +17,12 @@ #endif #endif +// Work around lack of `noexcept` keyword support in VS2013 +#if defined(_MSC_VER) && (_MSC_VER <= 1800) && !defined(_ALLOW_KEYWORD_MACROS) +#define _ALLOW_KEYWORD_MACROS 1 +#define noexcept throw( ) +#endif + #include #include