On Fri, 2 Sep 2022 09:51:06 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:

> During the discussion of 
> [JDK-8292981](https://bugs.openjdk.org/browse/JDK-8292981) an opinion was 
> voiced that we should stop using INTPTR_FORMAT when printing pointers.
> 
> Some background that could explain why some tend to use INTPTR_FORMAT instead 
> of PTR_FORMAT:
> 
> Both those format specifiers require an integer and the compiler barfs if you 
> send in a pointer. We therefore have a utility function named p2i, which 
> converts the pointer to an integer. So, everywhere we have to write print 
> line like this: `print("my pointer: " PTR_FORMAT, p2i(my_pointer));`. Now, 
> p2i returns an intptr_t, and it becomes natural for some to consider the type 
> of the converted value (that we need because of the mentioned workaround), 
> instead of the original type of the value.
> 
> With this enhancement I'd like to clean up the code that I often work in, so 
> that it uses PTR_FORMAT when printing pointers. I'm leaving the rest of the 
> code base for others to consider cleaning up.
> 
> Cleanups have been done in directories gc/, utilities/, memory/, and oops/.

Looks good.

I did notice a couple INTPTR_FORMAT => (new) SIZE_FORMAT_X_0.  Good change.

I also noticed a place where there are sequential asserts, one (previously)
using INTPTR_FORMAT and the next PTR_FORMAT, for the same value. Seems like
somebody wasn't paying attention. Again good change.

Also noticed some pre-existing unnecessary (void*) casts of the pointer
argument to p2i. Not your problem, and not really appropriate for this change
to do anything about them, but I really hate eye-catching cast junk like that.

There were also a few INTPTR_FORMAT uses that were left in place. All that I
recall looked like they could/should instead be SIZE_FORMAT_X_0 or maybe even
SIZE_FORMAT_X if the zero padding isn't useful. Leaving those for a later pass
seems fine. I wonder how many INTPTR_FORMATs are left in the areas you worked
on?

-------------

Marked as reviewed by kbarrett (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10141

Reply via email to