Consider using C++ for the internals of shapelib

[schwehr]

Shapelib the library already compiles pretty cleanly as C++. What do people think about making that always the case. I propose keeping the API as C, but we can then have things like constexpr and a cleanup class that would make the code easier to follow. Things I'm thinking about:

• constexpr can be used for the length of arrays
• using unique_ptr for internal only allocations
• Something like absl::Cleanup with lambda's greatly simplify returns from functions that are holding resources (like open hdf or shp file references)
• and...

#25 talks about these:

#ifdef __cplusplus
#define STATIC_CAST(type,x) static_cast<type>(x)
#define REINTERPRET_CAST(type,x) reinterpret_cast<type>(x)
#define CONST_CAST(type,x) const_cast<type>(x)
#define SHPLIB_NULLPTR nullptr
#else
#define STATIC_CAST(type,x) ((type)(x))
#define REINTERPRET_CAST(type,x) ((type)(x))
#define CONST_CAST(type,x) ((type)(x))
#define SHPLIB_NULLPTR NULL
#endif

Thoughts?

[rouault]

I would be fine about that for GDAL usage, but PostGIS (@pramsey ) or GRASS GIS ( @neteler ) may want to chim in to say if their projects have a C++11 compiler set up.

[schwehr]

@rouault Exactly. I was hoping to engage more of the users of shapelib via #32

[schwehr]

This one is already C++: https://github.com/fulcrumapp/shp-ruby/tree/master/ext/shp

[schwehr]

I was thinking about starting with the bin targets as they aren't linked against. And then probably with contrib first

But none of this should happen fast. I'd rather have unit testing in place before doing anything major. And it would be good to set up a release process and cut a branch point before doing this. That was, there is a place for fixes to for folks who can't switch to C++11 (which is where I assume this switch would go first)

[neteler]

I would be fine about that for GDAL usage, but PostGIS (@pramsey ) or GRASS GIS ( @neteler ) may want to chim in to say if their projects have a C++11 compiler set up.

We are currently defining this in a new RFC: https://trac.osgeo.org/grass/wiki/RFC/7_LanguageStandardsSupport

Citation: "However, C++11 is at this date in general considered the standard and until compelling reasons argue otherwise, the C++11 standard should be policy of the GRASS GIS project."

(perhaps @metzm, @nilason or others still have additions here)

[schwehr]

See also

• https://gdal.org/development/rfc/rfc68_cplusplus11.html
• https://trac.osgeo.org/geos/wiki/RFC5

[metzm]

In GRASS, we use a customized subset of shapelib which is only used to handle dbf files.

Thus using C++ for the internals of shapelib does not immediately affect GRASS. If shapelib moves from C to C++, GRASS will decide when it is appropriate to follow this change and update its own copy of shapelib.

In short, no objections from my side as GRASS developer maintaining the local copy of shapelib (ok, last change was 2016) in GRASS.

[pramsey]

Sorry, why change a perfectly lovely pure C library to a C++ library?

[schwehr]

That's a reasonable question to ask why C++ for the internals of shapelib. However, I can't say I'd call shapelib lovely.

Some partial thoughts:

The internal code is very hard for me to follow. It might be correct, but it's not obviously correct as I read it.

I can use some very boring parts of C++ to make the code more robust and be much clearer in what it's doing. None of these changes would be exposed to existing users of the library. This is internal only. I have no intension of altering the existing API.

Some of the things that would make parts of the code much more compact, more capable, and hopefully easier for the compiler to optimize:

• std::string to avoid bare mallocs and any other direct memory manipulation
• range based loops for internal iteration over things
• std::vector
• #define -> constexpr when possible
• the ability to make more things const
• use std::unique_ptr to avoid worry of freeing on error
• std::numeric_limits makes dealing with valid ranges much easier
• nullptr has a type whereas NULL is what it is
• The ability to always use the more specific casting from C++
• (less importantly) anonymous namespaces rather than static for local functions

Things I am proposing to not include:

• Inheritance
• C++20 and probably C++17 (so no std::string_view)
• streams
• exceptions
• aggressive overloading
• Altering the structs in shapefil.h as that is a part of the currently exposed interface

Starting with some of the binaries, especially in contrib would likely be best. Some of the programs look to be partially completed ideas. Having the C++ stdlib available makes flushing out some of these easier.

For C++ users (who know what we are getting into), we could design a common C++ API that is optional and separate. There are several floating around that can't be contributed back to the project. It would be nice to provide one to shapelib that is okay for anyone to opt into. This doesn't require the internals of shapelib be C++, but it does make it easier.

Other thoughts definitely welcome and I'm not expecting to get to this any time soon.

[pramsey]

Sounds OK in principle I suppose. We have our copy in-tree and probably won't change that. We managed to maintain a pure-C stance for a long time, though that eroded in the last cycle. As long as the API doesn't change and features don't leak in sounds good to me.