FYI: my patch has been accepted and pushed upstream into Harfbuzz: Closed #655 https://github.com/harfbuzz/harfbuzz/issues/655 via #656 https://github.com/harfbuzz/harfbuzz/pull/656
On Tue, Dec 19, 2017 at 11:43 AM, Volker Simonis <volker.simo...@gmail.com> wrote: > Thanks Phil! > > I've just pushed this to jdk/jdk10. > > I've also opened an issue [1] in the upstream Harfbuzz project and > submitted a pull request with the fix [2]. > > So hopefully this won't be an issue with the next integration any more. > > Regards, > Volker > > [1] https://github.com/harfbuzz/harfbuzz/issues/655 > [2] https://github.com/harfbuzz/harfbuzz/pull/656 > > On Mon, Dec 18, 2017 at 6:24 PM, Philip Race <philip.r...@oracle.com> wrote: >> +1 from me .. >> >> yes push to jdk/jdk10. >> >> -phil. >> >> >> On 12/18/17, 1:51 AM, Volker Simonis wrote: >>> >>> Hi Matthias, >>> >>> the change looks good and I can sponsor it. I'd just propose to >>> slightly change the comment to "..by the overloaded versions of 'cmp' >>> in IntType" if you don't mind. There's no need for a new webrew tough >>> - I can make that change before pushing. >>> >>> I'm just waiting for one more review from the 2D group. Afterwards >>> I'll push directly to jdk/jdk10. From my understanding, the fix will >>> than be automatically forward-integrated into jdk/jdk. >>> >>> Regards, >>> Volker >>> >>> >>> On Fri, Dec 15, 2017 at 2:24 PM, Baesken, Matthias >>> <matthias.baes...@sap.com> wrote: >>>> >>>> Hello, I created a second webrev : >>>> >>>> >>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8193515.1/ >>>> >>>> >>>>>> Whatever we do, can it be wrapped in an appropriate #ifdef AIX so that >>>>>> other platforms are guaranteed to be unaffected ? >>>> >>>> The new webrev uses the simpler cast-version proposed by Volki , and >>>> guards the AIX change by ifdef (suggested by Phil). >>>> >>>>>>> In the meantime we try to find out how latest xlC version 13 behaves . >>>> >>>> In the meantime I checked with XLC13 as well , but we see the error >>>> with XLC 13 too (same as XLC 12). >>>> >>>> Please review the change . >>>> >>>> It should go later into jdk10 as well (because jdk10 contains the new >>>> Harfbuzz 1.7.1 too ). >>>> >>>> >>>> Thanks, Matthias >>>> >>>> >>>>> -----Original Message----- >>>>> From: Baesken, Matthias >>>>> Sent: Donnerstag, 14. Dezember 2017 18:03 >>>>> To: 'Phil Race'<philip.r...@oracle.com>; Volker Simonis >>>>> <volker.simo...@gmail.com> >>>>> Cc: Simonis, Volker<volker.simo...@sap.com>; 2d-dev@openjdk.java.net >>>>> Subject: RE: [OpenJDK 2D-Dev] RFR : 8193515 : AIX : new Harfbuzz 1.7.1 >>>>> version fails to compile >>>>> >>>>> Hi Phil, hi Volki, >>>>> I think wrapping Volkis version into #ifdef _AIX >>>>> plus adding a small comment sounds like a good idea . >>>>> >>>>> In the meantime we try to find out how latest xlC version 13 behaves . >>>>> >>>>> Best regards, Matthias >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Phil Race [mailto:philip.r...@oracle.com] >>>>>> Sent: Donnerstag, 14. Dezember 2017 17:53 >>>>>> To: Volker Simonis<volker.simo...@gmail.com>; Baesken, Matthias >>>>>> <matthias.baes...@sap.com> >>>>>> Cc: Simonis, Volker<volker.simo...@sap.com>; 2d-dev@openjdk.java.net >>>>>> Subject: Re: [OpenJDK 2D-Dev] RFR : 8193515 : AIX : new Harfbuzz 1.7.1 >>>>>> version fails to compile >>>>>> >>>>>> Whatever we do, can it be wrapped in an appropriate #ifdef AIX so that >>>>>> other platforms are guaranteed to be unaffected ? >>>>>> >>>>>> For upstream you can report it at github >>>>>> https://github.com/harfbuzz/harfbuzz >>>>>> and see how Behdad would like to handle it. >>>>>> >>>>>> I expect he would want it removed once the compiler bug is fixed. >>>>>> >>>>>> -pgil. >>>>>> >>>>>> On 12/14/2017 08:13 AM, Volker Simonis wrote: >>>>>>> >>>>>>> Hi Matthias, >>>>>>> >>>>>>> thanks for addressing this issue! >>>>>>> >>>>>>> I'm pretty sure that his is a compiler bug and I have a short >>>>>>> reproducer which I'll send to IBM. >>>>>>> >>>>>>> The problem is that xlC can't distinguish a static member function >>>>>>> (which uses an ordinary function call) from a non-static member >>>>>>> function (which uses a member function call) with the same name. >>>>>>> >>>>>>> I've just found a slightly more elegant (and less intrusive) fix. We >>>>>>> can help the compiler to find the correct method by simply casting it >>>>>>> to the correct version: >>>>>>> >>>>>>> diff -r be065f758154 >>>>>>> src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot-shape- >>>>>> >>>>>> complex-arabic-fallback.hh >>>>>>> >>>>>>> --- a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh >>>>>>> >>>>>>> Thu Dec 14 12:49:47 2017 +0530 >>>>>>> +++ b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh >>>>>>> >>>>>>> Thu Dec 14 17:11:49 2017 +0100 >>>>>>> @@ -77,7 +77,7 @@ >>>>>>> >>>>>>> /* Bubble-sort or something equally good! >>>>>>> * May not be good-enough for presidential candidate interviews, >>>>>>> but good-enough for us... */ >>>>>>> - hb_stable_sort (&glyphs[0], num_glyphs, OT::GlyphID::cmp, >>>>>> >>>>>> &substitutes[0]); >>>>>>> >>>>>>> + hb_stable_sort (&glyphs[0], num_glyphs, (int(*)(const OT::GlyphID >>>>>>> *, const OT::GlyphID *)) OT::GlyphID::cmp,&substitutes[0]); >>>>>>> >>>>>>> OT::Supplier<OT::GlyphID> glyphs_supplier (glyphs, >>>>>>> num_glyphs); >>>>>>> OT::Supplier<OT::GlyphID> substitutes_supplier (substitutes, >>>>>> >>>>>> num_glyphs); >>>>>>> >>>>>>> @@ -126,7 +126,7 @@ >>>>>>> first_glyphs_indirection[num_first_glyphs] = first_glyph_idx; >>>>>>> num_first_glyphs++; >>>>>>> } >>>>>>> - hb_stable_sort (&first_glyphs[0], num_first_glyphs, >>>>>>> OT::GlyphID::cmp,&first_glyphs_indirection[0]); >>>>>>> >>>>>>> + hb_stable_sort (&first_glyphs[0], num_first_glyphs, (int(*)(const >>>>>>> OT::GlyphID *, const OT::GlyphID *)) OT::GlyphID::cmp, >>>>>>> &first_glyphs_indirection[0]); >>>>>>> >>>>>>> /* Now that the first-glyphs are sorted, walk again, populate >>>>>>> ligatures. >>>>> >>>>> */ >>>>>>> >>>>>>> for (unsigned int i = 0; i< num_first_glyphs; i++) >>>>>>> >>>>>>> I'll also try to bring this change upstream into Harfbuzz, but we need >>>>>>> to push this into the new jdk/jdk10 repo because there will be no more >>>>>>> HarfBuzz integration for Java 10. >>>>>>> >>>>>>> Regards, >>>>>>> Volker >>>>>>> >>>>>>> >>>>>>> On Thu, Dec 14, 2017 at 4:10 PM, Baesken, Matthias >>>>>>> <matthias.baes...@sap.com> wrote: >>>>>>>> >>>>>>>> Hello, after upgrading to new Harfbuzz 1.7.1 the openjdk build >>>>>>>> fails on >>>>>>>> AIX. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> I created the following bug : >>>>>>>> >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8193515 >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> The compile error we get on AIX (using XLC 12.1) is : >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> === Output from failing command(s) repeated here === >>>>>>>> >>>>>>>> * For target >>>>>>>> support_native_java.desktop_libfontmanager_hb-ot-shape-complex- >>>>>> >>>>>> arabic.o: >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh", >>>>>>>> >>>>>>>> line 80.3: 1540-0218 (S) The call does not match any parameter list >>>>>>>> for >>>>>>>> "hb_stable_sort". >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb- >>>>>> >>>>>> private.hh", >>>>>>>> >>>>>>>> line 723.1: 1540-1283 (I) "template<class T, class T2> >>>>>>>> hb_stable_sort(T *, >>>>>>>> unsigned int, int (*)(const T *, const T *), T2 *)" is not a viable >>>>>>>> candidate. >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh", >>>>>>>> >>>>>>>> line 80.43: 1540-0298 (I) Template argument deduction cannot be >>>>>> >>>>>> performed >>>>>>>> >>>>>>>> using the function "template int cmp(Type2) const". >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb- >>>>>> >>>>>> private.hh", >>>>>>>> >>>>>>>> line 748.1: 1540-1283 (I) "template<class T> hb_stable_sort(T *, >>>>> >>>>> unsigned >>>>>>>> >>>>>>>> int, int (*)(const T *, const T *))" is not a viable candidate. >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh", >>>>>>>> >>>>>>>> line 80.3: 1540-0215 (I) The wrong number of arguments has been >>>>>> >>>>>> specified >>>>>>>> >>>>>>>> for "template<class T> hb_stable_sort(T *, unsigned int, int >>>>>>>> (*)(const T >>>>> >>>>> *, >>>>>>>> >>>>>>>> const T *))". >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh", >>>>>>>> >>>>>>>> line 129.3: 1540-0218 (S) The call does not match any parameter list >>>>>>>> for >>>>>>>> "hb_stable_sort". >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb- >>>>>> >>>>>> private.hh", >>>>>>>> >>>>>>>> line 723.1: 1540-1283 (I) "template<class T, class T2> >>>>>>>> hb_stable_sort(T *, >>>>>>>> unsigned int, int (*)(const T *, const T *), T2 *)" is not a viable >>>>>>>> candidate. >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh", >>>>>>>> >>>>>>>> line 129.55: 1540-0298 (I) Template argument deduction cannot be >>>>>> >>>>>> performed >>>>>>>> >>>>>>>> using the function "template int cmp(Type2) const". >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb- >>>>>> >>>>>> private.hh", >>>>>>>> >>>>>>>> line 748.1: 1540-1283 (I) "template<class T> hb_stable_sort(T *, >>>>> >>>>> unsigned >>>>>>>> >>>>>>>> int, int (*)(const T *, const T *))" is not a viable candidate. >>>>>>>> >>>>>>>> " >>>>>>>> /jdk/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-ot- >>>>>> >>>>>> shape-complex-arabic-fallback.hh", >>>>>>>> >>>>>>>> line 129.3: 1540-0215 (I) The wrong number of arguments has been >>>>>> >>>>>> specified >>>>>>>> >>>>>>>> for "template<class T> hb_stable_sort(T *, unsigned int, int >>>>>>>> (*)(const T >>>>> >>>>> *, >>>>>>>> >>>>>>>> const T *))". >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> The compilation “complains” about the hb_stable_sort template used >>>>> >>>>> in >>>>>>>> >>>>>>>> hb-ot-shape-complex-arabic-fallback.hh . After looking a bit into >>>>>>>> this , >>>>>>>> the third parameter >>>>>>>> >>>>>>>> OT::GlyphID::cmp >>>>>>>> >>>>>>>> of >>>>>>>> >>>>>>>> hb_stable_sort (&glyphs[0], num_glyphs, OT::GlyphID::cmp, >>>>>>>> &substitutes[0]); >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> seems to trigger this XLC 12 issue . >>>>>>>> >>>>>>>> XLC 12 does not like the fact that we have two cmp functions (one a >>>>>>>> template) in >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> hb-open-type-private.hh : >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 610 template<typename Type, unsigned int Size> >>>>>>>> >>>>>>>> 611 struct IntType >>>>>>>> >>>>>>>> 612 { >>>>>>>> >>>>>>>> .... >>>>>>>> >>>>>>>> 617 static inline int cmp (const IntType<Type,Size> *a, const >>>>>>>> IntType<Type,Size> *b) { return b->cmp (*a); } >>>>>>>> >>>>>>>> 622 >>>>>>>> >>>>>>>> 623 template<typename Type2> >>>>>>>> >>>>>>>> 624 inline int cmp (Type2 a) const >>>>>>>> >>>>>>>> 625 { >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> ( GlyphID is an IntType ) >>>>>>>> >>>>>>>> This looks like an XLC bug, however it is pretty easy to workaround >>>>>>>> it by >>>>>>>> using a helper compare-function with a unique name (issue with cmp >>>>>>>> is >>>>>> >>>>>> that >>>>>>>> >>>>>>>> it is not unique, that confuses XLC ). >>>>>>>> >>>>>>>> See this webrev : >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8193515/ >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Please review it. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Thanks, Matthias
