Upgrade to clipper2?

[mpadge]

Just wondering if you've considered upgrading to clipper2. It really is a huge improvement! If so, I would also like to ask whether you'd consider a large-scale restructure through bundling a vendored version of cpp11, instead of current configure files? I would argue the sole advantage there that any bugs in compiling and/or installing would then become effectively shared bugs through being traced back to cpp11, so any problems with your build system would be shared within a much larger community than this package alone.

Here is an example of it in action (using old clipper1). Direct interfaces with ClipperLib remain very similar to your current ones, but no longer need any of the C interfaces to SEXP objects. Your code would be considerably simpler, the package could still remain dependency-free, and as far I see it, there'd only be one potential disadvantage of relying on ongoing functionality of external cpp11 code, but that has the advantage described above, and alleviates the disadvantages of currently fragile build system. Interested to hear your thoughts?

[rubak]

We have indeed discussed upgrading to Clipper2 when we first noticed the new version on GitHub about a year ago. Other projects have kept us back from attacking it.

I will meet @baddstats in another connection next week, so we can try to find a bit of time to discuss your idea. Can you elaborate a bit more on the cpp11 idea? As I understand it:

• We would still have the original source files from Clipper2 in src/ together with an additional interface file, say src/interface.cpp (which would then use cpp11 datatypes rather than SEXP).
• There will be no Makevars and configue.
• We would have a inst/ containing header files copied from cpp11.
• The cpp11 package is only required for developers building the package before CRAN submission, and executing a specific cpp11 function will generate src/cpp11.cpp and with this file the package is ready for submission.
• A normal package update would be to update the original Clipper2 code, changing interface.cpp if needed and in that case run cpp11 to generate a new src/cpp11.cpp.
• If something odd is discovered with the cpp11 interface an update of the contents of inst/ may also be needed.

Is that correct? If not could you try to spell out a similar bullet point list?

Edit: Clipper2 says "C++: Requires C++17 (but could be modified to C++11 with only minor changes)". Do you have any thoughts on the minor changes?

[mpadge]

@rubak Thanks for the quick response. Those points are exactly as I would have written them so yes, your understanding there is bang-on. I find the defined cpp11 types for int and double very intuitive and convenient to work with (there are issues with other types, but clipper is pure int, so all is good there.)

---

I did have an idea of the C++17 specifics of clipper2, which was what originally prevented me from directly integrating that within my own code, but I didn't document the issues. I'll have a look at my prototype clipper2 integration and get back to you if anything comes back to me in the meantime. Thanks!

---

Edit: It's somewhere in CPP/Clipper2Lib/include/clipper2/clipper.export.h introduced in this commit. I suspect the C++17 dependency is related to the construction of the EXTERN_DLL_EXPORT objects, but it all seems to me pure C++11-compliant, so I'm not sure?

[baddstats]

@rubak and @mpadge Thank you both for spending time and energy on this. A couple of constraints to think about:

• polyclip needs to remain free open source, so any vendor code would need to be FOSS
• the configure script detects whether there is an existing installation of Clipper and uses that installation instead of compiling our version of Clipper from source. That feature was requested by a user. I'd like to retain this if possible

[rubak]

Regarding the first point there is no problem as far as I can see. The cpp11 headers are MIT license which is very permissive and you are allowed to put them in a GPL project and release all the code as GPL (with copyright attribution to the cpp11 code).

The second point I didn't think about. I do like this feature, so maybe that is an argument for first trying to fix the configure file of the current polyclip, which hopefully isn't a huge task. @mpadge do you have some specific pointers to what we should change to avoid compiler conflicts?

Then it could be a second task to try to make a GitHub-only polyclip2 package based on Clipper2 with cpp11 bindings. In the first take I think we should just leave the C++17 requirement as it is. This is the default C++ version in R 4.3 anyway. Once polyclip2 is working we can consider if and how it should replace polyclip.

[mpadge]

I agree - using a system version of clipper if available is very advantageous, and cpp11 would remove that ability. So i suggest retracting the cpp11 suggestions. There is one further important advantage of not using cpp11, through that strictly requiring linking to code in the /src directory, and not in /inst. I generally bundle clipper in my own projects because I need access to the actual C++ routines. polyclip currently has all code in /src, but moving the headers to /inst would enable LinkingTo this package for direct access to the C++ code. Not suggesting that need be done urgently, but using cpp11 would prevent LinkingTo, which is another distinct disadvantage.

As for configure, the strict C++17 requirement of clipper2 would make updating for CRAN easier. Updating current clipper1 version may be a bit trickier. I got a direct email from Uwe L confirming that they simply check grep -r CXX11, which would fail current configure file, and require re-writing either to avoid explicit specification (which I'm not even sure how to do?), or to hard-code C++17 regardless, simply to satisfy that cran requirement.

[rubak]

I sounds nice to move headers to /inst if possible so other packages can link to polyclip and access the C++ code directly. However, I only think this is safe and easy for a header only library. The WRE has a section about linking between packages. I think trying this out belongs to the long term todo list.

An important question before editing configure files is whether we want to still support R <= 3.5 with new versions of polyclip? Everything is much easier from R 3.6 because C++11 is default. I'm a bit in doubt what to do, but I lean towards only supporting R >= 3.6. If you have a really old system you then have to download an old version of polyclip and install that manually.

Updating configure is a bit more urgent as the current version does create a NOTE on R CMD check. I don't know how fast CRAN will demand a new version passing checks flawlessly.