On Wed, 4 Feb 2026 10:57:49 GMT, Quan Anh Mai <[email protected]> wrote:

>> Marc Chevalier has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains 14 additional 
>> commits since the last revision:
>> 
>>  - Addressing more comments
>>  - Merge
>>  - Some reviews
>>  - Details
>>  - exception handling
>>  - oops
>>  - Also cache correctly whole stable fields
>>  - fix comment
>>  - Remove debug
>>  - ShouldNotReachHere
>>  - ... and 4 more: 
>> https://git.openjdk.org/valhalla/compare/b50e1f43...34be1bd4
>
> src/hotspot/share/ci/ciInstance.cpp line 65:
> 
>> 63: // ciInstance::field_value_impl
>> 64: ciConstant ciInstance::field_value_impl(ciField* field) {
>> 65:   BasicType field_btype = field->type()->basic_type();
> 
> Just a very little nitpick: We often call the `BasicType` of something `bt`.

I'm fine renaming it, it was just like that before and I've kept it. But I like 
that it mentions of what it is the basic type. I've seen things as `bt2` and 
wasn't thrilled. Is `field_bt` fine for you (or `bt_field`)? Anyway, there 
aren't that many stuff going on in this method, so I'd be fine with a simple 
`bt`.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1923#discussion_r2763721247

Reply via email to