simongog / sdsl-lite

Succinct Data Structure Library 2.0
Other
2.21k stars 350 forks source link

Reference-Return at post-increment #207

Closed koeppl closed 9 years ago

koeppl commented 9 years ago

One of my students has found a bug in cst_sct3.hpp that caused runtime failure for gcc 4.9 and clang 3.5.0 when using the -O0 debug optimization. Here is a patch that also changes children to a call-by-reference (possible speed-up):

diff --git a/include/sdsl/cst_sct3.hpp b/include/sdsl/cst_sct3.hpp
index 3066f7f..7e4da67 100644
--- a/include/sdsl/cst_sct3.hpp
+++ b/include/sdsl/cst_sct3.hpp
@@ -621,7 +621,7 @@ class cst_sct3
          *  \par Time complexity
          *     \f$ \Order{1}\f$
          */
-        cst_node_child_proxy<cst_sct3> children(const node_type v) const
+        cst_node_child_proxy<cst_sct3> children(const node_type& v) const
         {
             return cst_node_child_proxy<cst_sct3>(this,v);
         }
diff --git a/include/sdsl/suffix_tree_helper.hpp b/include/sdsl/suffix_tree_helper.hpp
index 8850052..d30e11d 100644
--- a/include/sdsl/suffix_tree_helper.hpp
+++ b/include/sdsl/suffix_tree_helper.hpp
@@ -36,7 +36,7 @@ class cst_node_child_proxy_iterator : public std::iterator<std::forward_iterator
             m_cur_node = m_cst->sibling(m_cur_node);
             return *this;
         }
-        iterator_type& operator++(int) {
+        iterator_type operator++(int) {
             iterator_type it = *this;
             ++(*this);
             return it;
simongog commented 9 years ago

Hi Dominik, yes the return should be by-value. Would you like to make a pull request with the change, or should I fix it? Thank you very much and best, Simon

koeppl commented 9 years ago

Hi Simon, I created the request https://github.com/simongog/sdsl-lite/pull/208