Skip to content

Commit

Permalink
Fixed bug #76153 Intl compilation fails with icu4c 61.1
Browse files Browse the repository at this point in the history
Additionally, ICU >= 59.1 requires C++11, so add the flags. Some
refactoring is needed to comply with the latest recommended build
options, such as automatic icu namespace addition.
  • Loading branch information
weltling committed Mar 28, 2018
1 parent ac4d9fd commit 710284c
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 2 deletions.
3 changes: 3 additions & 0 deletions acinclude.m4
Expand Up @@ -2248,6 +2248,9 @@ AC_DEFUN([PHP_SETUP_ICU],[
ICU_LIBS=`$ICU_CONFIG --ldflags --ldflags-icuio`
PHP_EVAL_INCLINE($ICU_INCS)
PHP_EVAL_LIBLINE($ICU_LIBS, $1)
ICU_EXTRA_FLAGS=`$ICU_CONFIG --cxxflags`
ICU_EXTRA_FLAGS="$ICU_EXTRA_FLAGS -DU_USING_ICU_NAMESPACE=1"

This comment has been minimized.

Copy link
@srl295

srl295 Mar 28, 2018

@weltling please fix the source to use the icu:: namespace. See the docs:

Namespace (ICU 61 and later): Since ICU 61, call sites need to qualify ICU types explicitly, for example icu::UnicodeString, or do using icu::UnicodeString; where appropriate. If your code relies on the "using namespace icu;" that used to be in unicode/uversion.h, then you need to update your code.

This comment has been minimized.

Copy link
@weltling

weltling Mar 28, 2018

Author Contributor

Thanks for checking. Clear, a proper fix should be done, likely for master. After 57.1 more and more breaches are introduced, some refactoring is also due for better C++11 support, etc.

Thanks.

This comment has been minimized.

Copy link
@srl295

srl295 Mar 29, 2018

Sure! Also added the same commen at https://bugs.php.net/bug.php?id=76153

fi
])

Expand Down
2 changes: 1 addition & 1 deletion ext/intl/config.m4
Expand Up @@ -86,7 +86,7 @@ if test "$PHP_INTL" != "no"; then
breakiterator/codepointiterator_methods.cpp \
uchar/uchar.c \
idn/idn.c \
$icu_spoof_src, $ext_shared,,$ICU_INCS -Wno-write-strings -D__STDC_LIMIT_MACROS -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1,cxx)
$icu_spoof_src, $ext_shared,,$ICU_INCS -Wno-write-strings -D__STDC_LIMIT_MACROS -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 $ICU_EXTRA_FLAGS,cxx)
PHP_ADD_BUILD_DIR($ext_builddir/collator)
PHP_ADD_BUILD_DIR($ext_builddir/converter)
PHP_ADD_BUILD_DIR($ext_builddir/common)
Expand Down
2 changes: 1 addition & 1 deletion ext/intl/config.w32
Expand Up @@ -130,7 +130,7 @@ if (PHP_INTL != "no") {
ADD_FLAG("LIBS_INTL", "iculx.lib");
}

ADD_FLAG("CFLAGS_INTL", "/EHsc");
ADD_FLAG("CFLAGS_INTL", "/EHsc /D U_USING_ICU_NAMESPACE=1");
AC_DEFINE("HAVE_INTL", 1, "Internationalization support enabled");
} else {
WARNING("intl not enabled; libraries and/or headers not found");
Expand Down

1 comment on commit 710284c

@ilovezfs
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srl295 as you feared, it looks like some upstreams are not using the namespace in response to this change.

Please sign in to comment.