RFR: 8342869: Errors related to unused code on Windows after 8339120 in awt

Magnus Ihse Bursie ihse at openjdk.org
Thu Dec 12 09:39:45 UTC 2024


On Thu, 12 Dec 2024 05:39:04 GMT, Julian Waters <jwaters at openjdk.org> wrote:

>> You need to split this up into multiple parts. One part is about removing dead code. Do not comment it out, just remove it. Open a new JBS issue on client-libs for removal of dead code. This should be trivial to get pushed.
>> 
>> Then, you have some other changes. Looks like you moved something from a header file to a cpp file. Make that a separate PR, also on client-libs.
>> 
>> Then you are making some annotation stuff. I have not seen these before. Are they established? This might need some more discussion to get a consensus on how to proceed with.
>> 
>> And finally you are bumping the C++ language level. You have already opened a separate JBS issue for that, and we've said that we do that when we do that. So just drop that part.
>
> The dead code removal may not be correct, same goes for the rest of the changes. Most, if not all of them (Especially the C++17 change) are pure placeholders to simply draw attention to the problem sites. I think all of them need discussion on how to proceed with (Same with the corresponding jdk.accessibility changes), not just the annotations. Speaking of which, the annotations are C++ attributes, introduced in C++11, and are (fortunately) Standard and not compiler extensions. Splitting this up to even smaller changes may or may not inconvenience the few reviewers that AWT and A11Y has, which is what I'm worried about

The code has likely been dead for years. If no-one has reported any bugs about this, it is likely correct that it is unused.

If you think that is a bug, do some git archaeology to determine when it became dead, or locate what pathway you think it should be used it, and try to provoke an error. 

I still think publishing a simple PR which just removes dead code is the best way for you to proceed here. Such a PR should be trivial to review.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21655#discussion_r1881717235


More information about the build-dev mailing list