RFR: 8296886: Fix various include sort order issues
Stefan Karlsson
stefank at openjdk.org
Mon Nov 14 08:01:32 UTC 2022
On Fri, 11 Nov 2022 14:26:20 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:
> The sorted blocks of includes have deteriorated to the point that I felt compelled to clean up some of the issues.
>
> One of the more prevalent issues is that files in src/hotspot/share/include are not properly sorted. There has been some discussion that that was done on purpose, but it just adds another exception to the include rules that don't have any practical purposes, IMHO. It also goes against our written style guide around include files. One argument why it was OK have the files in include/ pushed up to the top of the sorted block, was that the file was included without specifying a directory. That's an argument that contradicts how we treat platform-dependent files, which (unfortunately) often also are specified without a prefixed directory, so I don't think that's a good enough argument, again IMHO. To remove this special case, I've removed the extraneous make file entry to have src/hotspot/share/include in the set of directories to search for headers when compiling HotSpot. Now all the header files in src/hotspot/share/include gets included by specifying the path from src/hotspot/share,
just like the other platform-independent headers in HotSpot.
>
> While going over the include headers I've also cleaned up surrounding whitespaces and incorrect include guards.
I'll split this out into separate changes.
* Regarding in `#include "include/xxx.h"`: This used to be `#include "prims/xxx.h"` and sorted the rest of the header file. Then they got moved to include/ and the directory was dropped for some reason. @dholmes-ora maybe you have a better name than `include/`, that is less eye jarring?
* Remember, a lot of the order we used was generated by a script. Some of the complaints above are against the style that was used ever since we added these includes to the files.
* System files comes after the HotSpot includes, with a blank line before.
* Platform files don't necessarily use the macro stems and then don't sort after the shared includes. I'm fine with changing this, but that would have to be tackled as a separate proposal.
* Sounds like a good proposal to move the external, shared headers like jimage.h and jni.h to a separate section *after* the HotSpot includes.
* I also remember that unittest.h needs to go at the end of the list, and have seen many violations of that. I added a blank line as a suggestion that this might give a visual aid of what's going on.
* <new> is typically provided via allocation.hpp, but I can leave the few odd <new> includes.
-------------
PR: https://git.openjdk.org/jdk/pull/11108
More information about the hotspot-dev
mailing list