Hi Christoph, I had a look at this change. The integration is good.
I thought about the adaptions you had to do. They look good for the compilers you mention, and I would assume they also work with more recent ones. I guess nobody uses more recent compilers on Solaris. On AIX I know further adaptions are needed, and as they are missing it is ruled out anybody compiles with xlc 16. So looks good, too. I checked the tests, and the only somewhat related failing one is java/awt/FontMetrics/MaxAdvanceIsMax.java, but that is also failing without your patch. So testing is fine. Best regards, Goetz. > -----Original Message----- > From: jdk-updates-dev <jdk-updates-dev-boun...@openjdk.java.net> On > Behalf Of Langer, Christoph > Sent: Dienstag, 30. April 2019 11:26 > To: jdk-updates-...@openjdk.java.net > Cc: 2d-dev <2d-dev@openjdk.java.net>; build-...@openjdk.java.net; Baesken, > Matthias <matthias.baes...@sap.com> > Subject: [CAUTION] [11u] RFR 8210782: Upgrade HarfBuzz to the latest 2.3.1 > > Hi, > > please help reviewing the backport of JDK-8210782: Upgrade HarfBuzz to the > latest 2.3.1. > > This has been backported to 11.0.4-oracle already. I took the large change > down to 11u-dev. It applies quite nicely, apart from a little issue in > make/lib/Awt2dLibraries.gmk: > > --- Awt2dLibraries.gmk > +++ Awt2dLibraries.gmk > @@ -613,8 +614,7 @@ > type-limits missing-field-initializers implicit-fallthrough \ > strict-aliasing undef unused-function, \ > DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor strict- > overflow \ > - maybe-uninitialized \ > - missing-attributes class-memaccess, \ > + maybe-uninitialized class-memaccess, \ > DISABLED_WARNINGS_clang := unused-value incompatible-pointer-types \ > tautological-constant-out-of-range-compare int-to-pointer-cast \ > sign-compare undef missing-field-initializers, \ > > The original change would remove the disabling of the "missing-attributes" > warnings, but since this warning type is not disabled in jdk11u-dev > currently, I > would just skip that diff. > > With that change, most platforms did build fine, except for Solaris (where we > use Oracle Studio 12u4 in jdk11u-dev vs Oracle Studio 12u6 in jdk/jdk) and AIX > (xlc12 vs. xlc16). > > To keep support for Oracle Studio 12u4 for Solaris, I had to remove the > warning > flag "refmemnoconstr_aggr" from line 622 (as opposed to the original change). > Seems that it is not yet supported in OS12u4. > > Furthermore, a code tweak had to be done (Thanks, Matthias, for your help > here): > > --- a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-subset-cff- > common.hh Fri Mar 01 16:59:19 2019 -0800 > +++ b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-subset-cff- > common.hh Mon Apr 29 16:26:41 2019 +0200 > @@ -280,6 +280,10 @@ > { > str_buff_t &flatStr; > bool drop_hints; > + > + // Solaris: OS12u4 complains about "A class with a reference member lacks a > user-defined constructor" > + // so provide the constructor > + flatten_param_t(str_buff_t& sbt, bool dh) : flatStr(sbt), drop_hints(dh) {} > }; > > template <typename ACC, typename ENV, typename OPSET> > @@ -305,7 +309,9 @@ > return false; > cs_interpreter_t<ENV, OPSET, flatten_param_t> interp; > interp.env.init (str, acc, fd); > - flatten_param_t param = { flat_charstrings[i], drop_hints }; > + // Solaris: OS12u4 does not like the C++11 style init > + // flatten_param_t param = { flat_charstrings[i], drop_hints }; > + flatten_param_t param(flat_charstrings[i], drop_hints); > if (unlikely (!interp.interpret (param))) > return false; > } > > For AIX, this tweak had to be added (credit goes to Matthias as well): > diff -r 2b3dbedfbfb9 > src/java.desktop/share/native/libfontmanager/harfbuzz/hb-null.hh > --- a/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-null.hh Fri > Mar 01 16:59:19 2019 -0800 > +++ b/src/java.desktop/share/native/libfontmanager/harfbuzz/hb-null.hh > Mon Apr 29 16:26:41 2019 +0200 > @@ -83,7 +83,9 @@ > > template <typename T, typename V, typename B> > struct _hb_assign > -{ static inline void value (T &o, const V v) { o = v; } }; > +// add cast to please AIX xlc12.1 > +//{ static inline void value (T &o, const V v) { o = v; } }; > +{ static inline void value (T &o, const V v) { o = (T&) v; } }; > template <typename T, typename V> > struct _hb_assign<T, V, _hb_bool_type<(bool) (1 + (unsigned int) T::min_size)> > > > { static inline void value (T &o, const V v) { o.set (v); } }; > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8210782 > Original Change: http://hg.openjdk.java.net/jdk/jdk/rev/7c11a7cc7c1d > Review discussion for jdk/jdk: https://mail.openjdk.java.net/pipermail/2d- > dev/2019-March/009914.html > New Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8210782.jdk11u/ > > Thanks & Best regards > Christoph