Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v4]
On Wed, 28 Jun 2023 16:15:39 GMT, Ioi Lam wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Use pointer_delta_as_int for the name that uses pointer_delta, fix
>> negative case to just do checked_cast.
>
> src/hotspot/share/utilities/globalDefinitions.hpp line 529:
>
>> 527: template
>> 528: inline int pointer_delta_as_int(const volatile T* left, const volatile
>> T* right) {
>> 529: return checked_cast(pointer_delta(left, right, sizeof(T)));
>
> For clarity, I think you should add a comment saying the returned value is
> always non-negative.
done, thanks!
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14675#discussion_r1245681394
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v4]
On Wed, 28 Jun 2023 13:17:21 GMT, Coleen Phillimore wrote: >> This is another version of PR https://github.com/openjdk/jdk/pull/14659 but >> I've added a pointer delta function in globalDefinitions.hpp to use for >> these pointer diff calculations that return int everywhere. If the name is >> agreeable, I'll fix the other cases of this like this. It's better than raw >> casts. >> Tested with tier1-4. > > Coleen Phillimore has updated the pull request incrementally with one > additional commit since the last revision: > > Use pointer_delta_as_int for the name that uses pointer_delta, fix negative > case to just do checked_cast. Thanks Ioi and Fred and Stefan for suggestions. - PR Comment: https://git.openjdk.org/jdk/pull/14675#issuecomment-1612003549
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v4]
On Wed, 28 Jun 2023 13:17:21 GMT, Coleen Phillimore wrote: >> This is another version of PR https://github.com/openjdk/jdk/pull/14659 but >> I've added a pointer delta function in globalDefinitions.hpp to use for >> these pointer diff calculations that return int everywhere. If the name is >> agreeable, I'll fix the other cases of this like this. It's better than raw >> casts. >> Tested with tier1-4. > > Coleen Phillimore has updated the pull request incrementally with one > additional commit since the last revision: > > Use pointer_delta_as_int for the name that uses pointer_delta, fix negative > case to just do checked_cast. Looks good to me. - Marked as reviewed by fparain (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/14675#pullrequestreview-1503631614
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v4]
On Wed, 28 Jun 2023 13:17:21 GMT, Coleen Phillimore wrote:
>> This is another version of PR https://github.com/openjdk/jdk/pull/14659 but
>> I've added a pointer delta function in globalDefinitions.hpp to use for
>> these pointer diff calculations that return int everywhere. If the name is
>> agreeable, I'll fix the other cases of this like this. It's better than raw
>> casts.
>> Tested with tier1-4.
>
> Coleen Phillimore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Use pointer_delta_as_int for the name that uses pointer_delta, fix negative
> case to just do checked_cast.
LGTM.
src/hotspot/share/utilities/globalDefinitions.hpp line 529:
> 527: template
> 528: inline int pointer_delta_as_int(const volatile T* left, const volatile
> T* right) {
> 529: return checked_cast(pointer_delta(left, right, sizeof(T)));
For clarity, I think you should add a comment saying the returned value is
always non-negative.
-
Marked as reviewed by iklam (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14675#pullrequestreview-1503498625
PR Review Comment: https://git.openjdk.org/jdk/pull/14675#discussion_r1245463745
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v4]
> This is another version of PR https://github.com/openjdk/jdk/pull/14659 but > I've added a pointer delta function in globalDefinitions.hpp to use for these > pointer diff calculations that return int everywhere. If the name is > agreeable, I'll fix the other cases of this like this. It's better than raw > casts. > Tested with tier1-4. Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision: Use pointer_delta_as_int for the name that uses pointer_delta, fix negative case to just do checked_cast. - Changes: - all: https://git.openjdk.org/jdk/pull/14675/files - new: https://git.openjdk.org/jdk/pull/14675/files/858e9cfb..9fed8c89 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14675&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14675&range=02-03 Stats: 22 lines in 14 files changed: 0 ins; 1 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/14675.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14675/head:pull/14675 PR: https://git.openjdk.org/jdk/pull/14675
