On Fri, 11 Nov 2022 14:26:20 GMT, Stefan Karlsson <stef...@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
