Compilation errors with release candidate for BH package 1.75.0

[eddelbuettel]

As mentioned in issue 76 at the BH repo, its new version creates compilation issues with package TDA. I have not had time to really dive into this, but this may be caused by TDA also including some Boost headers to that we may get a conflict between different versions. The container library is included in BH so you could skip that; dealing with serialization may be trickier but an update to the current version may help.

Please reach out if you have any questions, and a big Thank You! for maintaining TDA on CRAN.

[jkim82133]

Hello, I am really sorry and realized few days ago that this email / issue thread was referencing TDA.

As you suggested, I removed duplicate container/detail/copy_move_algo.hpp from our own copy in TDA. Then TDA compiled fine, but after submitting it to CRAN, I got warning messages:

https://win-builder.r-project.org/incoming_pretest/TDA_1.7_20210126_170540/Windows/00check.log https://win-builder.r-project.org/incoming_pretest/TDA_1.7_20210126_170540/Debian/00check.log

The warning messages are basically complaining about memmove function [-Wclass-memaccess]. I had the same problem before. Back then, I fixed that warning by modifying the first argument of memmove in copy_move_algo.hpp with applying static_cast<void >: std::memmove(dest_raw, beg_raw, sizeof(value_type)n); -> std::memmove(static_cast<void >(dest_raw), beg_raw, sizeof(value_type)n); std::memmove(boost::movelib::iterator_to_raw_pointer(r), boost::movelib::iterator_to_raw_pointer(f), sizeof(value_type)n); -> std::memmove(static_cast<void >(boost::movelib::iterator_to_raw_pointer(r)), boost::movelib::iterator_to_raw_pointer(f), sizeof(value_type)n); std::memmove(boost::movelib::iterator_to_raw_pointer(r), boost::movelib::iterator_to_raw_pointer(f), sizeof(value_type)n); -> std::memmove(static_cast<void >(boost::movelib::iterator_to_raw_pointer(r)), boost::movelib::iterator_to_raw_pointer(f), sizeof(value_type)n); std::memmove((boost::movelib::iterator_to_raw_pointer)(r), (boost::movelib::iterator_to_raw_pointer)(f), sizeof(value_type)n); -> std::memmove(static_cast<void >((boost::movelib::iterator_to_raw_pointer)(r)), (boost::movelib::iterator_to_raw_pointer)(f), sizeof(value_type)*n);

At that time, I modified boost library by myself since as I understand, the warning message is coming from the boost library itself. Is my understanding correct? Or is it something to be fixed on TDA side?

Sorry again and thank you!

[eddelbuettel]

Hello, I am really sorry and realized few days ago that this email / issue thread was referencing TDA.

Well I opened it at your repo which commonly signals it concerns your code. Sorry if that wasn't clear. I believe I also emailed you in December just like the maintainers of the other packages.

Re the warnings: not sure. It may suggest a different initalization: ... with no trivial copy-assignment; use copy-assignment or copy-initialization instead What are the lines in your code that trigger this?

[eddelbuettel]

I think I just got one quieted down. In essence instead of

 MyType  x = MyType(a, b, c);

do

     MyType  x(a, b, c);

My editor reindent so apologies for the but basically this:

diff --git a/src/diag.cpp b/src/diag.cpp
index ed3f894..e5ec98c 100644
--- a/src/diag.cpp
+++ b/src/diag.cpp
@@ -402,9 +402,7 @@ Rcpp::List AlphaShapeFiltration(

   Rcpp::NumericMatrix coordinates;

-  Gudhi::Simplex_tree<> smplxTree =
-      AlphaShapeFiltrationGudhi< Gudhi::Simplex_tree<> >(
-          X, printProgress, Rprintf, coordinates);
+  Gudhi::Simplex_tree<> smplxTree(AlphaShapeFiltrationGudhi< Gudhi::Simplex_tree<> >(X, printProgress, Rprintf, coordinates));
   Rcpp::List filtration =
       filtrationGudhiToRcpp< Rcpp::List, Rcpp::NumericVector >(smplxTree);
   filtration.push_back(coordinates);
@@ -507,4 +505,4 @@ Rcpp::List AlphaComplexDiag(
       concatStlToRcpp< Rcpp::NumericMatrix >(persDgm, true, 3),
       concatStlToRcpp< Rcpp::NumericMatrix >(persLoc, false, 2),
       StlToRcppMatrixList< Rcpp::List, Rcpp::NumericMatrix >(persCycle));
-}
\ No newline at end of file
+}

Edit: Or maybe not. It's a bit hard to tell as there is as usual a fair amount of noise Boost and all that.

[jkim82133]

Sorry, I needed a quite amount of time to delve into this thing. As I understand, the [-Wclass-memaccess] warning message is to warn when the raw memory function call might bypass the class's constructor or copy assignment or etc. ( https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html ).

And for the case of R package TDA, this warning message comes when the container boost::flat_map is used with a class with nontrivial constructor or nontrivial copy assignment. And this warning message comes when 1) constructor / copy assignment is called, or 2) memory access function from the container boost::flat_map is called such as flat_map::emplace_hint() or flat_map::erase().

1) can be avoided when copy assignment is avoided such as your suggestion, so I tried this (which needed to restructure my code) and silenced many warning messages. But still, at some part of the algorithm calling emplace_hint() or erase() is inevitable. In boost library, these kind of memory functions are implemented with raw memory function memmove() (which is located in boost/container/detail/copy_move_algo.hpp ). For example, in the corresponding install.out ( https://win-builder.r-project.org/incoming_pretest/TDA_1.7.1_20210208_173610/Debian/00install.out , https://win-builder.r-project.org/incoming_pretest/TDA_1.7.1_20210208_173610/Windows/00install.out ), the [-Wclass-memaccess] message is coming from calling flat_map::emplace_hint() from boost library with the Gudhi class Gudhi::Simplex_tree_node_explicit_storage<Gudhi::Simplex_tree<> >, which has nontrivial copy constructor.

So I think it is neither the TDA package nor BH package to be fixed since the boost library is implemented in accessing raw memory functions. Rather, the [-Wclass-memaccess] warning message should be considered a false positive. So I am trying to convince CRAN package maintainers.