From bcc489c3d07de9f008eb22d6564c3b9d6da21bff Mon Sep 17 00:00:00 2001 From: Daniel Poetzl Date: Thu, 1 Mar 2018 17:23:00 +0000 Subject: [PATCH 1/6] Move sharing map unit tests to catch --- unit/Makefile | 1 + unit/sharing_map.cpp | 187 ++++++++++++++++++++++-------------------- unit/sharing_node.cpp | 11 +-- 3 files changed, 102 insertions(+), 97 deletions(-) diff --git a/unit/Makefile b/unit/Makefile index 05f188506aa..8fccc515dd1 100644 --- a/unit/Makefile +++ b/unit/Makefile @@ -28,6 +28,7 @@ SRC += unit_tests.cpp \ java_bytecode/inherited_static_fields/inherited_static_fields.cpp \ pointer-analysis/custom_value_set_analysis.cpp \ sharing_node.cpp \ + sharing_map.cpp \ solvers/refinement/string_constraint_generator_valueof/calculate_max_string_length.cpp \ solvers/refinement/string_constraint_generator_valueof/get_numeric_value_from_character.cpp \ solvers/refinement/string_constraint_generator_valueof/is_digit_with_radix.cpp \ diff --git a/unit/sharing_map.cpp b/unit/sharing_map.cpp index 9911f460855..746d9376946 100644 --- a/unit/sharing_map.cpp +++ b/unit/sharing_map.cpp @@ -1,6 +1,12 @@ -#include -#include +/*******************************************************************\ +Module: Unit tests for sharing map + +Author: Daniel Poetzl + +\*******************************************************************/ + +#include #include typedef sharing_mapt smt; @@ -24,50 +30,48 @@ void fill2(smt &sm) void sharing_map_interface_test() { - std::cout << "Running interface test" << std::endl; - - // empty map + SECTION("Empty map") { smt sm; - assert(sm.empty()); - assert(sm.size()==0); - assert(!sm.has_key("i")); + REQUIRE(sm.empty()); + REQUIRE(sm.size() == 0); + REQUIRE(!sm.has_key("i")); } - // insert elements + SECTION("Insert elements") { smt sm; smt::const_find_type r1=sm.insert(std::make_pair("i", "0")); - assert(r1.second); + REQUIRE(r1.second); smt::const_find_type r2=sm.insert("j", "1"); - assert(r2.second); + REQUIRE(r2.second); smt::const_find_type r3=sm.insert(std::make_pair("i", "0")); - assert(!r3.second); + REQUIRE(!r3.second); - assert(sm.size()==2); - assert(!sm.empty()); + REQUIRE(sm.size() == 2); + REQUIRE(!sm.empty()); } - // place elements + SECTION("Place elements") { smt sm1; smt sm2(sm1); smt::find_type r1=sm1.place("i", "0"); - assert(r1.second); - assert(!sm2.has_key("i")); + REQUIRE(r1.second); + REQUIRE(!sm2.has_key("i")); std::string &s1=r1.first; s1="1"; - assert(sm1.at("i")=="1"); + REQUIRE(sm1.at("i") == "1"); } - // retrieve elements + SECTION("Retrieve elements") { smt sm; sm.insert("i", "0"); @@ -77,109 +81,107 @@ void sharing_map_interface_test() std::string s; s=sm2.at("i"); - assert(s=="0"); + REQUIRE(s == "0"); try { sm2.at("k"); - assert(false); + REQUIRE(false); } catch (...) {} s=sm2.at("j"); - assert(s=="1"); + REQUIRE(s == "1"); - assert(sm.has_key("i")); - assert(sm.has_key("j")); - assert(!sm.has_key("k")); + REQUIRE(sm.has_key("i")); + REQUIRE(sm.has_key("j")); + REQUIRE(!sm.has_key("k")); std::string &s2=sm.at("i"); s2="3"; - assert(sm2.at("i")=="3"); + REQUIRE(sm2.at("i") == "3"); - assert(sm.size()==2); + REQUIRE(sm.size() == 2); smt::find_type r=sm.find("i"); - assert(r.second); + REQUIRE(r.second); r.first="4"; - assert(sm2.at("i")=="4"); + REQUIRE(sm2.at("i") == "4"); smt::const_find_type rc=sm2.find("k"); - assert(!rc.second); + REQUIRE(!rc.second); } - // remove elements + SECTION("Remove elements") { smt sm; sm.insert("i", "0"); sm.insert("j", "1"); - assert(sm.erase("k")==0); - assert(sm.size()==2); + REQUIRE(sm.erase("k") == 0); + REQUIRE(sm.size() == 2); - assert(sm.erase("i")==1); - assert(!sm.has_key("i")); + REQUIRE(sm.erase("i") == 1); + REQUIRE(!sm.has_key("i")); - assert(sm.erase("j")==1); - assert(!sm.has_key("j")); + REQUIRE(sm.erase("j") == 1); + REQUIRE(!sm.has_key("j")); sm.insert("i", "0"); sm.insert("j", "1"); smt sm3(sm); - - assert(sm.has_key("i")); - assert(sm.has_key("j")); - assert(sm3.has_key("i")); - assert(sm3.has_key("j")); + + REQUIRE(sm.has_key("i")); + REQUIRE(sm.has_key("j")); + REQUIRE(sm3.has_key("i")); + REQUIRE(sm3.has_key("j")); sm.erase("i"); - assert(!sm.has_key("i")); - assert(sm.has_key("j")); + REQUIRE(!sm.has_key("i")); + REQUIRE(sm.has_key("j")); - assert(sm3.has_key("i")); - assert(sm3.has_key("j")); + REQUIRE(sm3.has_key("i")); + REQUIRE(sm3.has_key("j")); sm3.erase("i"); - assert(!sm3.has_key("i")); + REQUIRE(!sm3.has_key("i")); } - // operator[] + SECTION("Subscript operator") { smt sm; sm["i"]; - assert(sm.has_key("i")); + REQUIRE(sm.has_key("i")); sm["i"]="0"; - assert(sm.at("i")=="0"); + REQUIRE(sm.at("i") == "0"); sm["j"]="1"; - assert(sm.at("j")=="1"); + REQUIRE(sm.at("j") == "1"); } } void sharing_map_copy_test() { - std::cout << "Running copy test" << std::endl; - smt sm1; const smt &sm2=sm1; fill(sm1); - assert(sm2.find("i").first=="0"); - assert(sm2.find("j").first=="1"); - assert(sm2.find("k").first=="2"); + REQUIRE(sm2.find("i").first == "0"); + REQUIRE(sm2.find("j").first == "1"); + REQUIRE(sm2.find("k").first == "2"); smt sm3=sm1; const smt &sm4=sm3; - assert(sm3.erase("l")==0); - assert(sm3.erase("i")==1); + REQUIRE(sm3.erase("l") == 0); + REQUIRE(sm3.erase("i") == 1); - assert(!sm4.has_key("i")); + REQUIRE(!sm4.has_key("i")); sm3.place("i", "3"); - assert(sm4.find("i").first=="3"); + REQUIRE(sm4.find("i").first == "3"); } class some_keyt @@ -208,8 +210,6 @@ class some_key_hash void sharing_map_collision_test() { - std::cout << "Running collision test" << std::endl; - typedef sharing_mapt smt; smt sm; @@ -223,20 +223,18 @@ void sharing_map_collision_test() sm.erase(8); - assert(sm.has_key(0)); - assert(sm.has_key(16)); + REQUIRE(sm.has_key(0)); + REQUIRE(sm.has_key(16)); - assert(sm.has_key(1)); - assert(sm.has_key(2)); + REQUIRE(sm.has_key(1)); + REQUIRE(sm.has_key(2)); - assert(!sm.has_key(8)); + REQUIRE(!sm.has_key(8)); } void sharing_map_view_test() { - std::cout << "Running view test" << std::endl; - - // view test + SECTION("View") { smt sm; @@ -245,10 +243,10 @@ void sharing_map_view_test() smt::viewt view; sm.get_view(view); - assert(view.size()==3); + REQUIRE(view.size() == 3); } - // delta view test (no sharing, same keys) + SECTION("Delta view (no sharing, same keys)") { smt sm1; fill(sm1); @@ -259,14 +257,14 @@ void sharing_map_view_test() smt::delta_viewt delta_view; sm1.get_delta_view(sm2, delta_view); - assert(delta_view.size()==3); + REQUIRE(delta_view.size() == 3); delta_view.clear(); sm1.get_delta_view(sm2, delta_view, false); - assert(delta_view.size()==3); + REQUIRE(delta_view.size() == 3); } - // delta view test (all shared, same keys) + SECTION("delta view (all shared, same keys)") { smt sm1; fill(sm1); @@ -276,36 +274,36 @@ void sharing_map_view_test() smt::delta_viewt delta_view; sm1.get_delta_view(sm2, delta_view); - assert(delta_view.size()==0); + REQUIRE(delta_view.size() == 0); delta_view.clear(); sm1.get_delta_view(sm2, delta_view, false); - assert(delta_view.size()==0); + REQUIRE(delta_view.size() == 0); } - // delta view test (some sharing, same keys) + SECTION("delta view (some sharing, same keys)") { smt sm1; fill(sm1); smt sm2(sm1); auto r=sm2.find("i"); - assert(r.second); + REQUIRE(r.second); r.first="3"; smt::delta_viewt delta_view; sm1.get_delta_view(sm2, delta_view); - assert(delta_view.size()>0); // not everything is shared - assert(delta_view.size()<3); // there is some sharing + REQUIRE(delta_view.size() > 0); // not everything is shared + REQUIRE(delta_view.size() < 3); // there is some sharing delta_view.clear(); sm1.get_delta_view(sm2, delta_view, false); - assert(delta_view.size()>0); // not everything is shared - assert(delta_view.size()<3); // there is some sharing + REQUIRE(delta_view.size() > 0); // not everything is shared + REQUIRE(delta_view.size() < 3); // there is some sharing } - // delta view test (no sharing, different keys) + SECTION("delta view (no sharing, different keys)") { smt sm1; fill(sm1); @@ -316,21 +314,30 @@ void sharing_map_view_test() smt::delta_viewt delta_view; sm1.get_delta_view(sm2, delta_view); - assert(delta_view.size()==0); + REQUIRE(delta_view.size() == 0); delta_view.clear(); sm1.get_delta_view(sm2, delta_view, false); - assert(delta_view.size()==3); + REQUIRE(delta_view.size() == 3); } } -int main() +TEST_CASE("Sharing map interface") { sharing_map_interface_test(); +} + +TEST_CASE("Sharing map copying") +{ sharing_map_copy_test(); - sharing_map_collision_test(); - sharing_map_view_test(); +} - return 0; +TEST_CASE("Sharing map collisions") +{ + sharing_map_collision_test(); } +TEST_CASE("Sharing map views") +{ + sharing_map_view_test(); +} diff --git a/unit/sharing_node.cpp b/unit/sharing_node.cpp index 04f440673a6..2c26e94bb25 100644 --- a/unit/sharing_node.cpp +++ b/unit/sharing_node.cpp @@ -2,9 +2,6 @@ /// \file Tests for sharing-node utility -#include -#include - #include #include @@ -12,7 +9,7 @@ void sharing_node_test() { typedef sharing_nodet snt; - // internal node test + SECTION("Internal nodes") { snt sn1; const snt &sn2=sn1; @@ -51,7 +48,7 @@ void sharing_node_test() REQUIRE(sn2.is_empty()); } - // container node test + SECTION("Container nodes") { snt sn1; @@ -78,7 +75,7 @@ void sharing_node_test() REQUIRE(p2->get_container().size()==1); } - // copy test 1 + SECTION("Copy test 1") { snt sn1; snt sn2; @@ -96,7 +93,7 @@ void sharing_node_test() REQUIRE(sn2.data.use_count()==2); } - // copy test 2 + SECTION("Copy test 2") { snt sn1; const snt &sn1c=sn1; From e54f740f8755c215027e8c92cbeaeb4a39bddcd1 Mon Sep 17 00:00:00 2001 From: Daniel Poetzl Date: Fri, 2 Mar 2018 10:53:17 +0000 Subject: [PATCH 2/6] Fix sharing map compiler warnings --- src/util/sharing_map.h | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/util/sharing_map.h b/src/util/sharing_map.h index a558ec42901..718e2b0f78f 100644 --- a/src/util/sharing_map.h +++ b/src/util/sharing_map.h @@ -450,7 +450,7 @@ SHARING_MAPT2(, size_type)::erase( return 0; node_type *del=nullptr; - unsigned del_bit; + unsigned del_bit = 0; size_t key=hash()(k); node_type *p=↦ @@ -472,18 +472,15 @@ SHARING_MAPT2(, size_type)::erase( _sm_assert(p->is_container()); - { - const containert &c=as_const(p)->get_container(); + const containert &c = as_const(p)->get_container(); - if(c.size()==1) - { - del->remove_child(del_bit); - num--; - return 1; - } + if(c.size() == 1) + { + del->remove_child(del_bit); + num--; + return 1; } - containert &c=p->get_container(); _sm_assert(c.size()>1); p->remove_leaf(k); num--; From 3e22217209c30d8f915236abce7e376b2a8cab23 Mon Sep 17 00:00:00 2001 From: Daniel Poetzl Date: Mon, 5 Mar 2018 16:30:34 +0000 Subject: [PATCH 3/6] Sharing map documentation --- src/util/sharing_map.h | 235 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 235 insertions(+) diff --git a/src/util/sharing_map.h b/src/util/sharing_map.h index 718e2b0f78f..d9579c449ad 100644 --- a/src/util/sharing_map.h +++ b/src/util/sharing_map.h @@ -39,6 +39,72 @@ Author: Daniel Poetzl CV typename sharing_mapt::ST \ sharing_mapt +/// A map implemented as a tree where subtrees can be shared between different +/// maps. +/// +/// The map is implemented as a fixed-height n-ary trie. The height H and the +/// maximum number of children per inner node S are determined by the two +/// configuration parameters `bits` and `chunks` in sharing_map.h. It holds +/// that H = `bits` / `chunks` and S = 2 ^ `chunks`. +/// +/// When inserting a key-value pair into the map, first the hash of its key is +/// computed. The `bits` number of lower order bits of the hash are deemed +/// significant, and are grouped into `bits` / `chunk` chunks. The hash is then +/// treated as a string (with each chunk representing a character) for the +/// purposes of determining the position of the key-value pair in the trie. The +/// actual key-value pairs are stored in the leaf nodes. Collisions (i.e., two +/// different keys yield the same "string"), are handled by chaining the +/// corresponding key-value pairs in a `std::list`. +/// +/// The use of a trie in combination with hashing has the advantage that the +/// tree is unlikely to degenerate (if the number of hash collisions is low). +/// This makes re-balancing operations unnecessary which do not interact well +/// with sharing. A disadvantage is that the height of the tree is likely +/// greater than if the elements had been stored in a balanced tree (with +/// greater differences for sparser maps). +/// +/// The nodes in the sharing map are objects of type sharing_nodet. Each sharing +/// node has a `shared_ptr` to an object of type `dt` which can be shared +/// between nodes. +/// +/// Sharing is initially generated when a map is assigned to another map or +/// copied via the copy constructor. Then both maps contain a pointer to the +/// root node of the tree that represents the maps. On subsequent modifications +/// to one of the maps, nodes are copied and sharing is lessened as described in +/// the following. +/// +/// Retrieval, insertion, and removal operations interact with sharing as +/// follows: +/// - When a non-const reference to a value in the map that is contained in a +/// shared subtree is retrieved, the nodes on the path from the root of the +/// subtree to the corresponding key-value pair (and the key-value pair itself) +/// are copied and integrated with the map. +/// - When a key-value pair is inserted into the map and its position is in a +/// shared subtree, already existing nodes from the root of the subtree to the +/// position of the key-value pair are copied and integrated with the map, and +/// new nodes are created as needed. +/// - When a key-value pair is erased from the map that is in a shared subtree, +/// nodes from the root of the subtree to the last node that will still exist on +/// the path to the erased element after the element has been removed are +/// copied and integrated with the map, and the remaining nodes are removed. +/// +/// Several methods take a hint indicating whether the element is known not to +/// be in the map (`false`), known to be in the map (`true`), or it is unknown +/// whether the element is in the map (`unknown`). The value `unknown` is always +/// valid. When `true` or `false` are given they need to be accurate, otherwise +/// the behavior is undefined. A correct hint can prevent the need to follow a +/// path from the root to a key-value pair twice (e.g., once for checking that +/// it exists, and second for copying nodes). +/// +/// In the descriptions of the methods of the sharing map we also give the +/// complexity of the operations. We use the following symbols: +/// - N: number of key-value pairs in the map +/// - M: maximum number of key-value pairs that are chained in a leaf node +/// - H: height of the tree +/// - S: maximum number of children per internal node +/// +/// The first two symbols denote dynamic properties of a given map, whereas the +/// last two symbols are static configuration parameters of the map class. template < class keyT, class valueT, @@ -68,7 +134,16 @@ class sharing_mapt typedef size_t size_type; + /// Return type of methods that retrieve a const reference to a value. First + /// component is a reference to the value (or a dummy value if the given key + /// does not exist), and the second component indicates if the value with the + /// given key was found. typedef const std::pair const_find_type; + + /// Return type of methods that retrieve a reference to a value. First + /// component is a reference to the value (or a dummy value if the given key + /// does not exist), and the second component indicates if the value with the + /// given key was found. typedef const std::pair find_type; typedef std::vector keyst; @@ -89,7 +164,10 @@ class sharing_mapt static const std::string not_found_msg; + /// Number of bits in the hash deemed significant static const size_t bits; + + /// Size of a chunk of the hash that represents a character static const size_t chunk; static const size_t mask; @@ -136,6 +214,9 @@ class sharing_mapt mapped_type &operator[](const key_type &k); + /// Swap with other map + /// + /// Complexity: O(1) void swap(self_type &other) { map.swap(other.map); @@ -145,22 +226,32 @@ class sharing_mapt other.num=tmp; } + /// Get number of elements in map + /// + /// Complexity: O(1) size_type size() const { return num; } + /// Check if map is empty bool empty() const { return num==0; } + /// Clear map void clear() { map.clear(); num=0; } + /// Check if key is in map + /// + /// Complexity: + /// - Worst case: O(H * log(S) + M) + /// - Best case: O(H) bool has_key(const key_type &k) const { return get_leaf_node(k)!=nullptr; @@ -169,6 +260,9 @@ class sharing_mapt // views typedef std::pair view_itemt; + + /// View of the key-value pairs in the map. A view is a list of pairs with + /// the components being const references to the keys and values in the map. typedef std::vector viewt; class delta_view_itemt @@ -194,6 +288,9 @@ class sharing_mapt const mapped_type &other_m; }; + /// Delta view of the key-value pairs in two maps. A delta view of two maps is + /// a view of the key-value pairs in the maps that are contained in subtrees + /// that are not shared between them (also see get_delta_view()). typedef std::vector delta_viewt; void get_view(viewt &view) const; @@ -214,6 +311,15 @@ class sharing_mapt void gather_all(const node_type &n, delta_viewt &delta_view) const; }; +/// Get a view of the elements in the map +/// A view is a list of pairs with the components being const references to the +/// keys and values in the map. +/// +/// Complexity: +/// - Worst case: O(N * H * log(S)) +/// - Best case: O(N + H) +/// +/// \param[out] view: Empty view SHARING_MAPT(void)::get_view(viewt &view) const { assert(view.empty()); @@ -286,6 +392,39 @@ SHARING_MAPT(void)::gather_all(const node_type &n, delta_viewt &delta_view) while(!stack.empty()); } +/// Get a delta view of the elements in the map +/// +/// Informally, a delta view of two maps is a view of the key-value pairs in the +/// maps that are contained in subtrees that are not shared between them. +/// +/// A delta view is represented as a list of structs, with each struct having +/// four members (`in_both`, `key`, `value1`, `value2`). The elements `key`, +/// `value1`, and `value2` are const references to the corresponding elements in +/// the map. The first element indicates whether the key exists in both maps, +/// the second element is the key, the third element is the mapped value of the +/// first map, and the fourth element is the mapped value of the second map, or +/// a dummy element if the key exists only in the first map (in which case +/// `in_both` is false). +/// +/// Calling `A.delta_view(B, ...)` yields a view such that for each element in +/// the view one of two things holds: +/// - the key is contained in both A and B, and in the maps the corresponding +/// key-value pairs are not contained in a subtree that is shared between them +/// - the key is only contained in A +/// +/// When `only_common=true`, the first case above holds for every element in the +/// view. +/// +/// Complexity: +/// - Worst case: O(max(N1, N2) * H * log(S) * M1 * M2) (no sharing) +/// - Best case: O(1) (maximum sharing) +/// +/// The symbols N1, M1 refer to map A, and symbols N2, M2 refer to map B. +/// +/// \param other: other map +/// \param[out] delta_view: Empty delta view +/// \param only_common: Indicates if the returned delta view should only +/// contain key-value pairs for keys that exist in both maps SHARING_MAPT(void)::get_delta_view( const self_type &other, delta_viewt &delta_view, @@ -439,6 +578,15 @@ SHARING_MAPT2(const, node_type *)::get_leaf_node(const key_type &k) const return p; } +/// Erase element +/// +/// Complexity: +/// - Worst case: O(H * S + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element to erase +/// \param key_exists: Hint to indicate whether the element is known to exist +/// (possible values `unknown` or` true`) SHARING_MAPT2(, size_type)::erase( const key_type &k, const tvt &key_exists) @@ -488,6 +636,17 @@ SHARING_MAPT2(, size_type)::erase( return 1; } +/// Erase all elements +/// +/// Complexity: +/// - Worst case: O(K * (H * S + M)) +/// - Best case: O(K * H) +/// +/// \param ks: The keys of the element to erase +/// \param key_exists: Hint to indicate whether the elements are known to exist +/// (possible values `unknown` or `true`). Applies to all elements (i.e., have +/// to use `unknown` if for at least one element it is not known whether it +/// exists) SHARING_MAPT2(, size_type)::erase_all( const keyst &ks, const tvt &key_exists) @@ -502,6 +661,18 @@ SHARING_MAPT2(, size_type)::erase_all( return cnt; } +/// Insert element, return const reference +/// +/// Complexity: +/// - Worst case: O(H * S + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element to insert +/// \param m: The mapped value to insert +/// \param key_exists: Hint to indicate whether the element is known to exist +/// (possible values `false` or `unknown`) +/// \return Pair of const reference to existing or newly inserted element, and +/// boolean indicating if new element was inserted SHARING_MAPT2(, const_find_type)::insert( const key_type &k, const mapped_type &m, @@ -525,6 +696,7 @@ SHARING_MAPT2(, const_find_type)::insert( return const_find_type(as_const(p)->get_value(), true); } +// Insert element, return const reference SHARING_MAPT2(, const_find_type)::insert( const value_type &p, const tvt &key_exists) @@ -532,6 +704,18 @@ SHARING_MAPT2(, const_find_type)::insert( return insert(p.first, p.second, key_exists); } +/// Insert element, return non-const reference +/// +/// Complexity: +/// - Worst case: O(H * S + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element to insert +/// \param m: The mapped value to insert +/// \param key_exists: Hint to indicate whether the element is known to exist +/// (possible values false or unknown) +/// \return Pair of reference to existing or newly inserted element, and boolean +/// indicating if new element was inserted SHARING_MAPT2(, find_type)::place( const key_type &k, const mapped_type &m) @@ -550,12 +734,24 @@ SHARING_MAPT2(, find_type)::place( return find_type(p->get_value(), true); } +/// Insert element, return non-const reference SHARING_MAPT2(, find_type)::place( const value_type &p) { return place(p.first, p.second); } +/// Find element +/// +/// Complexity: +/// - Worst case: O(H * S + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element to search for +/// \param key_exists: Hint to indicate whether the element is known to exist +/// (possible values `unknown` or `true`) +/// \return Pair of reference to found value (or dummy value if not found), and +/// boolean indicating if element was found. SHARING_MAPT2(, find_type)::find( const key_type &k, const tvt &key_exists) @@ -575,6 +771,17 @@ SHARING_MAPT2(, find_type)::find( } +/// Find element +/// +/// Complexity: +/// - Worst case: O(H * log(S) + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element to search +/// \param key_exists: Hint to indicate whether the element is known to exist +/// (possible values `unknown` or `true`) +/// \return Pair of const reference to found value (or dummy value if not +/// found), and boolean indicating if element was found. SHARING_MAPT2(, const_find_type)::find(const key_type &k) const { const node_type *p=get_leaf_node(k); @@ -585,6 +792,17 @@ SHARING_MAPT2(, const_find_type)::find(const key_type &k) const return const_find_type(p->get_value(), true); } +/// Get element at key +/// +/// Complexity: +/// - Worst case: O(H * S + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element +/// \param key_exists: Hint to indicate whether the element is known to exist +/// (possible values `unknown` or `true`) +/// \throw `std::out_of_range` if key not found +/// \return The mapped value SHARING_MAPT2(, mapped_type &)::at( const key_type &k, const tvt &key_exists) @@ -597,6 +815,15 @@ SHARING_MAPT2(, mapped_type &)::at( return r.first; } +/// Get element at key +/// +/// Complexity: +/// - Worst case: O(H * log(S) + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element +/// \throw std::out_of_range if key not found +/// \return The mapped value SHARING_MAPT2(const, mapped_type &)::at(const key_type &k) const { const_find_type r=find(k); @@ -606,6 +833,14 @@ SHARING_MAPT2(const, mapped_type &)::at(const key_type &k) const return r.first; } +/// Get element at key, insert new if non-existent +/// +/// Complexity: +/// - Worst case: O(H * S + M) +/// - Best case: O(H) +/// +/// \param k: The key of the element +/// \return The mapped value SHARING_MAPT2(, mapped_type &)::operator[](const key_type &k) { return place(k, mapped_type()).first; From 47463a382dbc59b0734e5d247bfeb6326770cb5c Mon Sep 17 00:00:00 2001 From: Daniel Poetzl Date: Mon, 23 Apr 2018 17:39:43 +0100 Subject: [PATCH 4/6] Use std::size_t instead of unsigned in the sharing map --- src/util/sharing_map.h | 28 ++++++++++++---------------- src/util/sharing_node.h | 8 ++++---- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/util/sharing_map.h b/src/util/sharing_map.h index d9579c449ad..23da0dedda6 100644 --- a/src/util/sharing_map.h +++ b/src/util/sharing_map.h @@ -132,7 +132,7 @@ class sharing_mapt typedef sharing_mapt self_type; typedef sharing_nodet node_type; - typedef size_t size_type; + typedef std::size_t size_type; /// Return type of methods that retrieve a const reference to a value. First /// component is a reference to the value (or a dummy value if the given key @@ -165,13 +165,13 @@ class sharing_mapt static const std::string not_found_msg; /// Number of bits in the hash deemed significant - static const size_t bits; + static const std::size_t bits; /// Size of a chunk of the hash that represents a character - static const size_t chunk; + static const std::size_t chunk; - static const size_t mask; - static const size_t steps; + static const std::size_t mask; + static const std::size_t steps; // interface @@ -536,9 +536,9 @@ SHARING_MAPT2(, node_type *)::get_container_node(const key_type &k) size_t key=hash()(k); node_type *p=↦ - for(unsigned i=0; i>=chunk; p=p->add_child(bit); @@ -552,9 +552,9 @@ SHARING_MAPT2(const, node_type *)::get_container_node(const key_type &k) const size_t key=hash()(k); const node_type *p=↦ - for(unsigned i=0; i>=chunk; p=p->find_child(bit); @@ -598,14 +598,14 @@ SHARING_MAPT2(, size_type)::erase( return 0; node_type *del=nullptr; - unsigned del_bit = 0; + std::size_t del_bit = 0; size_t key=hash()(k); node_type *p=↦ - for(unsigned i=0; i>=chunk; const subt &sub=as_const(p)->get_sub(); @@ -712,8 +712,6 @@ SHARING_MAPT2(, const_find_type)::insert( /// /// \param k: The key of the element to insert /// \param m: The mapped value to insert -/// \param key_exists: Hint to indicate whether the element is known to exist -/// (possible values false or unknown) /// \return Pair of reference to existing or newly inserted element, and boolean /// indicating if new element was inserted SHARING_MAPT2(, find_type)::place( @@ -778,8 +776,6 @@ SHARING_MAPT2(, find_type)::find( /// - Best case: O(H) /// /// \param k: The key of the element to search -/// \param key_exists: Hint to indicate whether the element is known to exist -/// (possible values `unknown` or `true`) /// \return Pair of const reference to found value (or dummy value if not /// found), and boolean indicating if element was found. SHARING_MAPT2(, const_find_type)::find(const key_type &k) const diff --git a/src/util/sharing_node.h b/src/util/sharing_node.h index 9cec1cd2f3a..2c279c486ed 100644 --- a/src/util/sharing_node.h +++ b/src/util/sharing_node.h @@ -45,7 +45,7 @@ class sharing_nodet typedef sharing_nodet self_type; - typedef std::map subt; + typedef std::map subt; typedef std::list containert; typedef const std::pair const_find_type; @@ -156,7 +156,7 @@ class sharing_nodet // internal nodes - const self_type *find_child(const unsigned n) const + const self_type *find_child(const std::size_t n) const { const subt &s=get_sub(); typename subt::const_iterator it=s.find(n); @@ -167,13 +167,13 @@ class sharing_nodet return nullptr; } - self_type *add_child(const unsigned n) + self_type *add_child(const std::size_t n) { subt &s=get_sub(); return &s[n]; } - void remove_child(const unsigned n) + void remove_child(const std::size_t n) { subt &s=get_sub(); size_t r=s.erase(n); From 1f661f5d90283114752c7e0931d9f26bb687078a Mon Sep 17 00:00:00 2001 From: Daniel Poetzl Date: Mon, 23 Apr 2018 18:03:50 +0100 Subject: [PATCH 5/6] Move sharing map and sharing node unit tests to util --- unit/Makefile | 4 +- unit/{ => util}/sharing_map.cpp | 60 +++++++++++++++-------------- unit/{ => util}/sharing_node.cpp | 66 ++++++++++++++++---------------- 3 files changed, 67 insertions(+), 63 deletions(-) rename unit/{ => util}/sharing_map.cpp (88%) rename unit/{ => util}/sharing_node.cpp (55%) diff --git a/unit/Makefile b/unit/Makefile index 8fccc515dd1..4b3c4800526 100644 --- a/unit/Makefile +++ b/unit/Makefile @@ -27,8 +27,6 @@ SRC += unit_tests.cpp \ java_bytecode/java_utils_test.cpp \ java_bytecode/inherited_static_fields/inherited_static_fields.cpp \ pointer-analysis/custom_value_set_analysis.cpp \ - sharing_node.cpp \ - sharing_map.cpp \ solvers/refinement/string_constraint_generator_valueof/calculate_max_string_length.cpp \ solvers/refinement/string_constraint_generator_valueof/get_numeric_value_from_character.cpp \ solvers/refinement/string_constraint_generator_valueof/is_digit_with_radix.cpp \ @@ -47,6 +45,8 @@ SRC += unit_tests.cpp \ util/message.cpp \ util/optional.cpp \ util/parameter_indices.cpp \ + util/sharing_node.cpp \ + util/sharing_map.cpp \ util/simplify_expr.cpp \ util/small_shared_two_way_ptr.cpp \ util/string_utils/split_string.cpp \ diff --git a/unit/sharing_map.cpp b/unit/util/sharing_map.cpp similarity index 88% rename from unit/sharing_map.cpp rename to unit/util/sharing_map.cpp index 746d9376946..e08598fced4 100644 --- a/unit/sharing_map.cpp +++ b/unit/util/sharing_map.cpp @@ -43,13 +43,13 @@ void sharing_map_interface_test() { smt sm; - smt::const_find_type r1=sm.insert(std::make_pair("i", "0")); + smt::const_find_type r1 = sm.insert(std::make_pair("i", "0")); REQUIRE(r1.second); - smt::const_find_type r2=sm.insert("j", "1"); + smt::const_find_type r2 = sm.insert("j", "1"); REQUIRE(r2.second); - smt::const_find_type r3=sm.insert(std::make_pair("i", "0")); + smt::const_find_type r3 = sm.insert(std::make_pair("i", "0")); REQUIRE(!r3.second); REQUIRE(sm.size() == 2); @@ -61,12 +61,12 @@ void sharing_map_interface_test() smt sm1; smt sm2(sm1); - smt::find_type r1=sm1.place("i", "0"); + smt::find_type r1 = sm1.place("i", "0"); REQUIRE(r1.second); REQUIRE(!sm2.has_key("i")); - std::string &s1=r1.first; - s1="1"; + std::string &s1 = r1.first; + s1 = "1"; REQUIRE(sm1.at("i") == "1"); } @@ -77,36 +77,40 @@ void sharing_map_interface_test() sm.insert("i", "0"); sm.insert("j", "1"); - const smt &sm2=sm; + const smt &sm2 = sm; std::string s; - s=sm2.at("i"); + s = sm2.at("i"); REQUIRE(s == "0"); - try { + try + { sm2.at("k"); REQUIRE(false); - } catch (...) {} + } + catch(...) + { + } - s=sm2.at("j"); + s = sm2.at("j"); REQUIRE(s == "1"); REQUIRE(sm.has_key("i")); REQUIRE(sm.has_key("j")); REQUIRE(!sm.has_key("k")); - std::string &s2=sm.at("i"); - s2="3"; + std::string &s2 = sm.at("i"); + s2 = "3"; REQUIRE(sm2.at("i") == "3"); REQUIRE(sm.size() == 2); - smt::find_type r=sm.find("i"); + smt::find_type r = sm.find("i"); REQUIRE(r.second); - r.first="4"; + r.first = "4"; REQUIRE(sm2.at("i") == "4"); - smt::const_find_type rc=sm2.find("k"); + smt::const_find_type rc = sm2.find("k"); REQUIRE(!rc.second); } @@ -154,10 +158,10 @@ void sharing_map_interface_test() sm["i"]; REQUIRE(sm.has_key("i")); - sm["i"]="0"; + sm["i"] = "0"; REQUIRE(sm.at("i") == "0"); - sm["j"]="1"; + sm["j"] = "1"; REQUIRE(sm.at("j") == "1"); } } @@ -165,7 +169,7 @@ void sharing_map_interface_test() void sharing_map_copy_test() { smt sm1; - const smt &sm2=sm1; + const smt &sm2 = sm1; fill(sm1); @@ -173,8 +177,8 @@ void sharing_map_copy_test() REQUIRE(sm2.find("j").first == "1"); REQUIRE(sm2.find("k").first == "2"); - smt sm3=sm1; - const smt &sm4=sm3; + smt sm3 = sm1; + const smt &sm4 = sm3; REQUIRE(sm3.erase("l") == 0); REQUIRE(sm3.erase("i") == 1); @@ -195,7 +199,7 @@ class some_keyt bool operator==(const some_keyt &other) const { - return s==other.s; + return s == other.s; } }; @@ -237,9 +241,9 @@ void sharing_map_view_test() SECTION("View") { smt sm; - + fill(sm); - + smt::viewt view; sm.get_view(view); @@ -270,7 +274,7 @@ void sharing_map_view_test() fill(sm1); smt sm2(sm1); - + smt::delta_viewt delta_view; sm1.get_delta_view(sm2, delta_view); @@ -287,10 +291,10 @@ void sharing_map_view_test() fill(sm1); smt sm2(sm1); - auto r=sm2.find("i"); + auto r = sm2.find("i"); REQUIRE(r.second); - r.first="3"; - + r.first = "3"; + smt::delta_viewt delta_view; sm1.get_delta_view(sm2, delta_view); diff --git a/unit/sharing_node.cpp b/unit/util/sharing_node.cpp similarity index 55% rename from unit/sharing_node.cpp rename to unit/util/sharing_node.cpp index 2c26e94bb25..9d72919cfed 100644 --- a/unit/sharing_node.cpp +++ b/unit/util/sharing_node.cpp @@ -2,8 +2,8 @@ /// \file Tests for sharing-node utility -#include #include +#include void sharing_node_test() { @@ -12,7 +12,7 @@ void sharing_node_test() SECTION("Internal nodes") { snt sn1; - const snt &sn2=sn1; + const snt &sn2 = sn1; const snt *p2; @@ -27,22 +27,22 @@ void sharing_node_test() REQUIRE(!sn2.is_container()); REQUIRE(!sn2.is_leaf()); - REQUIRE(sn2.get_sub().size()==3); + REQUIRE(sn2.get_sub().size() == 3); - p2=sn2.find_child(0); - REQUIRE(p2!=nullptr); + p2 = sn2.find_child(0); + REQUIRE(p2 != nullptr); - p2=sn1.find_child(0); - REQUIRE(p2!=nullptr); + p2 = sn1.find_child(0); + REQUIRE(p2 != nullptr); - p2=sn2.find_child(3); - REQUIRE(p2==nullptr); + p2 = sn2.find_child(3); + REQUIRE(p2 == nullptr); - p2=sn1.find_child(3); - REQUIRE(p2==nullptr); + p2 = sn1.find_child(3); + REQUIRE(p2 == nullptr); sn1.remove_child(0); - REQUIRE(sn2.get_sub().size()==2); + REQUIRE(sn2.get_sub().size() == 2); sn1.clear(); REQUIRE(sn2.is_empty()); @@ -58,21 +58,21 @@ void sharing_node_test() sn1.add_child(0); sn1.add_child(1); - p1=sn1.add_child(2); - p2=p1; + p1 = sn1.add_child(2); + p2 = p1; - REQUIRE(p1->find_leaf("a")==nullptr); - REQUIRE(p2->find_leaf("a")==nullptr); + REQUIRE(p1->find_leaf("a") == nullptr); + REQUIRE(p2->find_leaf("a") == nullptr); p1->place_leaf("a", "b"); - REQUIRE(p2->get_container().size()==1); + REQUIRE(p2->get_container().size() == 1); p1->place_leaf("c", "d"); - REQUIRE(p2->get_container().size()==2); + REQUIRE(p2->get_container().size() == 2); REQUIRE(p2->is_container()); p1->remove_leaf("a"); - REQUIRE(p2->get_container().size()==1); + REQUIRE(p2->get_container().size() == 1); } SECTION("Copy test 1") @@ -80,43 +80,43 @@ void sharing_node_test() snt sn1; snt sn2; - sn2=sn1; - REQUIRE(sn1.data.use_count()==3); - REQUIRE(sn2.data.use_count()==3); + sn2 = sn1; + REQUIRE(sn1.data.use_count() == 3); + REQUIRE(sn2.data.use_count() == 3); sn1.add_child(0); - REQUIRE(sn1.data.use_count()==1); + REQUIRE(sn1.data.use_count() == 1); // the newly created node is empty as well - REQUIRE(sn2.data.use_count()==3); + REQUIRE(sn2.data.use_count() == 3); - sn2=sn1; - REQUIRE(sn2.data.use_count()==2); + sn2 = sn1; + REQUIRE(sn2.data.use_count() == 2); } SECTION("Copy test 2") { snt sn1; - const snt &sn1c=sn1; + const snt &sn1c = sn1; snt *p; - p=sn1.add_child(0); + p = sn1.add_child(0); p->place_leaf("x", "y"); - p=sn1.add_child(1); + p = sn1.add_child(1); p->place_leaf("a", "b"); p->place_leaf("c", "d"); snt sn2; - const snt &sn2c=sn2; - sn2=sn1; + const snt &sn2c = sn2; + sn2 = sn1; REQUIRE(sn1.is_internal()); REQUIRE(sn2.is_internal()); sn1.remove_child(0); - REQUIRE(sn1c.get_sub().size()==1); + REQUIRE(sn1c.get_sub().size() == 1); - REQUIRE(sn2c.get_sub().size()==2); + REQUIRE(sn2c.get_sub().size() == 2); } } From e7b3adec38c42690707b41eb2aa947b607bab11a Mon Sep 17 00:00:00 2001 From: Daniel Poetzl Date: Wed, 25 Apr 2018 11:19:27 +0100 Subject: [PATCH 6/6] Workaround for Visual Studio loss of CV qualifier bug --- src/util/sharing_map.h | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/util/sharing_map.h b/src/util/sharing_map.h index 23da0dedda6..6e19206bb12 100644 --- a/src/util/sharing_map.h +++ b/src/util/sharing_map.h @@ -39,6 +39,11 @@ Author: Daniel Poetzl CV typename sharing_mapt::ST \ sharing_mapt +// Note: Due to a bug in Visual Studio we need to add an additional "const" +// qualifier to the return values of insert(), place(), and find(). The type +// defined in sharing_mapt already includes the const qualifier, but it is lost +// when accessed from outside the class. This is fixed in Visual Studio 15.6. + /// A map implemented as a tree where subtrees can be shared between different /// maps. /// @@ -673,10 +678,8 @@ SHARING_MAPT2(, size_type)::erase_all( /// (possible values `false` or `unknown`) /// \return Pair of const reference to existing or newly inserted element, and /// boolean indicating if new element was inserted -SHARING_MAPT2(, const_find_type)::insert( - const key_type &k, - const mapped_type &m, - const tvt &key_exists) +SHARING_MAPT2(const, const_find_type) +::insert(const key_type &k, const mapped_type &m, const tvt &key_exists) { _sn_assert(!key_exists.is_true()); @@ -697,9 +700,8 @@ SHARING_MAPT2(, const_find_type)::insert( } // Insert element, return const reference -SHARING_MAPT2(, const_find_type)::insert( - const value_type &p, - const tvt &key_exists) +SHARING_MAPT2(const, const_find_type) +::insert(const value_type &p, const tvt &key_exists) { return insert(p.first, p.second, key_exists); } @@ -714,9 +716,7 @@ SHARING_MAPT2(, const_find_type)::insert( /// \param m: The mapped value to insert /// \return Pair of reference to existing or newly inserted element, and boolean /// indicating if new element was inserted -SHARING_MAPT2(, find_type)::place( - const key_type &k, - const mapped_type &m) +SHARING_MAPT2(const, find_type)::place(const key_type &k, const mapped_type &m) { node_type *c=get_container_node(k); _sm_assert(c!=nullptr); @@ -733,8 +733,7 @@ SHARING_MAPT2(, find_type)::place( } /// Insert element, return non-const reference -SHARING_MAPT2(, find_type)::place( - const value_type &p) +SHARING_MAPT2(const, find_type)::place(const value_type &p) { return place(p.first, p.second); } @@ -750,9 +749,7 @@ SHARING_MAPT2(, find_type)::place( /// (possible values `unknown` or `true`) /// \return Pair of reference to found value (or dummy value if not found), and /// boolean indicating if element was found. -SHARING_MAPT2(, find_type)::find( - const key_type &k, - const tvt &key_exists) +SHARING_MAPT2(const, find_type)::find(const key_type &k, const tvt &key_exists) { _sm_assert(!key_exists.is_false()); @@ -778,7 +775,7 @@ SHARING_MAPT2(, find_type)::find( /// \param k: The key of the element to search /// \return Pair of const reference to found value (or dummy value if not /// found), and boolean indicating if element was found. -SHARING_MAPT2(, const_find_type)::find(const key_type &k) const +SHARING_MAPT2(const, const_find_type)::find(const key_type &k) const { const node_type *p=get_leaf_node(k);