Thanks David! On Dec 19, 2016 21:43, "David Holmes" <david.hol...@oracle.com> wrote:
> > > On 18/12/2016 10:13 PM, Thomas Stüfe wrote: > >> HI all, >> >> after investigating I see a number of issues preventing gtest from >> running on AIX, so I decided to get the fix for the wrong assert in the >> hotspot out of the way and open follow up issues for the other problems. >> >> So, this is just the fixed assert, fixed in the fashion David suggested. >> All other issues I will fix separately later. >> >> http://cr.openjdk.java.net/~stuefe/webrevs/8171225-aix-fix- >> gtest-assert-compile-error/webrev.01 >> > > Looks good. > > Thanks, > David > > (Hope the link is correct, the website seems down at the moment). >> >> Thank you for reviewing, >> >> Thomas >> >> >> On Sun, Dec 18, 2016 at 9:23 AM, Thomas Stüfe <thomas.stu...@gmail.com >> <mailto:thomas.stu...@gmail.com>> wrote: >> >> Hi David, >> >> thanks for your review, your suggestion makes sense. Will >> incorporate it into the next webrev; but first I have to try and >> meet Volkers linker demands :) >> >> Thanks, Thomas >> >> On Thu, Dec 15, 2016 at 11:33 AM, David Holmes >> <david.hol...@oracle.com <mailto:david.hol...@oracle.com>> wrote: >> >> Hi Thomas, >> >> On 15/12/2016 4:43 PM, Thomas Stüfe wrote: >> >> Hi all, >> >> please review this small change. It fixes the gtest build on >> AIX and >> enables it by default. >> >> Note that even though this is a fix for AIX, a cast needed >> to be added to >> shared test coding. This is because xlC struggles with >> certain template >> expansions and I had to help it by providing an explicit cast. >> >> >> These kind of problems have been reported in the past. The way >> we chose to address them was to convert to use ASSERT_TRUE(i == >> NULL) rather than apply casts to make the ASSERT_EQ(i, NULL) >> compile. >> >> Thanks, >> David >> >> >> Because linker options were changed as well, this >> unfortunately this >> spreads over two forest parts, so two webrevs were needed. >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8171225 >> <https://bugs.openjdk.java.net/browse/JDK-8171225> >> Webrevs: >> (hotspot) >> http://cr.openjdk.java.net/~stuefe/webrevs/8171225-aix-build >> -gtests/webrev.00/webrev/ >> <http://cr.openjdk.java.net/~stuefe/webrevs/8171225-aix-buil >> d-gtests/webrev.00/webrev/> >> (top level) >> http://cr.openjdk.java.net/~stuefe/webrevs/8171225-aix-build >> -gtests/toplevel-webrev.00/webrev/ >> <http://cr.openjdk.java.net/~stuefe/webrevs/8171225-aix-buil >> d-gtests/toplevel-webrev.00/webrev/> >> >> Note that the toplevel change contains the newly generated >> configure.sh. I >> was not sure if that was needed, but it is included for >> convenience. >> >> Kind Regards, Thomas >> >> >> >>
