On Fri, 21 May 2021 09:11:09 GMT, Stefan Karlsson <[email protected]> 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?
>
> Stefan Karlsson has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Clean up assembler_<cpu>.inline.hpp
Looks great!
-------------
Marked as reviewed by eosterlund (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4127
