On Mon, 20 Jan 2025 23:35:23 GMT, Alexander Zuev <kiz...@openjdk.org> wrote:

>> Julian Waters has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains eight commits:
>> 
>>  - Merge branch 'openjdk:master' into accessibility
>>  - Cast to void in AccessBridgeCalls.c
>>  - static_cast to void in jaccessinspector.cpp
>>  - Formatting changes in AccessBridgeEventHandler.cpp
>>  - Merge branch 'master' into accessibility
>>  - Remove now unused result
>>  - Merge branch 'master' into accessibility
>>  - 8342870
>
> src/jdk.accessibility/windows/native/jaccesswalker/jaccesswalker.cpp line 255:
> 
>> 253:     case WM_CREATE:
>> 254:         RECT rcClient;    // dimensions of client area
>> 255:      // HWND hwndEdit;    // handle of tree-view control
> 
> Again, i would not remove or comment out variables assigned within the code - 
> they might not be used down the line but in interactive debug session it is 
> good to have them to see what was the result of the function call without 
> stepping inside the function. I agree that removing the declarations of 
> unused variables can be justified but this in my opinion is not the case.

If we had C++17 [[maybe_unused]] would be available to mark these instances so 
the compiler doesn't complain, but we're not there yet. As an alternative they 
can be cast to void (Like it was done above in AccessBridgeCalls.c). Could that 
be done instead? I do have doubts that the compilers will retain these locals 
when optimizations are turned on, but if that is the case, then I agree that 
these should not be removed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21656#discussion_r1922935414

Reply via email to