[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Integrated: 8267464: Circular-dependency resilient inline headers
From:       Stefan Karlsson <stefank () openjdk ! java ! net>
Date:       2021-05-31 9:02:32
Message-ID: H9v57pFhdfqtfzMpdJfBS5Ed3hNBQg6hSM3XuNYmGcY=.3f812158-2b34-418e-8172-5d47af7fa8cc () github ! com
[Download RAW message or body]

On Thu, 20 May 2021 11:55:05 GMT, Stefan Karlsson <stefank@openjdk.org> wrote:

> 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?

This pull request has now been integrated.

Changeset: 64f0f689
Author:    Stefan Karlsson <stefank@openjdk.org>
URL:       https://git.openjdk.java.net/jdk/commit/64f0f68958a74d4ee34c4b738759fc2278fa8b47
                
Stats:     490 lines in 242 files changed: 349 ins; 118 del; 23 mod

8267464: Circular-dependency resilient inline headers

Reviewed-by: kbarrett, eosterlund, dholmes, kvn

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

PR: https://git.openjdk.java.net/jdk/pull/4127


[prev in list] [next in list] [prev in thread] [next in thread]