RFR: 8311077: Fix -Wconversion warnings in jvmti code [v3]
Coleen Phillimore
coleenp at openjdk.org
Fri Jun 30 13:17:00 UTC 2023
On Fri, 30 Jun 2023 02:18:14 GMT, David Holmes <dholmes at openjdk.org> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add a comment about u1 cast to new_index for the ldc bytecode.
>
> Sorry Coleen but this raises a lot of questions for me. I expect there are a few ways to silence these conversion warnings but many of the proposed changes don't look semantically right to me. I realize that we have a lot of inconsistencies in the way we declare and use types and that the proposed changes may be the most minimal way to fix things, but it is better to type them correctly from the start IMO and only cast at the "boundaries" if it requires a change to propagate too far.
@dholmes-ora The reason that these changes are broken up like this is because there are a huge amount of -Wconversion warnings and to do it right, we need to examine where the warnings are to see if there are bugs, and to correct the code to either use the right types or allow the cast with an assertion check if necessary. The changes are not designed to be completely minimal but balanced so that people can review them and that changes that aren't necessary aren't made.
We pass around 'int' as a parameter type for values that are u2, for example. It really matters most when we're storing that value or using it in a context that requires the narrow type. If we're using it as an index which most constant pool and constant pool cache indices are used as, we can keep it as an int. A u2 parameter is promoted to int so it's the same thing.
I hope I've answered your questions about these specific types in this change.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14710#issuecomment-1614635744
More information about the hotspot-dev
mailing list