harfbuzz 1.6.1 and later require C++11

[ryandesign]

Hi, I'm the maintainer of harfbuzz in MacPorts. We noticed that harfbuzz 1.6.1 and later require C++11. This means harfbuzz 1.6.1 and later fail to build with any compiler included with Xcode on Mac OS X 10.6 Snow Leopard and earlier:

cc1plus: error: unrecognized command line option "-std=c++11"

harfbuzz 1.6.0 and earlier built fine on Mac OS X 10.4–10.6 using gcc 4.2.1.

I realize those are old systems and you may not want to limit yourself to the features available in those old compilers. If so, for MacPorts, we can add a dependency on a newer compiler. I was just surprised to find a change in minimum requirements in what based on the version number appeared to be just a bugfix release, and surprised not to find any mention of the changed requirement in the NEWS or BUILD.md files.

[behdad]

Can you try removing that -std=c++11 from configure.ac and see if it compiles? It should. We are not using any features, just nullptr and static_assert, and we have polyfill implementations for both.

[behdad]

But yeah, those are really old systems...

[ryandesign]

Can you try removing that -std=c++11 from configure.ac and see if it compiles?

Thanks for the suggestion! Without that flag, I get:

libtool: compile:  /Volumes/Data/macports/tiger/bin/g++-apple-4.2 -DHAVE_CONFIG_H -I. -I.. -pthread -I/Volumes/Data/macports/tiger/include/glib-2.0 -I/Volumes/Data/macports/tiger/lib/glib-2.0/include -I/Volumes/Data/macports/tiger/include/freetype2 -I/Volumes/Data/macports/tiger/include/libpng16 -I./hb-ucdn -I/Volumes/Data/macports/tiger/include -pipe -Os -arch ppc -fno-rtti -fno-exceptions -Wcast-align -fvisibility-inlines-hidden -MT libharfbuzz_la-hb-common.lo -MD -MP -MF .deps/libharfbuzz_la-hb-common.Tpo -c hb-common.cc  -fno-common -DPIC -o .libs/libharfbuzz_la-hb-common.o
hb-common.cc: In function '_xlocale* get_C_locale()':
hb-common.cc:727: error: invalid cast from type 'const<anonymous class>' to type 'int32_t'

[behdad]

Can you try with this patch as well:

diff --git a/src/hb-atomic-private.hh b/src/hb-atomic-private.hh
index 2d567579..afefe4d1 100644
--- a/src/hb-atomic-private.hh
+++ b/src/hb-atomic-private.hh
@@ -89,9 +89,9 @@ typedef int32_t hb_atomic_int_impl_t;
 #define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwapPtrBarrier ((void *) (O), (void *) (N), (void **) (P))
 #else
 #if __ppc64__ || __x86_64__ || __aarch64__
-#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap64Barrier ((int64_t) (O), (int64_t) (N), (int64_t*) (P))
+#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap64Barrier ((int64_t) (O), (int64_t) (void *) (N), (int64_t*) (P))
 #else
-#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap32Barrier ((int32_t) (O), (int32_t) (N), (int32_t*) (P))
+#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap32Barrier ((int32_t) (O), (int32_t) (void *) (N), (int32_t*) (P))
 #endif
 #endif
 

[ryandesign]

Can you try with this patch as well:

It looks like that didn't change the error message.

[behdad]

Actually, more like this:

diff --git a/src/hb-atomic-private.hh b/src/hb-atomic-private.hh
index 2d567579..93438401 100644
--- a/src/hb-atomic-private.hh
+++ b/src/hb-atomic-private.hh
@@ -89,9 +89,9 @@ typedef int32_t hb_atomic_int_impl_t;
 #define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwapPtrBarrier ((void *) (O), (void *) (N), (void **) (P))
 #else
 #if __ppc64__ || __x86_64__ || __aarch64__
-#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap64Barrier ((int64_t) (O), (int64_t) (N), (int64_t*) (P))
+#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap64Barrier ((int64_t) (void *) (O), (int64_t) (void *) (N), (int64_t*) (P))
 #else
