> There are several places in hotspot where an internal function should have > been declared static, but isn't. > > These were discovered by trying to use the gcc option > `-Wmissing-declarations` and the corresponding clang option > `-Wmissing-prototypes`. These warnings check that a function either: > a) is declared static, or > b) has a declaration before its definition. > > The rationale of this is that functions are either internal to a compilation > unit, or exported to be linked by some other compilation unit. In the former > case, it should be marked static. In the latter case, it should be declared > in a header file, which should be included by the implementation as well. If > there is a discrepancy between the exported prototype and the implemented > prototype, this will be discovered during compilation (instead of as a > runtime error). Additionally, marking internal methods as static allows the > compiler to make better optimization, like inlining. > > This seem to be to be a sane assumption, and I think Hotspot (and the entire > JDK) would increase code quality by turning on these warnings. The absolute > majority of the code already adheres to these rules, but there are still some > places that needs to be fixed. > > This is the first part of addressing these issues, where all places that are > trivially missing static are fixed. > > I have discovered these by running with the warnings mentioned above turned > on. I have filtered out those places were an export was obviously missing. > The remaining warnings I have manually inspected. About 1/4 of them were > *_init() functions (which are directly declared in `init.cpp`) and another > 1/4 were documented as "use in debugger"; these I have not touched. I also > ignored functions with names suggesting it might be used in the debugger, > even if not documented as such, or any places that even seemed remotely > non-trivial. Finally I also reverted a few changes after it turned out that > gcc complained about unused functions. These places are actually dead code, > but it is not clear if they should be removed or if there is actually a call > missing somewhere (I believe it is a mix of both). In any case, I did not > want any such complexities in this PR. > > When the functions where marked static, gcc started complaining if they were > not used, since it consider it dead code. To address this, I had to add or > fix some `#ifdef`s. Since this code were not actually used unless these > criteria were fulfilled before either (it was just not discovered by the > compiler), I thi...
Magnus Ihse Bursie has updated the pull request incrementally with two additional commits since the last revision: - Revert spurious changes in non-modified file - Fix indentation issues ------------- Changes: - all: https://git.openjdk.org/jdk/pull/17806/files - new: https://git.openjdk.org/jdk/pull/17806/files/97595a3e..9702d84a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17806&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17806&range=00-01 Stats: 32 lines in 13 files changed: 3 ins; 0 del; 29 mod Patch: https://git.openjdk.org/jdk/pull/17806.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17806/head:pull/17806 PR: https://git.openjdk.org/jdk/pull/17806
