Request or comment: 8071690: Include local HotSpot headers before the system headers

[Volker Simonis]

Hi,

I've just opened "8071690: Include local HotSpot headers before the
system headers" but before I start working on it I'd like to hear what
others think about the problem. If there's a consensus that it will be
worth while doing this change I'll be happy to start working on it.

https://bugs.openjdk.java.net/browse/JDK-8071690

The following description is copied from the bug report for your convenience:

There's no general rule in which order header files should be included
but I think a good hint is to include local, project specific headers
before system headers. This is mainly for two reasons:

1. It prevents that dependencies from local header files to system
headers are hidden because a local header is included after a system
header by chance. Instead, every local header should explicitly
specify the system headers it depends on.

2. It enables the definition of local macros which control the
behaviour of the system headers which are included later on.

Point two may be considered bad style, but we actually use it for
example in src/share/vm/utilities/globalDefinitions.hpp where we
define "__STDC_FORMAT_MACROS" before we include "<inttypes.h>" in the
compiler specific global definitions file.

"__STDC_FORMAT_MACROS" controls the definition of the printf format
macros in "<inttypes.h>" but this can only work if "<inttypes.h>" is
really included AFTER the definition of "__STDC_FORMAT_MACROS". And
this can only wok if every file includes the local HotSpot headers
before any system headers, because otherwise "<inttypes.h>" may be
indirectly included by a system header before we had a chance to
define "__STDC_FORMAT_MACROS".

This is exactly what happened after the integration of 8050807 which
added the system include "<fcntl.h>" to vmError.cpp as follows:

#include <fcntl.h>
#include "precompiled.hpp"
#include "code/codeCache.hpp"

This change broke the build on AIX because "<fcntl.h>" indirectly
included "<inttypes.h>" BEFORE we defined "__STDC_FORMAT_MACROS".

To prevent such errors in the future I propose to change all HotSpot
source files to always include the system headers AFTER the inclusion
of the project specific HotSpot headers.

I’ve attached a small Pythin script to this bug report which can be
used as follows to detect the files which are currently violating this
rule:

find src/ \( -name "*.cpp" -o -name "*.hpp" \) -type f -exec python
include.py {} \;

src/os/solaris/dtrace/generateJvmOffsets.cpp: system header #include
<proc_service.h> included before local header #include
"code/codeBlob.hpp"
src/os/windows/vm/decoder_windows.hpp: system header #include
<imagehlp.h> included before local header #include
"utilities/decoder.hpp"
src/os_cpu/bsd_zero/vm/os_bsd_zero.cpp: system header # include
<pthread_np.h> included before local header #include
"assembler_zero.inline.hpp"
src/share/vm/adlc/adlc.hpp: system header #include <iostream> included
before local header #include "string.h"
src/share/vm/libadt/port.hpp: system header #include <string.h>
included before local header #include "port_tandem.hpp"
src/share/vm/runtime/os.hpp: system header # include <setjmp.h>
included before local header # include "jvm_solaris.h"
src/share/vm/trace/traceDataTypes.hpp: system header #include
<stddef.h> included before local header #include
"utilities/globalDefinitions.hpp"
src/share/vm/utilities/dtrace.hpp: system header #include
<sys/types.h> included before local header #include
"dtracefiles/hotspot.h"
src/share/vm/utilities/elfFile.cpp: system header #include <new>
included before local header #include "memory/allocation.inline.hpp"
src/share/vm/utilities/elfFile.hpp: system header #include <stdio.h>
included before local header #include "globalDefinitions.hpp"
src/share/vm/utilities/vmError.cpp: system header #include <fcntl.h>
included before local header #include "precompiled.hpp"

The script is written in Python intentionally such that it can be
easily added as an automated hook to jcheck to prevent violations of
this inclusion rule in the future.

Of course we can also try to not rely on the inclusion order at all.
IMHO it actually seems that this is the cleanest solution, but it may
require moving some macro definitions from header files right into the
command line (e.g. -D__STDC_FORMAT_MACROS for the example above). By
the way, that's actually the way how I've fixed the above mentioned
compile error on AIX without the need to touch any shared files.

What do you think?

Regards,
Volker