-#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap32Barrier ((int32_t) (O), (int32_t) (N), (int32_t*) (P))
+#define hb_atomic_ptr_impl_cmpexch(P,O,N)      OSAtomicCompareAndSwap32Barrier ((int32_t) (void *) (O), (int32_t) (void *) (N), (int32_t*) (P))
 #endif
 #endif
 

[ryandesign]

Actually, more like this:

Ok, looks like that gets us further:

libtool: compile:  /Volumes/Data/macports/tiger/bin/g++-apple-4.2 -DHAVE_CONFIG_H -I. -I.. -pthread -I/Volumes/Data/macports/tiger/include/glib-2.0 -I/Volumes/Data/macports/tiger/lib/glib-2.0/include -I/Volumes/Data/macports/tiger/include/freetype2 -I/Volumes/Data/macports/tiger/include/libpng16 -I./hb-ucdn -I/Volumes/Data/macports/tiger/include -pipe -Os -arch ppc -fno-rtti -fno-exceptions -Wcast-align -fvisibility-inlines-hidden -MT libharfbuzz_la-hb-ot-shape-complex-thai.lo -MD -MP -MF .deps/libharfbuzz_la-hb-ot-shape-complex-thai.Tpo -c hb-ot-shape-complex-thai.cc  -fno-common -DPIC -o .libs/libharfbuzz_la-hb-ot-shape-complex-thai.o
hb-ot-shape-complex-thai.cc: In function 'hb_codepoint_t thai_pua_shape(hb_codepoint_t, thai_action_t, hb_font_t*)':
hb-ot-shape-complex-thai.cc:100: error: cannot convert 'const<anonymous class>' to 'const thai_pua_shape(hb_codepoint_t, thai_action_t, hb_font_t*)::thai_pua_mapping_t*' in initialization

[behdad]

Humm. No idea about that one. that looks wrong...
How much work is it for you to use a modern compiler on those settings? While I don't like using new language features, for C++, it's really limiting to have to work around implementation deficiencies of such old compilers.

[behdad]

Also, try with -std=c++0x, see if that works better.

[ryandesign]

I read the pdf where you got the nullptr polyfill. I see that in the drawbacks for that polyfill, it mentions misleading compiler diagnostics, which sounds like what I'm seeing.

We could easily have MacPorts use a newer clang compiler for Mac OS X 10.6 Snow Leopard and earlier on Intel Macs. It's trickier on PowerPC Macs because clang doesn't work well enough to be used on PowerPC. We would need to use a newer gcc instead, say gcc 6. The problem with that is that gcc ≥ 5.1 has a newer libstdc++ which is not compatible with the gcc 4.2.1 libstdc++ the OS comes with and which other software compiled under MacPorts is linked with. We can define _GLIBCXX_USE_CXX11_ABI=0 to tell gcc ≥ 5.1 to use the newer libstdc++ in a way that is supposed to be compatible with the libstdc++ from gcc < 5.1 but I'm not 100% certain if that compatibility extends all the way back to the 4.2.1 libstdc++. What we have sometimes seen in years past, when we tried to have some software built using a newer gcc 4.x and its newer libstdc++ exchange data with software built using gcc 4.2.1 and its libstdc++ is that the software would build but would crash at runtime when objects created with one of the libstdc++ libraries were passed to a library linked with the other libstdc++ library. I would prefer to avoid entering this uncertain territory.

If it were just a matter of telling users of older Mac OS versions to upgrade, that would be one thing, but many older Macs cannot be upgraded to newer OS versions. PowerPC Macs cannot upgrade past 10.5.x. 32-bit Intel Macs cannot go beyond 10.6.x. Telling these users to upgrade their OS is equivalent to telling them to buy a new computer. Some users may already have bought a new computer and are trying to repurpose their old one. Especially because these older Macs can't upgrade to newer OS versions, users may want to install MacPorts to use it to install newer software, and there's a good chance that what they will want to install depends on pango which depends on harfbuzz.

