Fixed erase bug.

configure.ac

@@ -34,7 +34,7 @@ LDFLAGS="$LIBM $LDFLAGS"
 CFLAGS="-Wall"
 
 AC_PROG_CXX
-CXXFLAGS="$CXXFLAGS -Wall -Wno-unused-function -DNDEBUG -O3 -fomit-frame-pointer"
+CXXFLAGS="-Wall -Wno-unused-function -DNDEBUG -O3 -fomit-frame-pointer $CXXFLAGS"
 AC_ENABLE_CXXMPH
 if test x$cxxmph = xtrue; then
   AC_COMPILE_STDCXX_0X

cxxmph/Makefile.am

@@ -1,7 +1,7 @@
 TESTS = $(check_PROGRAMS)
-check_PROGRAMS = seeded_hash_test mph_bits_test hollow_iterator_test mph_index_test trigraph_test string_util_test
+check_PROGRAMS = seeded_hash_test mph_bits_test hollow_iterator_test mph_index_test trigraph_test
 if USE_LIBCHECK
-  check_PROGRAMS += test_test map_tester_test mph_map_test
+  check_PROGRAMS += test_test map_tester_test mph_map_test dense_hash_map_test string_util_test
   check_LTLIBRARIES = libcxxmph_test.la
 endif
 
@@ -25,6 +25,8 @@ test_test_LDADD = libcxxmph_test.la $(CHECK_LIBS)
 
 mph_map_test_LDADD = libcxxmph_test.la $(CHECK_LIBS)
 mph_map_test_SOURCES = mph_map_test.cc
+dense_hash_map_test_LDADD = libcxxmph_test.la $(CHECK_LIBS)
+dense_hash_map_test_SOURCES = dense_hash_map_test.cc
 
 mph_index_test_LDADD   = libcxxmph.la
 mph_index_test_SOURCES = mph_index_test.cc
@@ -51,7 +53,7 @@ mph_bits_test_SOURCES = mph_bits_test.cc
 mph_bits_test_LDADD   = libcxxmph.la
 
 string_util_test_SOURCES = string_util_test.cc
-string_util_test_LDADD   = libcxxmph.la
+string_util_test_LDADD   = libcxxmph.la libcxxmph_test.la $(CHECK_LIBS)
 
 map_tester_test_SOURCES = map_tester.cc map_tester_test.cc
 map_tester_test_LDADD = libcxxmph.la libcxxmph_test.la $(CHECK_LIBS)

cxxmph/dense_hash_map_test.cc

@@ -0,0 +1,23 @@
+#include <cstdio>
+#include <cstdlib>
+#include <iostream>
+#include <string>
+
+#include "mph_map.h"
+#include "map_tester.h"
+#include "test.h"
+
+using namespace cxxmph;
+
+typedef MapTester<dense_hash_map> Tester;
+
+CXXMPH_CXX_TEST_CASE(small_insert, Tester::small_insert);
+CXXMPH_CXX_TEST_CASE(large_insert, Tester::large_insert);
+CXXMPH_CXX_TEST_CASE(small_search, Tester::small_search);
+CXXMPH_CXX_TEST_CASE(default_search, Tester::default_search);
+CXXMPH_CXX_TEST_CASE(large_search, Tester::large_search);
+CXXMPH_CXX_TEST_CASE(string_search, Tester::string_search);
+CXXMPH_CXX_TEST_CASE(rehash_zero, Tester::rehash_zero);
+CXXMPH_CXX_TEST_CASE(rehash_size, Tester::rehash_size);
+CXXMPH_CXX_TEST_CASE(erase_value, Tester::erase_value);
+CXXMPH_CXX_TEST_CASE(erase_iterator, Tester::erase_iterator);

cxxmph/map_tester.h

@@ -8,6 +8,7 @@
 #include <unordered_map>
 
 #include "string_util.h"
