[gdal-dev] Minimum supported C and C++ standards

[Even Rouault]

Le vendredi 06 mai 2016 23:07:26, Kurt Schwehr a écrit :
> There has been lively discussion on if/when GDAL moves to require compilers
> with newer C and C++ standard versions.  This email is meant to fork the
> this discussion away from the local stack + memset -> std::vector
> discussion.  I'm trying to draft a proposal based on the discussion, but it
> is a lot hardered.
> 
> I suggest folks think about starting off with an empty (or nearly empty)
> white list of features and give a look at documents like:
> 
> https://chromium-cpp.appspot.com/.

My personal issue is that most of the stuff mentionned in there is alien to me. 
At best only a rough theoretical understanding with no practical use. Perhaps 
I should find some time to educate myself on C++11/14 but reading pages like 
http://en.cppreference.com/w/cpp/utility/forward doesn't give a strong 
incentive to do so ;-)

> 
> GDAL is very different than Chromium, but I think it is great that they
> have a place for contributors to get a summary of what is considered okay
> and what is blacklisted.

I'll try to give my perspective over those choices of language features.

As Mateusz righly wrote, GDAL is a mix of C (not that much if you exclude the 
external libraries that are internalized), C++ consisting of C with classes 
(and avoiding the over use of classes that is typical in a bunch of Java 
projects), a few uses of templates for methods working on different data types 
in performance sensitive places.

C++03 already offers so many things to master that I can guess  that C++11 and 
C++14 have added again more "interesting" stuff on top of the old ways (since 
hardly nothing prevents you from using a shiny C++ feature next to/together 
with a C72 one. Which is porbably one of the main issue of the evolution of 
C++). Those new features certainly solve problems, but I'm pretty sure they 
also add new potential problems... I recommend having a look at 
http://horstmann.com/cpp/pitfalls.html and looking at the date. Now imagine 
what a later edition ~20 years later could be with all new C++ features. 
Actually look at the snippet in appendix for a good candidate (actually 
derived from a tutorial explaing rvalue reference)

Remembering a number of submitted patches, I'm pretty sure some occasional (or 
not so occasional) contributors to GDAL are not software engineers as their 
first qualification, which is OK since some parts of GDAL are really domain 
specific. And even those who are in the software business are not necessarily 
familiar with all C++ features, or on the contrary are familiar enough to know 
they don't want to use them, or worse, others can believe that they are 
familiar with them, but misuse them in subtle ways (never seen an 
enthusiastic fresh graduate wanting to apply blindly in your project the nice 
stuff seen in books?). IMHO we should try to focus to attract developers that 
are more interested by geospatial stuff than by C++ subtelties. GDAL purpose is 
not to be a showcase for C++ features, but to solve real-world problems. 
Hopefully everybody agrees with that.

If we move to a later C++ standard, or even use features of C++98 we currently 
don't use, I'd advocate for using things that are obviously making the code 
better / more readable. Honestly who finds that 
"std::unique_ptr<int *, std::function<void(char *)>> Vals(CPLCalloc(256, 0), 
CPLFree);" is obviously more readable, efficient and less error prone than
"std::vector Vals(256,0)" ? 

In my opinion the features of the language you use (of course, once you've 
understood and mastered them to the point where they are familiar to you 
without needing to open a programming book) play only a minor role in the 
maintenance and extensibility of the code (at least, they shouldn't !). The 
real challenges are to understand the overall design, the details of  the 
algorithms, the formats, the interactions between different parts of the code, 
what is performance sensitive and what is not, the history ... So definitely 
one of the best investment (and less risky) for maintenance is to add comments 
where they are needed. 

Perhaps rather than thinking to which C++ features we wish, we should 
concentrate on the problems we want to solve. The reasoning should be "We have 
this problem/need, we currently use solution X to address it, but this has the 
following defects and I think that feature Y is the best way to solve it".
And if that's about existing code, we must keep in mind that every change has 
the potential to break working things, so the ratio benefit / risk must be >> 1 
(>> meaning here "significantly greater than", and not the right bitshift 
operator, or the closing of 2 levels of templates, or the input stream 
operator, or an overloaded operator with a semantics left to the good will of 
the programmer)

The scope of changes should be also defined : restricted to internal 
implementation details or impacting the API ?

Tihs is a more trivial consideration, but code churn does make backporting 
harder (patches no longer apply cleanly). Although it's an inconvenience that 
is acceptable if it makes the future (significantly) better.

Cheers,

Even

(*) The following code compiles warning free with -std=c++11 -Wall -Wextra -
pedantic with GCC 5.2.0 but asserts at runtime. Find why. OK, you may find it 
easily since I mentionned this would assert. Now imagine this is a code 
contribution that you review. Would you figure out the error ?
Of course, with C or C with classes, the language already offers room for bugs. 
But the more language features, the more bug opportunities you add.

#include <string>
#include <cassert>

class Base
{
  protected:
    std::string x;
  public:
    Base(const std::string& xin) : x(xin) {}
    Base(Base const & rhs) : x(rhs.x) { }
    Base(Base&& rhs) { x = rhs.x; rhs.x = "moved"; }
};

class Derived: public Base
{
  protected:
    std::string y;
  public:
    Derived(const std::string& xin) : Base(xin), y(xin) { }
    Derived(Derived const & rhs) : Base(rhs), y(rhs.y) { }
    Derived(Derived&& rhs) : Base(rhs) { y = rhs.y; rhs.y = "moved"; }
    ~Derived() { assert(y == x); }
};

Derived foo(Derived d)
{
  return d;
}

int main()
{
  foo(Derived("x"));
  return 0;
}

-- 
Spatialys - Geospatial professional services
http://www.spatialys.com