I tried just changing -std=c++11 to -std=c++0x; gcc 4.2.1 doesn't support that either:

cc1plus: error: unrecognized command line option "-std=c++0x"

It looks like support for the -std=c++0x flag debuted in gcc 4.3 and support for nullptr was added in gcc 4.6.

I tried replacing the polyfill with #define nullptr NULL. That's probably not ideal, but since in dbdbfe3 you just replaced NULL with nullptr, this #define just reverts older systems back to something they understand. It seems to work (well, it compiles, at least).

There were still a few instances of:

error: 'nullptr' was not declared in this scope

Adding #include "hb-private.hh" to src/test-buffer-serialize.cc, src/test-size-params.cc, src/test-would-substitute.cc, src/test.cc and util/ansi-print.cc fixed that.

This is the complete patch:

--- configure.orig	2017-10-26 13:51:42.000000000 -0500
+++ configure	2017-10-27 01:22:26.000000000 -0500
@@ -18251,9 +18251,6 @@
 	# Make symbols link locally
 	LDFLAGS="$LDFLAGS -Bsymbolic-functions"
 
-	# Choose C++ version
-	CXXFLAGS="$CXXFLAGS -std=c++11"
-
 	# Make sure we don't link to libstdc++
 	CXXFLAGS="$CXXFLAGS -fno-rtti -fno-exceptions"
 
--- src/hb-private.hh.orig	2017-10-25 17:17:35.000000000 -0500
+++ src/hb-private.hh	2017-10-27 04:52:43.000000000 -0500
@@ -94,7 +94,7 @@
 private:
     void operator&() const;    // whose address can't be taken
 } _hb_nullptr = {};            // and whose name is nullptr
-#define nullptr _hb_nullptr
+#define nullptr NULL
 
 // Static assertions
 #ifndef static_assert
--- src/test-buffer-serialize.cc.orig	2017-10-15 05:05:46.000000000 -0500
+++ src/test-buffer-serialize.cc	2017-10-27 04:56:06.000000000 -0500
@@ -28,6 +28,7 @@
 #include "config.h"
 #endif
 
+#include "hb-private.hh"
 #include "hb.h"
 #ifdef HAVE_FREETYPE
 #include "hb-ft.h"
--- src/test-size-params.cc.orig	2017-10-15 05:05:46.000000000 -0500
+++ src/test-size-params.cc	2017-10-27 04:56:28.000000000 -0500
@@ -28,6 +28,7 @@
 #include "config.h"
 #endif
 
+#include "hb-private.hh"
 #include "hb.h"
 #include "hb-ot.h"
 
--- src/test-would-substitute.cc.orig	2017-10-15 05:05:46.000000000 -0500
+++ src/test-would-substitute.cc	2017-10-27 04:56:45.000000000 -0500
@@ -28,6 +28,7 @@
 #include "config.h"
 #endif
 
+#include "hb-private.hh"
 #include "hb.h"
 #include "hb-ot.h"
 
--- src/test.cc.orig	2017-10-15 05:05:46.000000000 -0500
+++ src/test.cc	2017-10-27 04:55:40.000000000 -0500
@@ -28,6 +28,7 @@
 #include "config.h"
 #endif
 
+#include "hb-private.hh"
 #include "hb.h"
 
 #ifdef HAVE_GLIB
--- util/ansi-print.cc.orig	2017-10-15 05:09:41.000000000 -0500
+++ util/ansi-print.cc	2017-10-27 04:57:28.000000000 -0500
@@ -28,6 +28,7 @@
 #include "config.h"
 #endif
 
+#include "hb-private.hh"
 #include "ansi-print.hh"
 
 #include <assert.h>

However, on C++11-capable compilers, there are lots of warnings:

warning: 'nullptr' macro redefined [-Wmacro-redefined]

To fix this, you may need to check in the configure script whether -std=c++11 or -std=c++0x are supported and if so use them.

[behdad]

Thanks. I'll try to clean this up. What surprises me is that if make my build use the polyfill, it compiles fine.

[behdad]

Ok, code should be clean now. Just the configure check left to be done.