[PATCH] Use C++11 for the code base

[PATCH] Use C++11 for the code base

[Dodji Seketeli]

Hello,

As the Enterprise Linux 6 platform has now essentially reached its
end of life for what it's worth (the Fedora EPEL6 distribution is not
maintained anymore) nothing ties us to using C++03 only anymore.

So, I think it makes sense to move the code base to the C++11
standard.

Why C++11 and not, say, C++14 or more?  Well, the more direct reason I
see is that we need to support long life cycle platforms, the older
one being Enterprise Linux 7 currently.  This is the Fedora EPEL7
distribution, in concrete terms.  And in that distribution, the
compiler is GCC 4.8.x.  And it supports C++11.

In practise, nothing changes in the code that is already there.

The new code however can use C++11 constructs just fine.

I have updated the CONTRIBUTING file to write down some of the unwritten
cultural biases of the current code base.  Hopefully these few lines
will help to shed some light on the choices made so far.

The update to that file also enacts the use of C++11 and sets some
limits to what we expects in terms of what the code base would look
like.

configure.ac is modified to unconditionally pass -std=c++11 to the
compiler and express that in the configuration text displayed at the
end of the configuration stage.

Some Makefile.am files are updated accordingly.

	* CONTRIBUTING: Enact use of c++11. Also, we favor those who
	read/debug/maintain the code as opposed to those who write it ;-)
	* configure.ac: Switch to c++11 unconditionally.
	* src/Makefile.am: Adjust.
	* tests/Makefile.am: Adjust.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>

Applied to master.

---
 CONTRIBUTING      | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure.ac      | 25 ++++++++++---------------
 src/Makefile.am   | 10 +++-------
 tests/Makefile.am | 15 ++++-----------
 4 files changed, 72 insertions(+), 33 deletions(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index b1f58df..7e3e856 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -96,6 +96,61 @@ So, to be complete the regression checking command to run against your
 patch should be: "make check-self-compare distcheck -j16", if you have
 a machine with a 16 threads processors, for instance.
 
+Coding language and style
+==========================
+
+The coding style is self evident when reading the source code.  So
+please, stick to and mimic what is already in there for the sake of
+consistency at very least.  Just for history, it's derived from the
+style of the C++ standard library from the GNU project.
+
+As of libabigail 2.0, the language we use is C++ 11.  The level
+supported is the one supported by the GCC 4.8.x series of compilers.
+This should be old and well tested enough to be supported by your
+current favorite compiler.
+
+Initially, the code base of the project was written in C++03, with the
+TR1 extensions.  That heritage is well visible in the code base as it
+is today.
+
+Please do not rush and send gazillions of patches that just re-write
+tons of code into your favorite C++ 11 flavour du jour.  We will
+likely reject those patches.  We want to keep the history of the code
+base in such a way that tools like "git blame <sourcefile> are still
+useful.
+
+So we'll accept patches changing parts of the code base to more recent
+C++ 11 constructs only if you happen to add functionality or fix
+things in that area.  If it makes "cultural common" sense to adopt
+those constructs.  What I mean by "cultural" is that must make sense
+in relative to the culture of the project.  And yes, that is
+subjective.  Sorry.
+
+As a generic rule, we tend to favor the lowest possible level of
+abstraction that makes sense without requiring future maintainers of
+the project to have a PhD in design patterns.  We are not impressed by
+design patterns.  We use them where they make clear sense, but we
+don't idolize them.  Put it another way, we will always favor the one
+who *READS* and debug the code over the one who writes it.  To put
+things in a hypothetical perspective, we'll rather accept a repetitive
+code that stays simple to read and debug over a highly abstract one
+using meta programming to save a few lines of repetitive code located
+in a very small number of places.
+
+Really, in this project, we care about low level binary analysis
+stuff.  Issues in that area can be hard to reproduce and quite
+challenging to debug.  So having tons of abstraction layers in the
+code base have proven to be a maintenance burden over the years, from
+our experience in working on similar projects.  So please help us
+avoid those mistakes that we make just for the pleasure of writing
+what can look as "pleasant code" at a first naive sight.
+
+That being said, we also love cleanly designed APIs that are fairly
+re-usable and well documented.  And we also praise abstraction and
+modularisation that we recognize as being the most basic tools of any
+engineer.  So we like to think about ourselves as well rounded people
+who care about maintaining things for a long time to come :-)
+
 Launching regression tests in Valgrind
 --------------------------------------
 
