RFR: 8314694: Separate checked_cast from globalDefinitions.hpp

[Kim Barrett]

On Tue, 22 Aug 2023 06:54:20 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Please review this change which moves checked_cast from globalDefinitions.hpp
>> to a separate file.  As part of this change we modify files that use
>> checked_cast to directly include that new file.  There are around 80 such
>> files, and that change constitutes the majority of the changed files and lines
>> in this PR.
>> 
>> This PR doesn't fix the definition of checked_cast (see JDK-8314258).  It just
>> moves the existing definition to a new file, in preparation for fixing it
>> later.  (I'm running tests on a fixed implementation.)
>> 
>> An alternative is to move checked_cast to a new file but have
>> globalDefinitions.hpp include that new file.  This would avoid touching the
>> include lists of currently using files.  It seems to me better to actually
>> separate it.
>> 
>> Fortunately, there was only one copyright update needed. Most of the uses were
>> added recently as part of addressing -Wconversion warnings, so those files
>> have already had copyright updates recently.
>> 
>> The other change was to move pointer_delta_as_int next to the related
>> pointer_delta, and change it to use a direct assert and static_cast, rather
>> than checked_cast.
>> 
>> With the exception of the simple change to pointer_delta_as_int the changes
>> in this PR are very simple and almost mechanical.  To find the files needing
>> an additional include and to demonstrate completing that task, I applied this
>> command to the hotspot directory:
>> 
>> 
>> egrep -r --files-with-matches --exclude-dir=.git " checked_cast<" . | \
>>   xargs egrep --files-without-match "utilities/checkedCast.hpp"
>> 
>> 
>> So perhaps this change is trivial, despite the number of files.
>> 
>> Testing:
>> mach5 tier1
>
> Looks good.
> 
> Interesting how many of the touched cpp files did not include globalDefinitions.hpp.

Thanks for reviews @tstuefe and @coleenp .

> Interesting how many of the touched cpp files did not include globalDefinitions.hpp.

Oh, there are much worse things than that!  Take a look at utilities/elfFile.hpp :(

There was some exploration of the "Include What You Use" tool a long time ago, but it seemed like it didn't quite do what
we wanted.  https://include-what-you-use.org/

I'll run the command to check for using files that are missing the new include before integration.  That might show more
files needing the include added.  For example, https://github.com/openjdk/jdk/pull/15289 is ready to integrate.

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

PR Comment: https://git.openjdk.org/jdk/pull/15377#issuecomment-1688526986