RFR(S): 8164738: Convert AltHashing_test to GTest
Coleen Phillimore
coleen.phillimore at oracle.com
Mon Aug 29 20:44:30 UTC 2016
This test conversion looks good.
On 8/29/16 11:06 AM, Kirill Zhaldybin wrote:
> David,
>
> Thank you for review!
>
>
> On 26.08.2016 06:38, David Holmes wrote:
>> On 25/08/2016 1:47 AM, Kirill Zhaldybin wrote:
>>> Dear all,
>>>
>>> Could you please review this fix for 8164738?
>>
>> Seems okay.
>>
>>> To convert the test I added new friend class to AltHashing class so we
>>> could access private member function static juint murmur3_32(const int*
>>> data, int len). There are also few formating fixes.
>>
>> Any reason all the murmur functions shouldn't be public? I'm not a
>> fan of friends. No big deal either way.
> Well, I am not an author so I could only speculate that if
> static juint murmur3_32(const int* data, int len);
> static juint murmur3_32(juint seed, const int* data, int len);
>
> are used only from AltHashing class according to general "the less
> visible the better" rule they were made private.
>
Yes, that's why the functions are private. In general, the tests should
probably be made friends if they're going to use private functions
rather than making the functions public for the rest of the JVM to use.
thanks,
Coleen
> Regards, Kirill
>>
>> Thanks,
>> David
>>
>>>
>>> WebRev:
>>> http://cr.openjdk.java.net/~kzhaldyb/webrevs/JDK-8164738/webrev.00/
>>> CR: https://bugs.openjdk.java.net/browse/JDK-8164738
>>>
>>> Regards, Kirill
>
More information about the hotspot-runtime-dev
mailing list