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
