Various improvements to the Patricia tree library

Summary:
- Fixes a bug in brace-initialization of Patricia-tree sets.

  - Improves the pretty-printing of Patricia-tree collections containing pointers (don't print the pointer, but the object itself).

  - Removes the `get_patricia_tree()` method and implements the `stable_equals()` predicate instead, since this is the only practical case where we need to compare the pointers to the underlying Patricia trees.

Reviewed By: int3

Differential Revision: D6446589

fbshipit-source-id: 55726dde5f9b3bc9a60f6246169fc133bee991ee

libredex/PatriciaTreeMap.h

@@ -11,7 +11,6 @@
 
 #include <cstdint>
 #include <functional>
-#include <initializer_list>
 #include <iostream>
 #include <iterator>
 #include <memory>
@@ -184,6 +183,28 @@ class PatriciaTreeMap final {
     return ptmap_impl::equals<IntegerType>(m_tree, other.m_tree);
   }
 
+  /*
+   * This faster equality predicate can be used to check whether a sequence of
+   * in-place modifications leaves a Patricia-tree map unchanged. For comparing
+   * two arbitrary Patricia-tree maps, one needs to use the `equals()`
+   * predicate.
+   *
+   * Example:
+   *
+   *   PatriciaTreeMap<...> m1, m2;
+   *   ...
+   *   m2 = m1;
+   *   m2.union_with(...);
+   *   m2.update(...);
+   *   m2.intersection_with(...);
+   *   if (m2.reference_equals(m1)) { // This is equivalent to m2.equals(m1)
+   *     ...
+   *   }
+   */
+  bool reference_equals(const PatriciaTreeMap& other) const {
+    return m_tree == other.m_tree;
+  }
+
   PatriciaTreeMap& update(
       const std::function<mapped_type(const mapped_type&)>& operation,
       Key key) {
@@ -233,11 +254,6 @@ class PatriciaTreeMap final {
 
   void clear() { m_tree.reset(); }
 
-  std::shared_ptr<ptmap_impl::PatriciaTree<IntegerType, Value>>
-  get_patricia_tree() const {
-    return m_tree;
-  }
-
  private:
   // These functions are used to handle the type conversions required when
   // manipulating sets of pointers. The first parameter is necessary to make
@@ -266,6 +282,18 @@ class PatriciaTreeMap final {
     return x;
   }
 
+  template <typename T = Key,
+            typename = typename std::enable_if_t<std::is_pointer<T>::value>>
+  static typename std::remove_pointer<T>::type deref(Key x) {
+    return *x;
+  }
+
+  template <typename T = Key,
+            typename = typename std::enable_if_t<!std::is_pointer<T>::value>>
+  static Key deref(Key x) {
+    return x;
+  }
+
   std::shared_ptr<ptmap_impl::PatriciaTree<IntegerType, Value>> m_tree;
 
   template <typename T, typename V>
@@ -280,7 +308,7 @@ inline std::ostream& operator<<(std::ostream& o,
                                 const PatriciaTreeMap<Key, Value>& s) {
   o << "{";
   for (auto it = s.begin(); it != s.end(); ++it) {
-    o << PatriciaTreeMap<Key, Value>::decode(it->first) << " -> " << it->second;
+    o << PatriciaTreeMap<Key, Value>::deref(it->first) << " -> " << it->second;
     if (std::next(it) != s.end()) {
       o << ", ";
     }

libredex/PatriciaTreeSet.h

@@ -110,7 +110,7 @@ class PatriciaTreeSet final {
 
   explicit PatriciaTreeSet(std::initializer_list<Element> l) {
     for (Element x : l) {
-      insert(encode(x));
+      insert(x);
     }
   }
 
@@ -150,6 +150,28 @@ class PatriciaTreeSet final {
     return pt_impl::equals<IntegerType>(m_tree, other.m_tree);
   }
 
+  /*
+   * This faster equality predicate can be used to check whether a sequence of
+   * in-place modifications leaves a Patricia-tree set unchanged. For comparing
+   * two arbitrary Patricia-tree sets, one needs to use the `equals()`
+   * predicate.
+   *
+   * Example:
+   *
+   *   PatriciaTreeSet<...> s, t;
+   *   ...
+   *   t = s;
+   *   t.union_with(...);
+   *   t.remove(...);
+   *   t.intersection_with(...);
+   *   if (s.reference_equals(t)) { // This test is equivalent to s.equals(t)
+   *     ...
+   *   }
+   */
+  bool reference_equals(const PatriciaTreeSet& other) const {
+    return m_tree == other.m_tree;
+  }
+
   PatriciaTreeSet& insert(Element key) {
     m_tree = pt_impl::insert<IntegerType>(encode(key), m_tree);
     return *this;
@@ -193,11 +215,6 @@ class PatriciaTreeSet final {
 
   void clear() { m_tree.reset(); }
 
-  std::shared_ptr<pt_impl::PatriciaTree<IntegerType>> get_patricia_tree()
-      const {
-    return m_tree;
-  }
-
  private:
   // These functions are used to handle the type conversions required when
   // manipulating sets of pointers. The first parameter is necessary to make
@@ -226,6 +243,18 @@ class PatriciaTreeSet final {
     return x;
   }
 
+  template <typename T = Element,
+            typename = typename std::enable_if_t<std::is_pointer<T>::value>>
+  static typename std::remove_pointer<T>::type deref(Element x) {
+    return *x;
+  }
+
+  template <typename T = Element,
+            typename = typename std::enable_if_t<!std::is_pointer<T>::value>>
+  static Element deref(Element x) {
+    return x;
+  }
+
   std::shared_ptr<pt_impl::PatriciaTree<IntegerType>> m_tree;
 
   template <typename T>
@@ -240,7 +269,7 @@ inline std::ostream& operator<<(std::ostream& o,
                                 const PatriciaTreeSet<Element>& s) {
   o << "{";
   for (auto it = s.begin(); it != s.end(); ++it) {
-    o << PatriciaTreeSet<Element>::decode(*it);
+    o << PatriciaTreeSet<Element>::deref(*it);
     if (std::next(it) != s.end()) {
       o << ", ";
     }

test/unit/PatriciaTreeMapAbstractEnvironmentTest.cpp

@@ -12,6 +12,7 @@
 #include <gtest/gtest.h>
 #include <limits>
 #include <random>
+#include <sstream>
 
 #include "HashedAbstractEnvironment.h"
 #include "HashedSetAbstractDomain.h"
@@ -247,11 +248,22 @@ TEST_F(PatriciaTreeMapAbstractEnvironmentTest, whiteBox) {
   // operation shares structure with the operands whenever possible). This is
   // what we check here.
   Environment e({{1, Domain({"a"})}});
-  auto tree_before = e.bindings().get_patricia_tree();
+  const auto& before = e.bindings();
   e.update(1, [](const Domain& x) { return Domain({"a"}); });
-  EXPECT_EQ(e.bindings().get_patricia_tree(), tree_before);
+  EXPECT_TRUE(e.bindings().reference_equals(before));
   e.meet_with(e);
-  EXPECT_EQ(e.bindings().get_patricia_tree(), tree_before);
+  EXPECT_TRUE(e.bindings().reference_equals(before));
   e.join_with(e);
-  EXPECT_EQ(e.bindings().get_patricia_tree(), tree_before);
+  EXPECT_TRUE(e.bindings().reference_equals(before));
+}
+
+TEST_F(PatriciaTreeMapAbstractEnvironmentTest, prettyPrinting) {
+  using StringEnvironment =
+      PatriciaTreeMapAbstractEnvironment<std::string*, Domain>;
+  std::string a = "a";
+  StringEnvironment e({{&a, Domain("A")}});
+
+  std::ostringstream out;
+  out << e.bindings();
+  EXPECT_EQ("{a -> [#1]{A}}", out.str());
 }

test/unit/PatriciaTreeSetTest.cpp

@@ -111,6 +111,8 @@ TEST_F(PatriciaTreeSetTest, basicOperations) {
     std::ostringstream out;
     out << s2;
     EXPECT_EQ("{0, 2, 3, 1023}", out.str());
+    pt_set s_init_list({0, 2, 3, 1023});
+    EXPECT_TRUE(s_init_list.equals(s2));
   }
 
   EXPECT_TRUE(empty_set.is_subset_of(s1));
@@ -190,29 +192,29 @@ TEST_F(PatriciaTreeSetTest, whiteBox) {
     pt_set s = this->generate_random_set();
     pt_set u = s.get_union_with(s);
     pt_set i = s.get_intersection_with(s);
-    EXPECT_EQ(s.get_patricia_tree(), u.get_patricia_tree());
-    EXPECT_EQ(s.get_patricia_tree(), i.get_patricia_tree());
+    EXPECT_TRUE(s.reference_equals(u));
+    EXPECT_TRUE(s.reference_equals(i));
     {
       s.insert(17);
-      auto tree = s.get_patricia_tree();
+      pt_set s0 = s;
       s.insert(17);
-      EXPECT_EQ(tree, s.get_patricia_tree());
+      EXPECT_TRUE(s.reference_equals(s0));
     }
     {
       s.remove(157);
-      auto tree = s.get_patricia_tree();
+      pt_set s0 = s;
       s.remove(157);
-      EXPECT_EQ(tree, s.get_patricia_tree());
+      EXPECT_TRUE(s.reference_equals(s0));
     }
     pt_set t = this->generate_random_set();
     pt_set ust = s.get_union_with(t);
     pt_set ist = s.get_intersection_with(t);
-    auto ust_tree = ust.get_patricia_tree();
-    auto ist_tree = ist.get_patricia_tree();
+    pt_set ust0 = ust;
+    pt_set ist0 = ist;
     ust.union_with(t);
     ist.intersection_with(t);
-    EXPECT_EQ(ust.get_patricia_tree(), ust_tree);
-    EXPECT_EQ(ist.get_patricia_tree(), ist_tree);
+    EXPECT_TRUE(ust.reference_equals(ust0));
+    EXPECT_TRUE(ist.reference_equals(ist0));
   }
 }
 
@@ -251,4 +253,9 @@ TEST_F(PatriciaTreeSetTest, setsOfPointers) {
   EXPECT_TRUE(s.equals(s_ab));
   s.filter([](std::string* x) { return *x > "g"; });
   EXPECT_TRUE(s.is_empty());
+
+  string_set t({&a});
+  std::ostringstream out;
+  out << t;
+  EXPECT_EQ("{a}", out.str());
 }