Post by Kurt SchwehrThere 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)
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 ;-)
Post by Kurt SchwehrGDAL 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