Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v3]
On Wed, 28 Jun 2023 07:52:35 GMT, Stefan Karlsson wrote: >> Taking out that cast does work, so I've fixed that. > >> pointer_delta has different semantics > > Right. That was "recently" added to pointer_delta with JDK-8260046. It begs > the question why felt the need to add it there but feel that it is OK to skip > it for delta_as_int? Is there some usage of delta_as_int that gives back > negative values? Could that call site be changed? There are sites where the result is negative but this is a good suggestion because it makes the name more consistent. I can change those to plain check_casts. - PR Review Comment: https://git.openjdk.org/jdk/pull/14675#discussion_r1245141370
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v3]
On Tue, 27 Jun 2023 15:11:34 GMT, Coleen Phillimore wrote: >> Hm. Maybe this would be ok. Our original idea was to make it T* not T until >> this cast. I don't know how many other cases there are that I haven't >> gotten to yet. But it would eliminate a cast, so that's good (unless these >> aren't the same). Some instances have ptr - constant that gets promoted I >> think. >> The reason we didn't pick pointer_delta_as_int because pointer_delta has >> different semantics. pointer_delta insists on positive results. > > Taking out that cast does work, so I've fixed that. > pointer_delta has different semantics Right. That was "recently" added to pointer_delta with JDK-8260046. It begs the question why felt the need to add it there but feel that it is OK to skip it for delta_as_int? Is there some usage of delta_as_int that gives back negative values? Could that call site be changed? - PR Review Comment: https://git.openjdk.org/jdk/pull/14675#discussion_r1244829942
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v3]
On Tue, 27 Jun 2023 20:48:27 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:
>
> Fixed the comment.
delta_as_int() could also be: delta2int() ptrdelta2int() ptrdiff2int()
ptrsub2int() in the theme of shorter names.
pointer_delta_as_int() ignoring it does something different than
pointer_delta() (allows negative returns).
Change them all to check_cast(a - b) - losing identifiable name
ptrdiff_cast(a - b) where int ptrdiff_cast(ptrdiff_t val) { return
check(val); }
Brainstorming in a PR. Your suggestions welcome.
-
PR Comment: https://git.openjdk.org/jdk/pull/14675#issuecomment-1610330128
Re: RFR: 8310906: Fix -Wconversion warnings in runtime, oops and some code header files. [v3]
> 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: Fixed the comment. - Changes: - all: https://git.openjdk.org/jdk/pull/14675/files - new: https://git.openjdk.org/jdk/pull/14675/files/99a15d6e..858e9cfb Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=14675&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=14675&range=01-02 Stats: 4 lines in 1 file changed: 1 ins; 0 del; 3 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
