RFR: 8267464: Circular-dependency resiliant inline headers

[Stefan Karlsson]

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?

-------------

Commit messages:
 - 8267464: Circular-dependency resiliant inline headers

Changes: https://git.openjdk.java.net/jdk/pull/4127/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4127&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267464
  Stats: 459 lines in 224 files changed: 350 ins; 104 del; 5 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