On Tue, 22 Aug 2023 04:13:13 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 Looks good! src/hotspot/share/utilities/globalDefinitions.hpp line 485: > 483: size_t delta = pointer_delta(left, right, sizeof(T)); > 484: assert(delta <= size_t(INT_MAX), "pointer delta out of range: %zu", > delta); > 485: return static_cast<int>(delta); I see what you did here... ------------- Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15377#pullrequestreview-1589256447 PR Review Comment: https://git.openjdk.org/jdk/pull/15377#discussion_r1301529527
