boost geometry C++14 R 3.6 patch

[Jean-Romain]

When releasing BH 1.75 we had to move to C++14 (#76). This did not create any trouble except for windows oldrel (R 3.6). For what I've seen googlePolylines (@dcooley), rlas, lidR and wellknown (@sckott) are affected with the same errors which are

BH/include/boost/geometry/index/detail/varray_detail.hpp:131:13: error: 'is_trivially_copyable' is not a member of 'std'
BH/include/boost/geometry/index/detail/varray_detail.hpp:483:46: error: 'is_trivially_constructible' is not a member of 'std'
BH/include/boost/geometry/strategies/compare.hpp:175:63: error: call to non-constexpr function 'const _Tp& std::min(const _Tp&, const _Tp&) [with _Tp = unsigned int]

After spending too much time on it I post my patch here for other maitainers and future maitainers.

The fist two are easy. R 3.6 on windows uses gcc 4.9 and gcc 4.9 has a partial support of C++14. is_trivially_copyable and is_trivially_constructible are missing (SO question). Consequently I added substitutes of these properties before to include boost headers

namespace std
{
  template <typename T>
  class is_trivially_copyable
  {
  public:
    bool operator()() { return value; };
    static constexpr bool value = true;
  };

  template <typename T>
  class is_trivially_constructible
  {
  public:
    bool operator()() { return value; };
    static constexpr bool value = true;
  };
}

Here we can see that I added substitutes that return true. Well, ok it is bad but anyway if we write bad code with non trivially copyable/constructible objects this will fail on all other compilers that have a full support of C++14

The third issue however was super tricky to me. L170-175 of boost/geometry/strategies/compare.hpp we can see that we need constexpr const T& min( const T& a, const T& b );

return compare::detail::compare_loop
   <
      ComparePolicy,
      0,
      ((std::min)(geometry::dimension<Point1>::value,
              geometry::dimension<Point2>::value))
   >::apply(left, right);

However constexpr min is missing too in gcc 4.9. There is only const min. constexpr is resolved at compile time and there is no way to redefine min as constexpr without compilation error (AFAIK). So I copied compare.hpp in my package and patched it like that

return compare::detail::compare_loop <ComparePolicy,  0, 2>::apply(left, right);

And I included my patched header before all other boost header to avoid inclusion of the actuall boost header.

It is ok to me because I know that my points have 2 dimensions in the code involved so hardcoding it is unsafe but ok.

To finished I wrapped that in precompiler directives to do not mess up everything with recent compilers.

#if defined(__GNUC__) && (__GNUC__ < 5) && !defined(__clang__) && !defined(__llvm__) && !defined(__INTEL_COMPILER)
#include "gcc_4_9_patch/std_is_trivially.hpp"
#include "gcc_4_9_patch/boost_compare_patched.hpp"
#endif

#include <boost/geometry.hpp>
#include <boost/geometry/geometries/geometries.hpp>

I tested this patch with rlas on CRAN winbuilder oldrel and it passed ok. I now need to do the same with lidR.

[eddelbuettel]

I appreciate you working through this.

At present, CRAN has other packages where R-oldrel fails on Windows because of the toolchain. Nobody is really complaining. Come April and R 4.1.0, the current rtools4 toolchain with gcc-8 and g++-8 will become old-rel. The current r-devel also just switched to C++14 as the default compilation (where available, this would include Windows if I am not mistaken), and my inclination is actually to ... simply sit pat and wait this out.

What you did (including your modified headers before the official one, and relying on the include to prevent overwriting your definitions) is good. Maybe #undef`-ing the include guard and doing the include afterwards is better if other definitions are needed -- I don't know.

[Jean-Romain]

I know that it is not a big deal and the CRAN does not complain about it. But I believed that the old toolchain will still live for longer than that. I didn't know that it will be retired in April. My patch suddenly becomes less valuable :disappointed: . Anyway I'm sure lot of people still use R 3.6...

On my side I still have clang-USBAN + Solaris errors. The Solaris error comes out of nowhere, is not directly linked to lidR but seems to be caused by old libraries (gdal and proj) and I can't reproduce it. Because I can't fix Solaris error I tried to fix the windows-oldrel in attempt to get more chance to get my next minor update accepted... :game_die: Anyway, in the worst case I learned a lot doing so. Nothing lost.

By the way the good way to target gcc < 5 specifically seems to be

#if defined(__GNUC__) && (__GNUC__ < 5) && !defined(__clang__) && !defined(__llvm__) && !defined(__INTEL_COMPILER)

because clang defines __GNUC__ (SO question)

[eddelbuettel]

Hi @Jean-Romain just checking in as I will likely work on Boost 1.78 in the next few days preparing a release. From checking lidR and rlas at CRAN, everything seems peachy now without issue stemming from package BH -- correct?

(And ditto for googlePolyline -- :wave: at @dcooley ?)

[Jean-Romain]

Everything is currently fine on CRAN. Did you notice any trouble with Boost 1.78?

[eddelbuettel]

Have not gotten to it -- have to mark student projects this weekend. Should get to it by Monday and run reverse depends checks hopefully early next week.

[dcooley]

:waves back: - googlePolylines is currently all OK on CRAN. Let me know if the reverse depenency checks give me any grief and I'll update as well.

[eddelbuettel]

Sounds good. I have the package ready, but the reverse-depends box is currently tied up with another Rcpp run. Will get to BH 1.78.0 tomorrow but do not expect any trouble.

[eddelbuettel]

I think we can close this as the old issue (for 1.75.0) it was raised for got sorted (as "promised") with the disappearance of the older build platform, and neither 1.75.0 or now 1.78.0 seem to pose issue. Yay for good code.