char16_t should not be used when C++11 isn't available

Description

When C code includes the ICU headers, the UChar type is defined to be uint16_t. But when C++ code includes the headers, UChar is char16_t even when U_SHOW_CPLUSPLUS_API has been set to 0. Apart from arguably being an inconsistency in the API, this means that C++98 code can't use the C API even though C99 code can.

I propose changing unicode/umachine.h to check not only __cplusplus but also U_SHOW_CPLUSPLUS_API when deciding how to typedef UChar. Since unicode/utypes.h includes unicode/umachine.h before setting a default value for U_SHOW_CPLUSPLUS_API, the check needs to take into account that U_SHOW_CPLUSPLUS_API may be undefined rather than simply looking at its value.

The motivation for this is that many C++98 projects use libxml2, which only has a C API, but uses ICU. Due to this, libxml/encoding.h includes unicode/ucnv.h which ends up including the ICU API into code that wasn't trying to use it. Defining U_SHOW_CPLUSPLUS_API=0 almost avoids compilation failure due to C++11 incompatibility, except for this one use of char16_t (which of course doesn't exist prior to C++11/C11).

Activity

Show:
Jeff Genovy
December 12, 2019, 2:42 AM

Resolve this as fixed with PR 924.
https://github.com/unicode-org/icu/pull/924

Thank you for reporting this issue and for the PR! 😊

Jeff Genovy
December 11, 2019, 6:41 PM

Discussed in the ICU-TC call on 2019-12-11, and accepted for ICU 67.1

Thanks for updating the PR!

(Note for posterity: This doesn’t change the behavior for C, only for C++98/03.)

Jeff Genovy
December 5, 2019, 7:09 PM

FYI: Changing the title of the bug from:
“char16_t should not be used when C++ API is hidden”
to
“char16_t should not be used when C++11 isn't available“.

Jeff Genovy
December 5, 2019, 6:56 PM

Thanks Markus!

I think that your suggestion would be much better/cleaner and would also help keep the usage of non-standard built-in _MSVC_LANG to just within the unicode/platform.h file.

As for updating your PR: I suppose that’s up to you.
I don’t mind creating a PR for this once the ticket itself is triaged and accepted.

I think that we’d want to do something like this:

 

Markus Scherer
December 5, 2019, 4:27 PM

Instead of raw {{__cplusplus}}, let’s use {{U_CPLUSPLUS_VERSION}} (unicode/platform.h). Its value is 1 for C++98 and C++03, and it already has logic for {{_MSVC_LANG}}.

Fixed
Your pinned fields
Click on the next to a field label to start pinning.

Assignee

Jeff Genovy

Reporter

Joshua Root

Components

Reviewer

Markus Scherer

Priority

medium

Fix versions