Thanks!
> -----Original Message----- > From: Volker Simonis [mailto:volker.simo...@gmail.com] > Sent: Montag, 18. Dezember 2017 10:51 > To: Baesken, Matthias <matthias.baes...@sap.com> > Cc: Phil Race <philip.r...@oracle.com>; 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 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- > d...@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- > d...@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 > >
