Bug 222270

Comment 1 Jan Beich 2017-09-12 21:45:09 UTC

Created attachment 186310 [details]
partial fix

Not enough to unbreak:

In file included from modules/web/src/main/native/Source/WebCore/dom/Document.cpp:193:
modules/web/src/main/native/Source/WTF/wtf/unicode/java/UnicodeJava.h:21:18: error: typedef redefinition with different types ('uint16_t' (aka 'unsigned short') vs 'char16_t')
typedef uint16_t UChar;
                 ^
/usr/local/include/unicode/umachine.h:347:22: note: previous definition is here
    typedef char16_t UChar;
                     ^

Comment 2 Tobias Kortkamp 2017-09-17 22:26:55 UTC

Created attachment 186490 [details]
openjfx8.diff

(In reply to Jan Beich from comment #1)
It builds fine when we redefine UChar as char16_t in UnicodeJava.h as well.

Comment 3 Jan Beich 2017-09-17 23:49:18 UTC

Comment on attachment 186490 [details]
openjfx8.diff

> -typedef uint16_t UChar;
> +typedef char16_t UChar;

Not compatible with ICU 58. Have you tried to move UChar definition under `#ifndef __UMACHINE_H__` (similar to UChar32)?

modules/web/src/main/native/Source/WTF/wtf/unicode/java/UnicodeJava.h:21:18: error: typedef redefinition with different types ('char16_t' vs 'unsigned short')
typedef char16_t UChar;
                 ^
/usr/local/include/unicode/umachine.h:335:29: note: previous definition is here
    typedef __CHAR16_TYPE__ UChar;
                            ^

Comment 4 Tobias Kortkamp 2017-09-18 05:28:39 UTC

Created attachment 186500 [details]
openjfx8.diff

Updated the patch to remove the typedef from UnicodeJava.h entirely.  It doesn't
appear to be needed and the port still builds fine with both ICU 58 and 59.

Comment 5 Jan Beich 2017-09-18 19:54:30 UTC

Comment on attachment 186500 [details]
openjfx8.diff

> + #if PLATFORM(JAVA) && OS(WINDOWS)
> + typedef wchar_t UChar;
> +-#else
> +-typedef uint16_t UChar;
> + #endif

Better nuke or ifdef out the whole conditional. unicode/umachine.h no longer defines UChar via wchar_t on U_SIZEOF_WCHAR_T==2 platforms (e.g., Windows).

  // ICU 58
  #if defined(UCHAR_TYPE)
      typedef UCHAR_TYPE UChar;
  #elif U_SIZEOF_WCHAR_T==2
      typedef wchar_t UChar;
  #elif defined(__CHAR16_TYPE__)
      typedef __CHAR16_TYPE__ UChar;
  #else
      typedef uint16_t UChar;
  #endif

vs.

  // ICU 59
  #if defined(U_COMBINED_IMPLEMENTATION) || defined(U_COMMON_IMPLEMENTATION) || \
	  defined(U_I18N_IMPLEMENTATION) || defined(U_IO_IMPLEMENTATION)
      typedef char16_t UChar;
  #elif defined(UCHAR_TYPE)
      typedef UCHAR_TYPE UChar;
  #elif defined(__cplusplus)
      typedef char16_t UChar;
  #else
      typedef uint16_t UChar;
  #endif

Comment 6 Tobias Kortkamp 2017-09-20 16:38:18 UTC

Created attachment 186570 [details]
openjfx8.diff

Nuked all typedefs and the redefinition of U_MASK as well.

Comment 7 Jan Beich 2017-09-21 17:27:25 UTC

Comment on attachment 186570 [details]
openjfx8.diff

Looks OK. I've only tested with ICU 59. Can you land it?

Comment 8 commit-hook 2017-09-21 17:56:16 UTC

A commit references this bug:

Author: tobik
Date: Thu Sep 21 17:55:30 UTC 2017
New revision: 450296
URL: https://svnweb.freebsd.org/changeset/ports/450296

Log:
  java/openjfx8-devel: Unbreak build with ICU 59.1

  modules/web/src/main/native/Source/JavaScriptCore/API/JSStringRef.cpp:40:13: error: no matching function for call to 'create'
      return &OpaqueJSString::create(chars, numChars).leakRef();
              ^~~~~~~~~~~~~~~~~~~~~~
  modules/web/src/main/native/Source/JavaScriptCore/API/OpaqueJSString.h:48:32: note: candidate function not viable: no known conversion from 'const JSChar *' (aka 'const unsigned short *') to 'const LChar *' (aka 'const unsigned char *') for 1st argument
      static Ref<OpaqueJSString> create(const LChar* characters, unsigned length)
                                 ^
  modules/web/src/main/native/Source/JavaScriptCore/API/OpaqueJSString.h:53:32: note: candidate function not viable: no known conversion from 'const JSChar *' (aka 'const unsigned short *') to 'const UChar *' (aka 'const char16_t *') for 1st argument
      static Ref<OpaqueJSString> create(const UChar* characters, unsigned length)
                                 ^
  modules/web/src/main/native/Source/JavaScriptCore/API/OpaqueJSString.h:43:32: note: candidate function not viable: requires 0 arguments, but 2 were provided
      static Ref<OpaqueJSString> create()
                                 ^
  modules/web/src/main/native/Source/JavaScriptCore/API/OpaqueJSString.h:58:53: note: candidate function not viable: requires 1 argument, but 2 were provided
      JS_EXPORT_PRIVATE static RefPtr<OpaqueJSString> create(const String&);
                                                      ^
  modules/web/src/main/native/Source/JavaScriptCore/API/JSStringRef.cpp:65:35: error: no matching function for call to 'createWithoutCopying'
      return OpaqueJSString::create(StringImpl::createWithoutCopying(chars, numChars)).leakRef();
                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  modules/web/src/main/native/Source/WTF/wtf/text/StringImpl.h:385:50: note: candidate function not viable: no known conversion from 'const JSChar *' (aka 'const unsigned short *') to 'const UChar *' (aka 'const char16_t *') for 1st argument
      WTF_EXPORT_STRING_API static Ref<StringImpl> createWithoutCopying(const UChar* characters, unsigned length);
                                                   ^
  modules/web/src/main/native/Source/WTF/wtf/text/StringImpl.h:386:50: note: candidate function not viable: no known conversion from 'const JSChar *' (aka 'const unsigned short *') to 'const LChar *' (aka 'const unsigned char *') for 1st argument
      WTF_EXPORT_STRING_API static Ref<StringImpl> createWithoutCopying(const LChar* characters, unsigned length);
                                                   ^
  modules/web/src/main/native/Source/JavaScriptCore/API/JSStringRef.cpp:90:12: error: cannot initialize return object of type 'const JSChar *' (aka 'const unsigned short *') with an rvalue of type 'const UChar *' (aka 'const char16_t *')
      return string->characters();
             ^~~~~~~~~~~~~~~~~~~~
  modules/web/src/main/native/Source/WTF/wtf/unicode/java/UnicodeJava.h:21:18: error: typedef redefinition with different types ('uint16_t' (aka 'unsigned short') vs 'char16_t')
  typedef uint16_t UChar;
                   ^
  /usr/local/include/unicode/umachine.h:347:22: note: previous definition is here
      typedef char16_t UChar;

  PR:		218788, 222270
  Submitted by:	jbeich
  Reviewed by:	jbeich
  Obtained from:	WebKit (rebased)

Changes:
  head/java/openjfx8-devel/Makefile
  head/java/openjfx8-devel/files/patch-icu59

Comment 9 Tobias Kortkamp 2017-09-21 17:56:48 UTC

(In reply to Jan Beich from comment #7)
Committed, thanks!