On Tue, 22 Aug 2023 21:25:48 GMT, Kim Barrett <kbarr...@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 > > Kim Barrett 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 16 additional > commits since the last revision: > > - add include needed post-JDK-8314274 > - Merge branch 'master' into move-checked-cast > - include checkedCast.hpp in cpu files > - include checkedCast.hpp in cpu/aarch64 files > - include checkedCast.hpp in cpu/x86 files > - include checkedCast.hpp in os files > - include checkedCast.hpp in remaining share files > - include checkedCast.hpp in classfile files > - include checkedCast.hpp in code files > - include checkedCast.hpp in oops files > - ... and 6 more: https://git.openjdk.org/jdk/compare/f0750585...3a3fbb0b Changes in `src/hotspot/share/prims` folder look good. Thanks, Serguei ------------- Marked as reviewed by sspitsyn (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15377#pullrequestreview-1590815334
