Bug 222270 - java/openjfx8-devel: fails to build with ICU 59.1
Summary: java/openjfx8-devel: fails to build with ICU 59.1
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Tobias Kortkamp
URL:
Keywords: needs-patch
Depends on:
Blocks: 218788
  Show dependency treegraph
 
Reported: 2017-09-12 21:36 UTC by Jan Beich
Modified: 2017-09-21 17:56 UTC (History)
0 users

See Also:
tobik: maintainer-feedback+


Attachments
partial fix (6.49 KB, patch)
2017-09-12 21:45 UTC, Jan Beich
no flags Details | Diff
openjfx8.diff (3.45 KB, patch)
2017-09-17 22:26 UTC, Tobias Kortkamp
tobik: maintainer-approval+
Details | Diff
openjfx8.diff (3.42 KB, patch)
2017-09-18 05:28 UTC, Tobias Kortkamp
tobik: maintainer-approval+
Details | Diff
openjfx8.diff (3.75 KB, patch)
2017-09-20 16:38 UTC, Tobias Kortkamp
tobik: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Beich freebsd_committer 2017-09-12 21:36:45 UTC
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();
           ^~~~~~~~~~~~~~~~~~~~

build log: http://sprunge.us/aJiW
Comment 1 Jan Beich freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 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 freebsd_committer 2017-09-21 17:56:48 UTC
(In reply to Jan Beich from comment #7)
Committed, thanks!