RFR: 8072911: Remove includes of oop.inline.hpp from .hpp files

[Stefan Karlsson]

On 2015-02-11 19:34, Volker Simonis wrote:
> I found the problem. Can you please include the following additional
> line in interp_masm_ppc_64.cpp:
>
> diff -r e9b3fedb3bca src/cpu/ppc/vm/interp_masm_ppc_64.cpp
> --- a/src/cpu/ppc/vm/interp_masm_ppc_64.cpp     Wed Feb 11 13:20:41 2015 +0100
> +++ b/src/cpu/ppc/vm/interp_masm_ppc_64.cpp     Wed Feb 11 19:00:23 2015 +0100
> @@ -29,6 +29,7 @@
>   #include "interp_masm_ppc_64.hpp"
>   #include "interpreter/interpreterRuntime.hpp"
>   #include "prims/jvmtiThreadState.hpp"
> +#include "runtime/sharedRuntime.hpp"
>
>   #ifdef PRODUCT
>   #define BLOCK_COMMENT(str) // nothing
>
> It seems this was somehow implicitly included before your change.

Thanks for verifying the build on these platforms and for providing the 
fix. I'll add it to the patch.

StefanK

>
> Thanks,
> Volker
>
>
> On Wed, Feb 11, 2015 at 6:56 PM, Volker Simonis
> <volker.simonis at gmail.com> wrote:
>> Hi Stefan,
>>
>> thanks for doing this change and thanks for changing the ppc-files as well.
>>
>> Unfortunately there still seems to be an error if building on
>> Linux/ppc64 without precompiled headers.
>> Can you please wait a little until I'll send you the additional
>> changes which are required before pushing this one.
>>
>> Thanks,
>> Volker
>>
>> PS: I've just checked that AIX/ppc64 builds without a problem.
>>
>> On Wed, Feb 11, 2015 at 5:57 PM, Jesper Wilhelmsson
>> <jesper.wilhelmsson at oracle.com> wrote:
>>> Looks good!
>>> /Jesper
>>>
>>> Stefan Karlsson skrev den 11/2/15 16:55:
>>>
>>>> Hi Bengt,
>>>>
>>>> On 2015-02-11 16:51, Bengt Rutisson wrote:
>>>>>
>>>>> Hi Stefan,
>>>>>
>>>>> Pretty difficult to review this type of change, but I think it looks
>>>>> good. I
>>>>> verified that no hpp files inlcude oop.inline.hpp after your change. Nice
>>>>> cleanup!
>>>>>
>>>>> One minor thing (not your code):
>>>>>
>>>>> In src/share/vm/oops/oop.hpp there is this comment:
>>>>>
>>>>>    // type test operations (inlined in oop.inline.h)
>>>>>
>>>>> I think the comment should say oop.inline.hpp not just .h. Do you mind
>>>>> fixing
>>>>> this as part of this change too?
>>>>
>>>> Thanks for reviewing. I'll fix the comment.
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>> Thanks,
>>>>> Bengt
>>>>>
>>>>>
>>>>>
>>>>> On 11/02/15 13:38, Stefan Karlsson wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Please, review this patch to fix our includes of oop.inline.hpp.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~stefank/8072911/webrev.01
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8072911
>>>>>>
>>>>>>
>>>>>>  From the enhancement description:
>>>>>> ---
>>>>>> Our inline.hpp files should preferably only be included from .cpp files
>>>>>> or
>>>>>> other .inline.hpp files. When they are included in .hpp files, the
>>>>>> include
>>>>>> dependency of the implementation spreads to unrelated parts of the code
>>>>>> base,
>>>>>> and it's easy to get circular dependencies.
>>>>>>
>>>>>> This guide line is documented on:
>>>>>> https://wiki.openjdk.java.net/display/HotSpot/StyleGuide#Files
>>>>>>
>>>>>> oop.inline.hpp is one of the more problematic files, and I propose a
>>>>>> patch to
>>>>>> get rid of all inclusions of oop.inline.hpp from other .hpp files.
>>>>>> ---
>>>>>>
>>>>>>
>>>>>> Summary of the code changes:
>>>>>>
>>>>>> oop.inline.hpp
>>>>>>   - Added to needed .cpp and .inline.hpp files.
>>>>>>
>>>>>> oop.inline2.hpp
>>>>>>   - This file isn't needed anymore and the contents have been moved to
>>>>>> oop.inline.hpp.
>>>>>>
>>>>>> oop.hpp
>>>>>>   - Inlined klass_gap_offset_in_bytes in the hpp file, since it's
>>>>>> currently
>>>>>> used from a few hpp files.
>>>>>>
>>>>>>   - Create non-inlined versions of is_instance checks, to be used in
>>>>>> asserts
>>>>>> in handles.hpp.
>>>>>>
>>>>>>   - Added has_klass_gap to get rid of dependency against globals.hpp
>>>>>>
>>>>>> javaClasses.hpp
>>>>>>   - There were a couple of usages of klass.hpp that were moved out to
>>>>>> javaClasses.cpp and a few that were moved to javaClasses.inline.hpp when
>>>>>> it
>>>>>> wasn't obvious if we needed to inline the functions or not.
>>>>>>
>>>>>> vmGCOperations.hpp
>>>>>>   - Moved the VM_GC_Operation destructor to the cpp file.
>>>>>>
>>>>>> collectedHeap.hpp
>>>>>>   - Moved print_on_error
>>>>>>
>>>>>> barrierSet.hpp
>>>>>>   - Moved the inline keyword from the hpp file to the inline.hpp file.
>>>>>>
>>>>>> cardTableModRefBS.hpp
>>>>>>   - Moved inline_write_ref_field to the inline.hpp file.
>>>>>>
>>>>>> objArrayOop.hpp
>>>>>>   - Moved obj_at to the inline.hpp file.
>>>>>>
>>>>>> graphKit.hpp
>>>>>>   - Moved byte_map_base_node to .cpp file, where it's used.
>>>>>>
>>>>>> <other added includes>
>>>>>>   - Added missing includes that were removed when oop.inline.hpp was
>>>>>> removed
>>>>>> from the hpp files.
>>>>>>
>>>>>>
>>>>>> The patch has been tested with JPRT.
>>>>>>
>>>>>> Thanks,
>>>>>> StefanK
>>>>>>