RFC: Organising .inline.hpp and .hpp includes

[Stefan Karlsson]

Hi David,


On 2016-08-11 09:31, David Holmes wrote:
> I'm trying to tackle the incorrect inclusion of .hpp files instead of 
> .inline.hpp starting with the use of atomic.inline.hpp ref:
>
> https://bugs.openjdk.java.net/browse/JDK-8157907
>
> The basic rules would seem to be:
>
> If class Foo has inline methods that need to call the inline Atomic 
> methods then:
>
> - Those methods should be defined in foo.inline.hpp
>
>    inline void Foo::a() { /* do stuff with atomics */ }
>
> - foo.hpp declares those methods:
>
>    inline void a();
>
> - foo.inline.hpp includes foo.hpp
> - foo.inline.hpp includes atomic.inline.hpp
> - foo.cpp includes foo.inline.hpp
>
> Do have agreement so far?

Yes.

>
> Now suppose we have class Bar which uses Foo.
>
> - bar.cpp should include foo.inline.hpp
> - If bar.hpp only references Foo but doesn't need to see a full 
> definition then bar.hpp can forward declare Foo
> - If bar.hpp defines inline methods that require full access to Foo 
> then either:
>
> a) bar.hpp includes foo.inline.hpp; or
> b) The inline functions of Bar are themselves moved to bar.inline.hpp 
> which includes foo.inline.hpp
>
> I think there was general opinion that (a) is undesirable, and that 
> (b) is preferable - is that right?

Yes.

Alternatively, if inlining isn't needed, place these functions in bar.cpp.

>
> What I am finding is that the fan out of applying (b) is rather large 
> - to the point where we may end up having x.hpp, x.inline.hpp and 
> x.cpp variants for very many x!

Yes, that's what I've also seen while investigating:

https://bugs.openjdk.java.net/browse/JDK-8154079
http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-April/022626.html

I have an updated version that works with and without precompiled headers:
http://cr.openjdk.java.net/~stefank/8154079/webrev.02/

Regarding atomic.inline.hpp and the atomic_<os_cpu>.inline.hpp files, 
these files typically don't have a lot of dependencies on other header 
files and a pragmatic way forward could be to even further minimize 
those dependencies and move the implementation into atomic_<os_cpu>.hpp 
files and get rid of atomic.inline.hpp. I know that goes against the 
guidelines mentioned above, but it's an alternative to use if the fan 
out becomes too large.

>
>
> Just to add another dimension to this, what should precompiled.hpp 
> include? I would assume the .inline.hpp files right?

I would prefer if we got rid of the .inline.hpp files from precompiled.hpp.

Thanks,
StefanK

>
> Thanks,
> David