RFR: 8296886: Fix various include sort order issues [v2]

[Stefan Karlsson]

On Fri, 11 Nov 2022 21:01:31 GMT, Kim Barrett <kbarrett at 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