Le lun. 6 juil. 2020 à 11:58, Werner LEMBERG <w...@gnu.org> a écrit :
>
> > Werner, please don't commit something if you think there are still
> > problems in it, that's what code reviews are,
>
> Normally, I do that. However, I've already invested a lot of time in
> writing ChangeLog entries...
>
> Would you consider dropping the ChangeLog entries. It is far simpler to
rely on the git history for this, and just enforcing that commiter provide
meaningful commit message should be enough?
Also, whenever you reformat some of the patches that are sent to you, the
original author has to resolve the conflicts manually, or drop its own
branch, which is not always practical (e.g. when there are other
work-in-progress commits/branches on top of the one that was originally
submitted). Would you consider using clang-format to automate the
formatting task? I think we can get pretty close to the FreeType formatting
standard with a Clang style sheet. It won't be 100% the same, but close
enough to avoid repetitive work.
> See attached patch for a fix.
>
> Applied, thanks. Note, however, that there are still issues with
> `make multi CC=c++` (I'm using g++ 7.5.0):
>
> Yes, here's another patch that tries to fix this. Sorry about that.
Also Ben Wagner noticed me that some third-party code is using FT_UNUSED(),
so I've moved it back to public-macros.h to avoid breaking this.
> c++ -ansi -pedantic \
> -Ifreetype2.compiled [...] \
> freetype2/src/base/ftsystem.c
> In file included from freetype2/include/freetype/config/ftconfig.h:45:0,
> from freetype2/src/base/ftsystem.c:29:
> freetype2/include/freetype/config/public-macros.h:83:61: error:
> expected unqualified-id before string constant
> #define FT_EXPORT( x ) FT_PUBLIC_FUNCTION_ATTRIBUTE extern "C" x
> ^
> freetype2/include/freetype/fterrors.h:281:3: note:
> in expansion of macro ‘FT_EXPORT’
> FT_EXPORT( const char* )
> ^~~~~~~~~
>
> > Sorry about that. We need a better way to automatically check our
> > builds. [...] I'll work on a proper rebuild-check script first
> > though...
>
> I don't worry about that right now, and please don't invest too much
> time here. This is something after a switch to a new build system
> IMHO.
>
>
> Werner
>
From 0cfacffe05ae6eeffe2df0d0663fa4666a8b6afc Mon Sep 17 00:00:00 2001
From: David Turner <david.turner....@gmail.com>
Date: Mon, 6 Jul 2020 10:56:36 +0200
Subject: [build] Fix multi and C++ builds.
The following builds were failing due to previous changes:
make multi
make multi CC="c++"
make CC="c++"
This patch fixes the issues, which were missing includes to
get the right macro definitions in multi-build mode.
Also, FT_UNUSED() is actually used by third-party code, so
move it back to <freetype/config/public-macros.h> to avoid
breaking it.
---
include/freetype/config/public-macros.h | 16 +++++++++++++---
include/freetype/internal/compiler-macros.h | 19 ++++---------------
src/cache/ftccback.h | 2 ++
src/lzw/ftzopen.h | 3 +++
4 files changed, 22 insertions(+), 18 deletions(-)
diff --git a/include/freetype/config/public-macros.h b/include/freetype/config/public-macros.h
index b1fa0f2b3..5cc3236ed 100644
--- a/include/freetype/config/public-macros.h
+++ b/include/freetype/config/public-macros.h
@@ -78,13 +78,23 @@ FT_BEGIN_HEADER
*
* FT_EXPORT( FT_Bool ) FT_Object_Method( FT_Object obj, ... );
*
+ * NOTE: This requires that all FT_EXPORT() uses are inside FT_BEGIN_HEADER ..
+ * FT_END_HEADER blocks. This guarantees that the functions are exported with
+ * C linkage, even when the header is included by a C++ source file.
*/
-#ifdef __cplusplus
-#define FT_EXPORT( x ) FT_PUBLIC_FUNCTION_ATTRIBUTE extern "C" x
-#else
#define FT_EXPORT( x ) FT_PUBLIC_FUNCTION_ATTRIBUTE extern x
+
+ /* `FT_UNUSED` indicates that a given parameter is not used -- */
+ /* this is only used to get rid of unpleasant compiler warnings. */
+ /* */
+ /* Technically, this was not meant to be part of the public API, */
+ /* but some third-party code depends on it. */
+ /* */
+#ifndef FT_UNUSED
+#define FT_UNUSED( arg ) ( (arg) = (arg) )
#endif
+
FT_END_HEADER
#endif /* FREETYPE_CONFIG_PUBLIC_MACROS_H_ */
diff --git a/include/freetype/internal/compiler-macros.h b/include/freetype/internal/compiler-macros.h
index 1f432bca5..b62c0777b 100644
--- a/include/freetype/internal/compiler-macros.h
+++ b/include/freetype/internal/compiler-macros.h
@@ -27,12 +27,6 @@ FT_BEGIN_HEADER
# if defined( _COMPILER_VERSION ) && ( _COMPILER_VERSION >= 730 )
# pragma set woff 3505
# endif
-#endif
-
- /* `FT_UNUSED` indicates that a given parameter is not used -- */
- /* this is only used to get rid of unpleasant compiler warnings. */
-#ifndef FT_UNUSED
-#define FT_UNUSED( arg ) ( (arg) = (arg) )
#endif
/* Fix compiler warning with sgi compiler. */
@@ -126,18 +120,13 @@ FT_BEGIN_HEADER
*
* FT_FUNCTION_DECLARATION(int) foo(int x);
*
- * NOTE: Technically, all FreeType headers put their function declarations
- * inside an extern "C" block, giving them C linkage. This means that using
- * this macro is only necessary within internal source files, but using it in
- * a header will be harmless.
+ * NOTE: This requires that all uses are inside FT_BEGIN_HEADER..FT_END_HEADER
+ * blocks. Which guarantees that the declarations have C linkage when the
+ * headers are included by C++ sources.
*
* NOTE: Do not use directly, use FT_LOCAL()/FT_BASE()/FT_EXPORT() instead.
*/
-#ifdef __cplusplus
-#define FT_FUNCTION_DECLARATION( x ) extern "C" x
-#else
#define FT_FUNCTION_DECLARATION( x ) extern x
-#endif
/* Same as FT_FUNCTION_DECLARATION(), but for function definitions instead.
* NOTE: Do not use directly, use FT_LOCAL_DEF()/FT_BASE_DEF()/FT_EXPORT_DEF()
@@ -171,7 +160,7 @@ FT_BEGIN_HEADER
* sub-directory, but are otherwise internal to the library.
*/
#define FT_LOCAL_ARRAY( x ) FT_INTERNAL_FUNCTION_ATTRIBUTE extern const x
-#define FT_LOCAL_ARRAY_DEF( x ) const x
+#define FT_LOCAL_ARRAY_DEF( x ) FT_FUNCTION_DEFINITION( const x )
/* Use FT_BASE()/FT_BASE_DEF() to declare or define an internal library
* function that are used by more than one single module.
diff --git a/src/cache/ftccback.h b/src/cache/ftccback.h
index 802fd4444..542acb156 100644
--- a/src/cache/ftccback.h
+++ b/src/cache/ftccback.h
@@ -25,6 +25,7 @@
#include "ftcglyph.h"
#include "ftcsbits.h"
+FT_BEGIN_HEADER
FT_LOCAL( void )
ftc_inode_free( FTC_Node inode,
@@ -84,6 +85,7 @@
ftc_node_destroy( FTC_Node node,
FTC_Manager manager );
+FT_END_HEADER
#endif /* FTCCBACK_H_ */
diff --git a/src/lzw/ftzopen.h b/src/lzw/ftzopen.h
index cb9206b3e..d8768f7b4 100644
--- a/src/lzw/ftzopen.h
+++ b/src/lzw/ftzopen.h
@@ -24,6 +24,7 @@
#include <freetype/freetype.h>
+FT_BEGIN_HEADER
/*
* This is a complete re-implementation of the LZW file reader,
@@ -165,6 +166,8 @@
/* */
+FT_END_HEADER
+
#endif /* FTZOPEN_H_ */
--
2.20.1
