kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi,

I have been alerted to some issues with the Boost-y unit tests. Nick
has helped me get set up with an Ubuntu build in docker.

I have tracked down the issues that caused compilation failures on old
GCC versions. It is because GCC <7 does not support the following
without at least an -fpermssive warning:

template<> struct boost::test_tools::print_log_value<TYPE>
{
    operator() (...){...}
}

Instead, you have to use the pre-C++11 namespace syntax:

namespace boost {
namespace test_tools {
   template<> struct print_log_value<TYPE>
   { ... }
} }

This would be OK, if verbose, but it is compounded by the problem that
in Boost 1.58 (Ubuntu 16.04 LTS), the namespace is boost::test_tools
and from 1.59, it is boost::test_tools:tt_detail. Having *either* two
or three braced levels makes this hard to do without horrible macros.

So, I propose to use the other way of printing in Boost tests: a free
operator<<:

std::ostream& operator<<( std::ostream& os, const Type& aThing )
{ ... }

This is slightly crusty, in that it's putting a free operator<< out
there that's not in the original library, but it is isolated and
contained only in the test code.

>From Boost 1.64, the customisation point boost_test_print_type is
introduced, which allows to not clutter up the namespace with stray
operator<< functions. Once we get to that minimum Boost version*, it's
then a simple change over to:

std::ostream& boost_test_print_type( std::ostream& os, const Type& aThing )
{ ... }

Attached is a patch to implement free operator<< logging for the only
problematic logger (for BOX2I). This is followed by the reversion of
decc7ed88, which should no longer be required.

I have checked this on GCC 8.2.1 and the Ubuntu build docker image.
JP, could you check this works on whatever system threw the error for
you?

Cheers,

John

* Also, once min Boost is >=1.59, lots of other hackish macros to work
around missing functions become superfluous, which will be nice!
Notably, Ubuntu LTS 18.04 uses Boost 1.65.

From e0236b351690f6de0a26e37a823b0c5b0d640280 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Sat, 12 Jan 2019 00:28:36 +0000
Subject: [PATCH 2/2] Revert "Try to fix a compil issue in qa test"

This reverts commit decc7ed888ec3a9b0afc8588e1209bddbc447495.

This compilation options (-fpermissive) is no longer needed,
as the code that caused the warnings on GCC <7 is no longer
present.

The reason this error was GCC bug #56480 [1], which disallowed
the following (valid, in C++11) syntax:

template<> struct namespace_a::namespace_b::a_struct<TYPE>
{...}

And instead insisted on:

namespace namespace_a {
namespace namespace_b {
    template<> struct a_struct<TYPE>
    {...}
}
}

[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
---
 qa/common/CMakeLists.txt | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/qa/common/CMakeLists.txt b/qa/common/CMakeLists.txt
index 4c85f2162..ab5bdcd26 100644
--- a/qa/common/CMakeLists.txt
+++ b/qa/common/CMakeLists.txt
@@ -51,7 +51,7 @@ set( common_srcs
     view/test_zoom_controller.cpp
 )
 
-set( common_libs
+set( common_libs 
     common
     legacy_gal
     polygon
@@ -62,11 +62,6 @@ set( common_libs
     ${wxWidgets_LIBRARIES}
 )
 
-# test_shape_arc.cpp does not compile without -fpermissive compil option:
-if( CMAKE_COMPILER_IS_GNUCXX )
-    set( CMAKE_CXX_FLAGS "-fpermissive ${CMAKE_CXX_FLAGS}" )
-endif()
-
 
 # Use code with GERBVIEW defines in place (primarily IU difference)
 add_executable( qa_common_gerbview ${common_srcs} )
-- 
2.20.1

From c255f204aa620e5fdf8380037ca2a38b923e35b6 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Fri, 11 Jan 2019 23:37:44 +0000
Subject: [PATCH 1/2] QA: Use free operator<< for Boost test logging

An unhappy conjunction of GCC bug #56480 [1] and Boost having
different namespaces for the print_log_value make it quite
ugly to support this method.

For the limited purposes of the unit tests, a free function in
the unit_test_utils header (in the absence of any implementation
in the main libraries) will do, even if it is a little intrusive.

From Boost 1.64 onwards, the customisation point boost_test_print_type
is avaiable, and anyt print functions should be transitioned over to
that method when the minimum Boost version is 1.64 or higher.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
---
 .../include/unit_test_utils/geometry.h           | 16 ++++++++--------
 .../include/unit_test_utils/unit_test_utils.h    | 12 ------------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/qa/unit_test_utils/include/unit_test_utils/geometry.h b/qa/unit_test_utils/include/unit_test_utils/geometry.h
index cc10cd381..2f8be6f70 100644
--- a/qa/unit_test_utils/include/unit_test_utils/geometry.h
+++ b/qa/unit_test_utils/include/unit_test_utils/geometry.h
@@ -8,17 +8,17 @@
 #include <math/box2.h>
 #include <math/vector2d.h>
 
+
 /**
- * Printer for BOX2I type
+ * Define a stream function for logging this type.
+ *
+ * TODO: convert to boost_test_print_type when Boost minver > 1.64
  */
-template <> struct BOOST_PRINT::print_log_value<BOX2I>
+std::ostream& operator<<( std::ostream& os, const BOX2I& aBox )
 {
-    void operator()( std::ostream& os, const BOX2I& aBox )
-    {
-        os << "BOX[ " << aBox.GetOrigin() << " + " << aBox.GetSize() << " ]";
-    }
-};
-
+    os << "BOX[ " << aBox.GetOrigin() << " + " << aBox.GetSize() << " ]";
+    return os;
+}
 
 namespace KI_TEST
 {
diff --git a/qa/unit_test_utils/include/unit_test_utils/unit_test_utils.h b/qa/unit_test_utils/include/unit_test_utils/unit_test_utils.h
index 9e54e0fbb..160d01d55 100644
--- a/qa/unit_test_utils/include/unit_test_utils/unit_test_utils.h
+++ b/qa/unit_test_utils/include/unit_test_utils/unit_test_utils.h
@@ -77,16 +77,4 @@
 
 #endif
 
-/*
- * Define a helper to make it easier to use the right namespace for
- * defining the print helpers like this:
- *
- * template<>
- * struct BOOST_PRINT::print_log_value< MY_TYPE >
- */
-#if BOOST_VERSION < 105900
-namespace BOOST_PRINT = boost::test_tools;
-#else
-namespace BOOST_PRINT = boost::test_tools::tt_detail;
-#endif
 #endif // UNIT_TEST_UTILS__H
\ No newline at end of file
-- 
2.20.1