+#include <check.h>
 
 namespace cxxmph {
 
@@ -27,7 +28,7 @@ struct MapTester {
     // Start counting from 1 to not touch default constructed value bugs
     int nkeys = 12 * 256 * 256;
     for (int i = 1; i < nkeys; ++i) m.insert(make_pair(i, i));
-    return static_cast<int>(m.size()) == nkeys - 0;
+    return static_cast<int>(m.size()) == nkeys - 1;
   }
   static bool small_search() {
     map_type<int64_t, int64_t> m;
@@ -87,7 +88,18 @@ struct MapTester {
     map_type<int64_t, int64_t> m;
     int nkeys = 10 * 1000;
     for (int i = 0; i < nkeys; ++i) { m.insert(make_pair(i, i)); }
-    for (int i = nkeys; i >= 0; --i) { 
+    for (int i = 0; i < nkeys; ++i) {
+       if (m.find(i) == m.end()) return false;
+    }
+    for (int i = nkeys - 1; i >= 0; --i) { if (m.find(i) == m.end()) return false; }
+    for (int i = nkeys - 1; i >= 0; --i) {
+      fail_unless(m.find(i) != m.end(), "after erase %d cannot be found", i);
+      fail_unless(m.find(i)->first == i, "after erase key %d cannot be found", i);
+    }
+    for (int i = nkeys - 1; i >= 0; --i) {
+      fail_unless(m.find(i) != m.end(), "after erase %d cannot be found", i);
+      fail_unless(m.find(i)->first == i, "after erase key %d cannot be found", i);
+      if (!(m.find(i)->first == i)) return false;
       m.erase(m.find(i));
       if (static_cast<int>(m.size()) != i) return false;
     }
@@ -97,7 +109,8 @@ struct MapTester {
     map_type<int64_t, int64_t> m;
     int nkeys = 10 * 1000;
     for (int i = 0; i < nkeys; ++i) { m.insert(make_pair(i, i)); }
-    for (int i = nkeys; i >= 0; --i) { 
+    for (int i = nkeys - 1; i >= 0; --i) { 
+      fail_unless(m.find(i) != m.end());
       m.erase(i);
       if (static_cast<int>(m.size()) != i) return false;
     }

cxxmph/mph_map.h

@@ -31,6 +31,7 @@
 #include <vector>
 #include <utility>  // for std::pair
 
+#include "string_util.h"
 #include "hollow_iterator.h"
 #include "mph_bits.h"
 #include "mph_index.h"
@@ -156,7 +157,7 @@ MPH_MAP_METHOD_DECL(insert_return_type, insert)(const value_type& x) {
 }
 
 MPH_MAP_METHOD_DECL(void_type, pack)() {
-  // fprintf(stderr, "Paki %d values\n", values_.size());
+  // CXXMPH_DEBUGLN("Packing %v values")(values_.size());
   if (values_.empty()) return;
   assert(std::unordered_set<key_type>(make_iterator_first(begin()), make_iterator_first(end())).size() == size());
   bool success = index_.Reset(
@@ -196,7 +197,9 @@ MPH_MAP_METHOD_DECL(void_type, clear)() {
 }
 
 MPH_MAP_METHOD_DECL(void_type, erase)(iterator pos) {
-  present_[pos.it_ - begin().it_] = false;
+  assert(pos.it_ - values_.begin() < present_.size());
+  assert(present_[pos.it_ - values_.begin()]);
+  present_[pos.it_ - values_.begin()] = false;
   *pos = value_type();
   --size_;
 }
@@ -227,9 +230,7 @@ MPH_MAP_INLINE_METHOD_DECL(my_int32_t, index)(const key_type& k) const {
   }
   if (__builtin_expect(index_.size(), 1)) {
     auto id = index_.index(k);
-    if (__builtin_expect(present_[id], true)) { 
-      return id;
-    }
+    if (__builtin_expect(present_[id], true)) return id;
   }
   return -1;
 }

cxxmph/mph_map_test.cc

@@ -11,6 +11,7 @@ using namespace cxxmph;
 
 typedef MapTester<mph_map> Tester;
 
+/*
 CXXMPH_CXX_TEST_CASE(small_insert, Tester::small_insert);
 CXXMPH_CXX_TEST_CASE(large_insert, Tester::large_insert);
 CXXMPH_CXX_TEST_CASE(small_search, Tester::small_search);
@@ -20,4 +21,5 @@ CXXMPH_CXX_TEST_CASE(string_search, Tester::string_search);
 CXXMPH_CXX_TEST_CASE(rehash_zero, Tester::rehash_zero);
 CXXMPH_CXX_TEST_CASE(rehash_size, Tester::rehash_size);
 CXXMPH_CXX_TEST_CASE(erase_value, Tester::erase_value);
+*/
 CXXMPH_CXX_TEST_CASE(erase_iterator, Tester::erase_iterator);

cxxmph/string_util.h

@@ -52,11 +52,6 @@ inline void tostr(ostream* out, const pair<F, S>& v) {
   *out << ")";
 }
 
-template <typename... Args, class T >
-std::string infoln(const std::string& format_string, Args&&... args) {
-  return stream_printf(format_string, 0, &std::cout, std::forward<Args>(args)...);
-}
-
 bool stream_printf(
     const std::string& format_string, uint32_t offset, std::ostream* out);
 
@@ -77,18 +72,14 @@ template <> struct pod_snprintf<true> {
 template <typename T, typename... Args>
 bool stream_printf(const std::string& format_string, uint32_t offset,
                    std::ostream* out, const T& value, Args&&... args) {
-  auto b = format_string.c_str() + offset;
   auto txt = format_string.c_str() + offset;
-  for (; *txt; ++txt) {
-    if (*txt == '%') {  
-      if (*(txt + 1) != '%') break;
-      *out << "%";
-      return stream_printf(format_string, offset + 2, out, value,
-                           std::forward<Args>(args)...);
-    }
-  }
-  if (txt != b) {
+  while (*txt) {
+    auto b = txt;
+    for (; *txt != '%'; ++txt);
+    if (*(txt + 1) == '%') ++txt;
+    else if (txt == b) break;
     *out << string(b, txt - b);
+    if (*(txt - 1) == '%') ++txt;
   }
   auto fmt = txt + 1;
   while (*fmt && *fmt != '%') ++fmt;
@@ -116,15 +107,20 @@ std::string format(const std::string& format_string, Args&&... args) {
   return out.str();
 }
 
+template <typename... Args>
+void infoln(const std::string& format_string, Args&&... args) {
+  stream_printf(format_string + "\n", 0, &std::cout, std::forward<Args>(args)...);
+}
+
 struct variadic_print {
   variadic_print(const std::string& file, uint32_t line, std::ostream* out,
                  const std::string& format_line) 
      : file_(file), line_(line), out_(out), format_line_(format_line) {}
   template <typename... Args>
   void operator()(Args&&... args) {
-    std::string fancy_format = "%s:%d: ";
-    fancy_format += format_line_;
-    stream_printf(fancy_format, 0, out_, std::forward<Args>(args)...);
+    std::string fancy_format = "%v:%d: ";
+    fancy_format += format_line_ + "\n";
+    stream_printf(fancy_format, 0, out_, file_, line_, std::forward<Args>(args)...);
   }
   const std::string& file_;
   const uint32_t& line_;

cxxmph/string_util_test.cc

@@ -1,5 +1,27 @@
 #include "string_util.h"
-int main(int argc, char** argv) {
-  return 0;
+#include "test.h"
+
+using namespace cxxmph;
+
+bool test_format() {
+  string expected = " %% 4 foo 0x0A bar ";
+  string foo = "foo";
+  string fmt = format(" %%%% %v %v 0x%.2X bar ", 4, foo, 10);
+  fail_unless(fmt == expected, "expected\n-%s-\n got \n-%s-", expected.c_str(), fmt.c_str());
+  return true;
 }
 
+bool test_infoln() {
+  infoln(string("%s:%d: MY INFO LINE"), __FILE__, __LINE__);
+  return true;
+}
+
+
+bool test_macro() {
+  CXXMPH_DEBUGLN("here i am")();
+  return true;
+}
+
+CXXMPH_TEST_CASE(test_format)
+CXXMPH_TEST_CASE(test_infoln)
+CXXMPH_TEST_CASE(test_macro)