diff --git a/configure.ac b/configure.ac
index 2f15fd0..37b7882 100644
--- a/configure.ac
+++ b/configure.ac
@@ -85,12 +85,6 @@ AC_ARG_ENABLE(zip-archive,
 	      ENABLE_ZIP_ARCHIVE=no)
 
 
-AC_ARG_ENABLE(cxx11,
-	      AS_HELP_STRING([--enable-cxx11=yes|no],
-			     [enable features that use the C++11 compiler]),
-	      ENABLE_CXX11=$enableval,
-	      ENABLE_CXX11=no)
-
 AC_ARG_ENABLE(apidoc,
 	      AS_HELP_STRING([--enable-apidoc=yes|no|auto],
 			     [enable generation of the apidoc in html]),
@@ -160,6 +154,11 @@ AC_LANG([C++])
 AC_LANG_COMPILER_REQUIRE
 
 dnl
+dnl We use C++11
+dnl
+CXX_STANDARD=c++11
+
+dnl
 dnl check if the c++ compiler has support __attribute__((visibility("hidden")))
 dnl
 AC_MSG_NOTICE([checking for GCC visibility attribute support ...])
@@ -588,14 +587,6 @@ AM_CONDITIONAL(ENABLE_ZIP_ARCHIVE, test x$ENABLE_ZIP_ARCHIVE = xyes)
 DEPS_CPPFLAGS="$XML_CFLAGS $LIBZIP_CFLAGS"
 AC_SUBST(DEPS_CPPFLAGS)
 
-dnl Handle conditional use of a C++11 compiler
-if test x$ENABLE_CXX11 = xyes; then
-   CXXFLAGS="$CXXFLAGS -std=c++11"
-   AC_DEFINE([WITH_CXX11], 1, [Defined to 1 if a C++11 compiler is used])
-fi
-
-AM_CONDITIONAL(ENABLE_CXX11, test x$ENABLE_CXX11 = xyes)
-
 dnl Check for the presence of doxygen program
 
 if test x$ENABLE_APIDOC != xno; then
@@ -673,6 +664,9 @@ if test x$ENABLE_UBSAN = xyes; then
     CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
 fi
 
+dnl Set the level of C++ standard we use.
+CXXFLAGS="$CXXFLAGS -std=$CXX_STANDARD"
+
 dnl Check if several decls and constant are defined in dependant
 dnl libraries
 HAS_EM_AARCH64=no
@@ -949,11 +943,12 @@ AC_MSG_NOTICE([
     C Compiler                                     : ${CC}
     C++ Compiler		                   : ${CXX}
     GCC visibility attribute supported             : ${SUPPORTS_GCC_VISIBILITY_ATTRIBUTE}
+    CXXFLAGS	   	     			   : ${CXXFLAGS}
     Python					   : ${PYTHON}
 
  OPTIONAL FEATURES:
     Enable zip archives                            : ${ENABLE_ZIP_ARCHIVE}
-    Use a C++-11 compiler                          : ${ENABLE_CXX11}
+    C++ standard level                             : ${CXX_STANDARD}
     libdw has the dwarf_getalt function            : ${FOUND_DWARF_GETALT_IN_LIBDW}
     Enable rpm support in abipkgdiff               : ${ENABLE_RPM}
     Enable rpm 4.15 support in abipkgdiff tests    : ${ENABLE_RPM415}
diff --git a/src/Makefile.am b/src/Makefile.am
index d7990e3..26fc4d1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -4,13 +4,9 @@ libabigaildir=$(libdir)
 
 AM_CXXFLAGS = $(VISIBILITY_FLAGS)
 
-if ENABLE_CXX11
-CXX11_SOURCES = abg-viz-common.cc			\
-		abg-viz-dot.cc				\
+VIZ_SOURCES = abg-viz-common.cc	\
+		abg-viz-dot.cc	\
 		abg-viz-svg.cc
-else
-CXX11_SOURCES =
-endif
 
 libabigail_la_SOURCES =			\
 abg-internal.h				\
@@ -42,7 +38,7 @@ abg-tools-utils.cc			\
 abg-elf-helpers.h			\
 abg-elf-helpers.cc			\
 abg-regex.cc				\
-$(CXX11_SOURCES)
+$(VIZ_SOURCES)
 
 libabigail_la_LIBADD = $(DEPS_LIBS)
 libabigail_la_LDFLAGS = -lpthread -Wl,--as-needed -no-undefined
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 6effbc2..655facb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -3,19 +3,11 @@ SUBDIRS = data
 
 ZIP_ARCHIVE_TESTS =
 if ENABLE_ZIP_ARCHIVE
-ZIP_ARCHIVE_TESTS += runtestwritereadarchive
-if ENABLE_CXX11
-ZIP_ARCHIVE_TESTS += runtestdot
-endif
+ZIP_ARCHIVE_TESTS += runtestwritereadarchive runtestdot
 endif
 
 AM_CXXFLAGS = $(VISIBILITY_FLAGS)
 
-CXX11_TESTS =
-if ENABLE_CXX11
-CXX11_TESTS += runtestsvg
-endif
-
 FEDABIPKGDIFF_TEST =
 if ENABLE_FEDABIPKGDIFF
 if ENABLE_RUNNING_TESTS_WITH_PY3
@@ -55,9 +47,10 @@ runtestlookupsyms		\
 runtestreadwrite		\
 runtestsymtab			\
 runtesttoolsutils		\
+runtestsvg			\
 $(FEDABIPKGDIFF_TEST) 		\
-$(ZIP_ARCHIVE_TESTS)		\
-$(CXX11_TESTS)
+$(ZIP_ARCHIVE_TESTS)
+
 
 if ENABLE_RUNNING_TESTS_WITH_PY3
 TESTS += runtestdefaultsupprspy3.sh
-- 
1.8.3.1


-- 
		Dodji

Re: [PATCH] Use C++11 for the code base

[Ben Woodard]

> On Dec 3, 2020, at 5:26 AM, Dodji Seketeli via Libabigail <libabigail@sourceware.org> wrote:
> 
> Hello,
> 
> As the Enterprise Linux 6 platform has now essentially reached its
> end of life for what it's worth (the Fedora EPEL6 distribution is not
> maintained anymore) nothing ties us to using C++03 only anymore.
> 
> So, I think it makes sense to move the code base to the C++11
> standard.
> 
> Why C++11 and not, say, C++14 or more?  Well, the more direct reason I
> see is that we need to support long life cycle platforms, the older
> one being Enterprise Linux 7 currently.  This is the Fedora EPEL7
> distribution, in concrete terms.  And in that distribution, the
> compiler is GCC 4.8.x.  And it supports C++11.

Why don’t you move the —enable-cxx11 forward to —enable-cxx17 then we can make sure that the codebase has no problems moving forward to that level of the language. In particular I think that c++14 fixes some issues with c++11 and even though RHEL7 can’t handle it out of the box, it would be beneficial to move in that direction. 17 and 20 are more just to make sure that the code base is ready to be compiled with those stricter standards.
> 
> In practise, nothing changes in the code that is already there.
> 
> The new code however can use C++11 constructs just fine.
> 
> I have updated the CONTRIBUTING file to write down some of the unwritten
> cultural biases of the current code base.  Hopefully these few lines
> will help to shed some light on the choices made so far.
> 
> The update to that file also enacts the use of C++11 and sets some
> limits to what we expects in terms of what the code base would look
> like.
> 
> configure.ac is modified to unconditionally pass -std=c++11 to the
> compiler and express that in the configuration text displayed at the
> end of the configuration stage.
> 
> Some Makefile.am files are updated accordingly.
> 
> 	* CONTRIBUTING: Enact use of c++11. Also, we favor those who
> 	read/debug/maintain the code as opposed to those who write it ;-)
> 	* configure.ac: Switch to c++11 unconditionally.
> 	* src/Makefile.am: Adjust.
> 	* tests/Makefile.am: Adjust.
> 
> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
> 
> Applied to master.
> 
> ---
> CONTRIBUTING      | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> configure.ac      | 25 ++++++++++---------------
> src/Makefile.am   | 10 +++-------
> tests/Makefile.am | 15 ++++-----------
> 4 files changed, 72 insertions(+), 33 deletions(-)
> 
> diff --git a/CONTRIBUTING b/CONTRIBUTING
> index b1f58df..7e3e856 100644
> --- a/CONTRIBUTING
> +++ b/CONTRIBUTING
> @@ -96,6 +96,61 @@ So, to be complete the regression checking command to run against your
> patch should be: "make check-self-compare distcheck -j16", if you have
> a machine with a 16 threads processors, for instance.
> 
> +Coding language and style
> +==========================
> +
> +The coding style is self evident when reading the source code.  So
> +please, stick to and mimic what is already in there for the sake of
> +consistency at very least.  Just for history, it's derived from the
> +style of the C++ standard library from the GNU project.
> +
> +As of libabigail 2.0, the language we use is C++ 11.  The level
> +supported is the one supported by the GCC 4.8.x series of compilers.
> +This should be old and well tested enough to be supported by your
> +current favorite compiler.
> +
> +Initially, the code base of the project was written in C++03, with the
> +TR1 extensions.  That heritage is well visible in the code base as it
> +is today.
> +
> +Please do not rush and send gazillions of patches that just re-write
> +tons of code into your favorite C++ 11 flavour du jour.  We will
> +likely reject those patches.  We want to keep the history of the code
> +base in such a way that tools like "git blame <sourcefile> are still
> +useful.
> +
> +So we'll accept patches changing parts of the code base to more recent
> +C++ 11 constructs only if you happen to add functionality or fix
> +things in that area.  If it makes "cultural common" sense to adopt
> +those constructs.  What I mean by "cultural" is that must make sense
> +in relative to the culture of the project.  And yes, that is
> +subjective.  Sorry.
> +
> +As a generic rule, we tend to favor the lowest possible level of
> +abstraction that makes sense without requiring future maintainers of
> +the project to have a PhD in design patterns.  We are not impressed by
> +design patterns.  We use them where they make clear sense, but we
> +don't idolize them.  Put it another way, we will always favor the one
> +who *READS* and debug the code over the one who writes it.  To put
> +things in a hypothetical perspective, we'll rather accept a repetitive
> +code that stays simple to read and debug over a highly abstract one
> +using meta programming to save a few lines of repetitive code located
> +in a very small number of places.
> +
> +Really, in this project, we care about low level binary analysis
> +stuff.  Issues in that area can be hard to reproduce and quite
> +challenging to debug.  So having tons of abstraction layers in the
> +code base have proven to be a maintenance burden over the years, from
> +our experience in working on similar projects.  So please help us
> +avoid those mistakes that we make just for the pleasure of writing
> +what can look as "pleasant code" at a first naive sight.
> +
> +That being said, we also love cleanly designed APIs that are fairly
> +re-usable and well documented.  And we also praise abstraction and
> +modularisation that we recognize as being the most basic tools of any
> +engineer.  So we like to think about ourselves as well rounded people
> +who care about maintaining things for a long time to come :-)
> +
> Launching regression tests in Valgrind
> --------------------------------------
> 
> diff --git a/configure.ac b/configure.ac
> index 2f15fd0..37b7882 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -85,12 +85,6 @@ AC_ARG_ENABLE(zip-archive,
> 	      ENABLE_ZIP_ARCHIVE=no)
> 
> 
> -AC_ARG_ENABLE(cxx11,
> -	      AS_HELP_STRING([--enable-cxx11=yes|no],
> -			     [enable features that use the C++11 compiler]),
> -	      ENABLE_CXX11=$enableval,
> -	      ENABLE_CXX11=no)
> -
> AC_ARG_ENABLE(apidoc,
> 	      AS_HELP_STRING([--enable-apidoc=yes|no|auto],
> 			     [enable generation of the apidoc in html]),
> @@ -160,6 +154,11 @@ AC_LANG([C++])
> AC_LANG_COMPILER_REQUIRE
> 
> dnl
> +dnl We use C++11
> +dnl
> +CXX_STANDARD=c++11
> +
> +dnl
> dnl check if the c++ compiler has support __attribute__((visibility("hidden")))
> dnl
> AC_MSG_NOTICE([checking for GCC visibility attribute support ...])
> @@ -588,14 +587,6 @@ AM_CONDITIONAL(ENABLE_ZIP_ARCHIVE, test x$ENABLE_ZIP_ARCHIVE = xyes)
> DEPS_CPPFLAGS="$XML_CFLAGS $LIBZIP_CFLAGS"
> AC_SUBST(DEPS_CPPFLAGS)
> 
> -dnl Handle conditional use of a C++11 compiler
> -if test x$ENABLE_CXX11 = xyes; then
> -   CXXFLAGS="$CXXFLAGS -std=c++11"
> -   AC_DEFINE([WITH_CXX11], 1, [Defined to 1 if a C++11 compiler is used])
> -fi
> -
> -AM_CONDITIONAL(ENABLE_CXX11, test x$ENABLE_CXX11 = xyes)
> -
> dnl Check for the presence of doxygen program
> 
> if test x$ENABLE_APIDOC != xno; then
> @@ -673,6 +664,9 @@ if test x$ENABLE_UBSAN = xyes; then
>     CXXFLAGS="$CXXFLAGS -fsanitize=undefined"
> fi
> 
> +dnl Set the level of C++ standard we use.
> +CXXFLAGS="$CXXFLAGS -std=$CXX_STANDARD"
> +
> dnl Check if several decls and constant are defined in dependant
> dnl libraries
> HAS_EM_AARCH64=no
> @@ -949,11 +943,12 @@ AC_MSG_NOTICE([
>     C Compiler                                     : ${CC}
>     C++ Compiler		                   : ${CXX}
>     GCC visibility attribute supported             : ${SUPPORTS_GCC_VISIBILITY_ATTRIBUTE}
> +    CXXFLAGS	   	     			   : ${CXXFLAGS}
>     Python					   : ${PYTHON}
> 
>  OPTIONAL FEATURES:
>     Enable zip archives                            : ${ENABLE_ZIP_ARCHIVE}
> -    Use a C++-11 compiler                          : ${ENABLE_CXX11}
> +    C++ standard level                             : ${CXX_STANDARD}
>     libdw has the dwarf_getalt function            : ${FOUND_DWARF_GETALT_IN_LIBDW}
>     Enable rpm support in abipkgdiff               : ${ENABLE_RPM}
>     Enable rpm 4.15 support in abipkgdiff tests    : ${ENABLE_RPM415}
> diff --git a/src/Makefile.am b/src/Makefile.am
> index d7990e3..26fc4d1 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -4,13 +4,9 @@ libabigaildir=$(libdir)
> 
> AM_CXXFLAGS = $(VISIBILITY_FLAGS)
> 
> -if ENABLE_CXX11
> -CXX11_SOURCES = abg-viz-common.cc			\
> -		abg-viz-dot.cc				\
> +VIZ_SOURCES = abg-viz-common.cc	\
> +		abg-viz-dot.cc	\
> 		abg-viz-svg.cc
> -else
> -CXX11_SOURCES =
> -endif
> 
> libabigail_la_SOURCES =			\
> abg-internal.h				\
> @@ -42,7 +38,7 @@ abg-tools-utils.cc			\
> abg-elf-helpers.h			\
> abg-elf-helpers.cc			\
> abg-regex.cc				\
> -$(CXX11_SOURCES)
> +$(VIZ_SOURCES)
> 
> libabigail_la_LIBADD = $(DEPS_LIBS)
> libabigail_la_LDFLAGS = -lpthread -Wl,--as-needed -no-undefined
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 6effbc2..655facb 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -3,19 +3,11 @@ SUBDIRS = data
> 
> ZIP_ARCHIVE_TESTS =
> if ENABLE_ZIP_ARCHIVE
> -ZIP_ARCHIVE_TESTS += runtestwritereadarchive
> -if ENABLE_CXX11
> -ZIP_ARCHIVE_TESTS += runtestdot
> -endif
> +ZIP_ARCHIVE_TESTS += runtestwritereadarchive runtestdot
> endif
> 
> AM_CXXFLAGS = $(VISIBILITY_FLAGS)
> 
> -CXX11_TESTS =
> -if ENABLE_CXX11
> -CXX11_TESTS += runtestsvg
> -endif
> -
> FEDABIPKGDIFF_TEST =
> if ENABLE_FEDABIPKGDIFF
> if ENABLE_RUNNING_TESTS_WITH_PY3
> @@ -55,9 +47,10 @@ runtestlookupsyms		\
> runtestreadwrite		\
> runtestsymtab			\
> runtesttoolsutils		\
> +runtestsvg			\
> $(FEDABIPKGDIFF_TEST) 		\
> -$(ZIP_ARCHIVE_TESTS)		\
> -$(CXX11_TESTS)
> +$(ZIP_ARCHIVE_TESTS)
> +
> 
> if ENABLE_RUNNING_TESTS_WITH_PY3
> TESTS += runtestdefaultsupprspy3.sh
> -- 
> 1.8.3.1
> 
> 
> -- 
> 		Dodji
> 

[PATCH] CONTRIBUTING: Move "Coding language and style" section

[Thomas Schwinge]

This section got added in commit 4f8c9b170d80b920d13e8f291b347df74db34307
"Use C++11 for the code base", but unfortunately got placed in the middle
of the "Regression tests" section.  Move it after that one.

	* CONTRIBUTING: Move "Coding language and style" section.

Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
---
 CONTRIBUTING | 110 +++++++++++++++++++++++++--------------------------
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git CONTRIBUTING CONTRIBUTING
index 3a2073b3..24a483b2 100644
--- CONTRIBUTING
+++ CONTRIBUTING
@@ -99,61 +99,6 @@ So, to be complete the regression checking command to run against your
 patch should be: "make check-self-compare distcheck-fast -j16", if you have
 a machine with a 16 threads processors, for instance.
 
-Coding language and style
-==========================
-
-The coding style is self evident when reading the source code.  So
-please, stick to and mimic what is already in there for the sake of
-consistency at very least.  Just for history, it's derived from the
-style of the C++ standard library from the GNU project.
-
-As of libabigail 2.0, the language we use is C++ 11.  The level
-supported is the one supported by the GCC 4.8.x series of compilers.
-This should be old and well tested enough to be supported by your
-current favorite compiler.
-
-Initially, the code base of the project was written in C++03, with the
-TR1 extensions.  That heritage is well visible in the code base as it
-is today.
-
-Please do not rush and send gazillions of patches that just re-write
-tons of code into your favorite C++ 11 flavour du jour.  We will
-likely reject those patches.  We want to keep the history of the code
-base in such a way that tools like "git blame <sourcefile> are still
-useful.
-
-So we'll accept patches changing parts of the code base to more recent
-C++ 11 constructs only if you happen to add functionality or fix
-things in that area.  If it makes "cultural common" sense to adopt
-those constructs.  What I mean by "cultural" is that must make sense
-in relative to the culture of the project.  And yes, that is
-subjective.  Sorry.
-
-As a generic rule, we tend to favor the lowest possible level of
-abstraction that makes sense without requiring future maintainers of
-the project to have a PhD in design patterns.  We are not impressed by
-design patterns.  We use them where they make clear sense, but we
-don't idolize them.  Put it another way, we will always favor the one
-who *READS* and debug the code over the one who writes it.  To put
-things in a hypothetical perspective, we'll rather accept a repetitive
-code that stays simple to read and debug over a highly abstract one
-using meta programming to save a few lines of repetitive code located
-in a very small number of places.
-
-Really, in this project, we care about low level binary analysis
-stuff.  Issues in that area can be hard to reproduce and quite
-challenging to debug.  So having tons of abstraction layers in the
-code base have proven to be a maintenance burden over the years, from
-our experience in working on similar projects.  So please help us
-avoid those mistakes that we make just for the pleasure of writing
-what can look as "pleasant code" at a first naive sight.
-
-That being said, we also love cleanly designed APIs that are fairly
-re-usable and well documented.  And we also praise abstraction and
-modularisation that we recognize as being the most basic tools of any
-engineer.  So we like to think about ourselves as well rounded people
-who care about maintaining things for a long time to come :-)
-
 Launching regression tests in Valgrind
 --------------------------------------
 
@@ -229,6 +174,61 @@ make sure to add your new test input files to the
 tests/data/Makefile.am file, in the EXTRA_DIST variable.  Look at how
 things are organized in that file, and please do things similarly.
 
+Coding language and style
+==========================
+
+The coding style is self evident when reading the source code.  So
+please, stick to and mimic what is already in there for the sake of
+consistency at very least.  Just for history, it's derived from the
+style of the C++ standard library from the GNU project.
+
+As of libabigail 2.0, the language we use is C++ 11.  The level
+supported is the one supported by the GCC 4.8.x series of compilers.
+This should be old and well tested enough to be supported by your
+current favorite compiler.
+
+Initially, the code base of the project was written in C++03, with the
+TR1 extensions.  That heritage is well visible in the code base as it
+is today.
+
+Please do not rush and send gazillions of patches that just re-write
+tons of code into your favorite C++ 11 flavour du jour.  We will
+likely reject those patches.  We want to keep the history of the code
+base in such a way that tools like "git blame <sourcefile> are still
+useful.
+
+So we'll accept patches changing parts of the code base to more recent
+C++ 11 constructs only if you happen to add functionality or fix
+things in that area.  If it makes "cultural common" sense to adopt
+those constructs.  What I mean by "cultural" is that must make sense
+in relative to the culture of the project.  And yes, that is
+subjective.  Sorry.
+
+As a generic rule, we tend to favor the lowest possible level of
+abstraction that makes sense without requiring future maintainers of
+the project to have a PhD in design patterns.  We are not impressed by
+design patterns.  We use them where they make clear sense, but we
+don't idolize them.  Put it another way, we will always favor the one
+who *READS* and debug the code over the one who writes it.  To put
+things in a hypothetical perspective, we'll rather accept a repetitive
+code that stays simple to read and debug over a highly abstract one
+using meta programming to save a few lines of repetitive code located
+in a very small number of places.
+
+Really, in this project, we care about low level binary analysis
+stuff.  Issues in that area can be hard to reproduce and quite
+challenging to debug.  So having tons of abstraction layers in the
+code base have proven to be a maintenance burden over the years, from
+our experience in working on similar projects.  So please help us
+avoid those mistakes that we make just for the pleasure of writing
+what can look as "pleasant code" at a first naive sight.
+
+That being said, we also love cleanly designed APIs that are fairly
+re-usable and well documented.  And we also praise abstraction and
+modularisation that we recognize as being the most basic tools of any
+engineer.  So we like to think about ourselves as well rounded people
+who care about maintaining things for a long time to come :-)
+
 Sign your work
 ==============
 
-- 
2.25.1

Re: [PATCH] CONTRIBUTING: Move "Coding language and style" section

[Dodji Seketeli]

Hello,

Thomas Schwinge <thomas@codesourcery.com> a écrit:

[...]

>  CONTRIBUTING | 110 +++++++++++++++++++++++++--------------------------
>  1 file changed, 55 insertions(+), 55 deletions(-)
>
> diff --git CONTRIBUTING CONTRIBUTING

I am not sure why, but on patches I use and receive, this line looks
like:

diff --git a/CONTRIBUTING b/CONTRIBUTING

See the difference of format?  This is what I get using git
format-patch.

I am not sure why yours is different.  Any idea?  I am using git 2.33.

In any case, when I do "git am" on your patch, git refuses to apply it.
I had to edit the patch so that the line looks like:

diff a/CONTRIBUTING b/CONTRIBUTING

at least.

[...]

> This section got added in commit 4f8c9b170d80b920d13e8f291b347df74db34307
> "Use C++11 for the code base", but unfortunately got placed in the middle
> of the "Regression tests" section.  Move it after that one.
>
> 	* CONTRIBUTING: Move "Coding language and style" section.
>
> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>

Applied to master.  Thanks!

[...]

Cheers,

-- 
		Dodji

Re: [PATCH] CONTRIBUTING: Move "Coding language and style" section

[Thomas Schwinge]

Hi Dodji!

On 2021-12-17T19:59:35+0100, Dodji Seketeli <dodji@redhat.com> wrote:
> Thomas Schwinge <thomas@codesourcery.com> a écrit:
>>  CONTRIBUTING | 110 +++++++++++++++++++++++++--------------------------
>>  1 file changed, 55 insertions(+), 55 deletions(-)
>>
>> diff --git CONTRIBUTING CONTRIBUTING
>
> I am not sure why, but on patches I use and receive, this line looks
> like:
>
> diff --git a/CONTRIBUTING b/CONTRIBUTING
>
> See the difference of format?  This is what I get using git
> format-patch.
>
> I am not sure why yours is different.  Any idea?

Yes: local 'git diff' configuration unintentionally leaked into
'git format-patch' as used by 'git send-email'.  I've fixed this;
later patches should apply fine as sent.

> In any case, when I do "git am" on your patch, git refuses to apply it.
> I had to edit the patch so that the line looks like:
>
> diff a/CONTRIBUTING b/CONTRIBUTING
>
> at least.

..., or -- as you supposedly have found out a few minutes later ;-) --
apply with "git am -p0".  Sorry for that.


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955