RFR(M): 8233787: Break cycle in vm_version* includes

[Erik Joelsson]

Build change looks ok.

/Erik

On 2019-11-15 07:08, Daniel D. Daugherty wrote:
> Adding build-dev at ... since this changeset now touches
> make/hotspot/lib/CompileJvm.gmk. The Build team has a standing
> request to be included on reviews that touch makefiles...
>
> Dan
>
>
> On 11/14/19 11:26 AM, Schmidt, Lutz wrote:
>> Hi Kim,
>>
>> that wasn't straightforward. Had to adapt 
>> make/hotspot/lib/CompileJvm.gmk. Build settings like 
>> HOTSPOT_VERSION_STRING have to flow into the compile step of 
>> abstract_vm_version.cpp now. For the details, see my comments below.
>>
>> Other than that, I hope the new webrev is even closer to your dreams:
>>     http://cr.openjdk.java.net/~lucy/webrevs/8233787.02/
>>
>> Thanks,
>> Lutz
>>
>>
>> On 14.11.19, 00:34, "Kim Barrett" <kim.barrett at oracle.com> wrote:
>>
>>      > On Nov 13, 2019, at 11:42 AM, Schmidt, Lutz 
>> <lutz.schmidt at sap.com> wrote:
>>      >
>>      > Hi Kim,
>>      >
>>      > there is a new webrev at 
>> http://cr.openjdk.java.net/~lucy/webrevs/8233787.01/
>>      >
>>      > It should be pretty close to what you view as the "right 
>> approach". There weren't too many changes relative to 8233787.00. 
>> Most files already had #include runtime/vm_version.hpp.
>>           This looks much better to me, but many (most?) of the changed
>>      #includes need to be moved into sort order.
>>
>> R: tried my best to fix the sort order. Sorry for not paying 
>> attention in the first place.
>> ------------------------------------------------------------------------------
>>      src/hotspot/share/runtime/vm_version.cpp
>>           Abstract_VM_Version definitions should be moved to 
>> abstract_vm_version.cpp.
>>      Maybe just rename the file; I think the only thing that would be 
>> left
>>      for vm_version.cpp would be VM_Version_init().  But maybe that 
>> should
>>      be left behind in vm_version.cpp?  Though that makes the review 
>> messier.
>>
>> R: Everything moved as you suggested. Doesn't make sense to have 
>> Abstract_VM_Version:: methods in vm_version.cpp file.
>> ------------------------------------------------------------------------------
>>      src/hotspot/share/runtime/abstract_vm_version.hpp
>>           Should #include globalDefinitions.hpp.
>>      - uint64_t features()
>>      - #define SUPPORTS_NATIVE_CX8
>>
>> R: Done.
>>           Should forward-declare class outsputStream.
>>      - print_virtualization_info
>>      - print_matching_lines_from_file (I wonder why this is *here*, 
>> but not your problem)
>>
>> R: Done.
>> ------------------------------------------------------------------------------
>>
>