> I'd like to propose a small adjustment to how we write .inline.hpp files, in
> the hope to reduce include problems because of circular dependencies between
> inline headers.
>
> This is a small, contrived example to show the problem:
>
> // a.hpp
> #pragma once
>
> void a1();
> void a2();
>
>
> // a.inline.hpp
> #pragma once
>
> #include "a.hpp"
> #include "b.inline.hpp"
>
> inline void a1() {
> b1();
> }
>
> inline void a2() {
> }
>
>
> // b.hpp
> #pragma once
>
> void b1();
> void b2();
>
>
> // b.inline.hpp
> #pragma once
>
> #include "a.inline.hpp"
> #include "b.hpp"
>
> inline void b1() {
> }
>
> inline void b2() {
> a1();
> }
>
> The following code compiles fine:
>
> // a.cpp
> #include "a.inline.hpp"
>
> int main() {
> a1();
>
> return 0;
> }
>
> But the following:
>
> // b.cpp
> #include "b.inline.hpp"
>
> int main() {
> b1();
>
> return 0;
> }
>
>
> fails with the this error message:
>
> In file included from b.inline.hpp:3,
> from b.cpp:1:
> a.inline.hpp: In function ‘void a1()’:
> a.inline.hpp:8:3: error: ‘b1’ was not declared in this scope; did you mean
> ‘a1’?
>
> We can see the problem with g++ -E:
>
> # 1 "a.inline.hpp" 1
> # 1 "a.hpp" 1
>
> void a1();
> void a2();
>
> # 4 "a.inline.hpp" 2
>
> inline void a1() {
> b1();
> }
>
> inline void a2() {
> }
>
> # 4 "b.inline.hpp" 2
> # 1 "b.hpp" 1
>
> void b1();
> void b2();
>
> # 5 "b.inline.hpp" 2
>
> inline void b1() {
> }
>
> inline void b2() {
> a1();
> }
>
> # 2 "b.cpp" 2
>
> int main() {
> b1();
>
> return 0;
> }
>
>
> b1() is called before it has been declared. b.inline.hpp inlined
> a.inline.hpp, which uses functions declared in b.hpp. However, at that point
> we've not included enough of b.inline.hpp to have declared b1().
>
> This is a small and easy example. In the JVM the circular dependencies
> usually involves more than two files, and often from different sub-systems of
> the JVM. We have so-far solved this by restructuring the code, making
> functions out-of-line, creating proxy objects, etc. However, the more we use
> templates, the more this is going to become a reoccurring nuisance. And in
> experiments with lambdas, which requires templates, this very quickly showed
> up as a problem.
>
> I propose that we solve most (all?) of these issues by adding a rule that
> states that .inline.hpp files should start by including the corresponding
> .hpp, and that the .hpp should contain the declarations of all externally
> used parts of the .inline.hpp file. This should guarantee that the
> declarations are always present before any subsequent include can create a
> circular include dependency back to the original file.
>
> In the example above, b.inline.hpp would become:
>
> // b.inline.hpp
> #pragma once
>
> #include "b.hpp"
> #include "a.inline.hpp"
>
> inline void b1() {
> }
>
> inline void b2() {
> a1();
> }
>
> and now both a.cpp and b.cpp compiles. The generated output now looks like
> this:
>
> # 1 "b.inline.hpp" 1
> # 1 "b.hpp" 1
>
> void b1();
> void b2();
>
> # 4 "b.inline.hpp" 2
> # 1 "a.inline.hpp" 1
>
> # 1 "a.hpp" 1
>
> void a1();
> void a2();
>
> # 4 "a.inline.hpp" 2
>
> inline void a1() {
> b1();
> }
>
> inline void a2() {
> }
>
> # 5 "b.inline.hpp" 2
>
> inline void b1() {
> }
>
> inline void b2() {
> a1();
> }
>
> # 2 "b.cpp" 2
>
> int main() {
> b1();
>
> return 0;
> }
>
> The declarations come first, and the compiler is happy.
>
> An alternative to this, that has been proposed by other HotSpot GC devs have
> been to always include all .hpp files first, and then have a section of
> .inline.hpp includes. I've experimented with that as well, but I think it
> requires more maintenance and vigilance of *users* .inline.hpp files (add
> .hpp include to the first section, add .inline.hpp section to second). The
> proposed structures only requires extra care from *writers* of .inline.hpp
> files. All others can include .hpp and .inline.hpp as we've been doing for a
> long time now.
>
> Some notes about this patch:
> 1) Some externally-used declarations have been placed in .inline.hpp files,
> and not in the .hpp. Those should be moved. I have not done that.
> 2) Some .inline.hpp files don't have a corresponding .hpp file. I could
> either try to fix that in this patch, or we could take care of that as
> separate patches later.
> 3) The style I chose was to add the .hpp include and a extra blank line,
> separating it from the rest of the includes. I think I like that style,
> because it makes it easy for those writing .inline.hpp to recognize this
> pattern. And it removes the confusion why this include isn't sorted into the
> other includes.
> 4) We have a few *special* platform dependent header files. Both those that
> simply are included into platform independent files, and those that inject
> code *into* the platform independent classes. Extra care, as always, need to
> be taken around those files.
> 5) Mostly tested locally, but I'll test on more platforms if the idea is
> accepted.
>
> What do you think?
Stefan Karlsson has updated the pull request with a new target base due to a
merge or a rebase. The incremental webrev excludes the unrelated changes
brought in by the merge/rebase. The pull request contains three additional
commits since the last revision:
- Merge remote-tracking branch 'origin/master' into
8267464_circular-dependency_resilient_inline_headers
- Clean up assembler_<cpu>.inline.hpp
- 8267464: Circular-dependency resiliant inline headers
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/4127/files
- new: https://git.openjdk.java.net/jdk/pull/4127/files/260c1115..4bcd4348
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4127&range=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4127&range=01-02
Stats: 5145 lines in 153 files changed: 2603 ins; 1960 del; 582 mod
Patch: https://git.openjdk.java.net/jdk/pull/4127.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/4127/head:pull/4127
PR: https://git.openjdk.java.net/jdk/pull/4127
