On Fri, 11 Nov 2022 21:01:31 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Stefan Karlsson has updated the pull request with a new target base due to a
>> merge or a rebase. The pull request now contains three commits:
>>
>> - Cleanups
>> - Merge remote-tracking branch 'upstream/master' into
>> various_include_order_fixes
>> - Various include order fixes
>
> src/hotspot/cpu/x86/macroAssembler_x86.cpp line 30:
>
>> 28: #include "compiler/compiler_globals.hpp"
>> 29: #include "compiler/disassembler.hpp"
>> 30: #include "crc32c.h"
>
> This ought to remain at the end, included using `CPU_HEADER_H("crc32c")`, but
> the file doesn't have the appropriate suffix. I think the file name should
> be changed so that can be done. This seems to be the only C++ file under
> cpu/ that doesn't have the appropriate suffix. There are similarly
> "misnamed" files under various os/ subdirs; I didn't look for any in os_cpu/.
I'd like to defer that discussion to a separate RFE.
> src/hotspot/os_cpu/bsd_zero/os_bsd_zero.cpp line 57:
>
>> 55: #if !defined(__APPLE__) && !defined(__NetBSD__)
>> 56: #include <pthread.h>
>> 57: # include <pthread_np.h> /* For pthread_attr_get_np */
>
> Remove unneeded space.
I reverted all such space cleanups, in the interest of getting the limited
cleanup fixed. If we want to fix these spaces I'd like that to happen as a
separate RFE.
> src/hotspot/share/cds/classListParser.cpp line 42:
>
>> 40: #include "interpreter/bytecodeStream.hpp"
>> 41: #include "interpreter/linkResolver.hpp"
>> 42: #include "jimage.hpp"
>
> Sorting jimage.hpp with hotspot/share stuff seems weird, and is kind of
> hiding this external dependency. (It's coming from java.base.)
We can hopefully discuss this as a separate RFE.
> src/hotspot/share/prims/whitebox.cpp line 26:
>
>> 24:
>> 25: #include "precompiled.hpp"
>> 26: #include <new>
>
> Why was `<new>` removed?
We tend to pull it in via memory/allocation.hpp. I can revert this, but I'm not
sure it is important. There's no consistency in when we include `<new>` or not.
> test/hotspot/gtest/jfr/test_adaptiveSampler.cpp line 48:
>
>> 46: #include "unittest.hpp"
>> 47:
>> 48: #include <cmath>
>
> Why is this after unittest.hpp?
System includes go after HotSpot includes.
-------------
PR: https://git.openjdk.org/jdk/pull/11108
