RFR(M): Fix non-PCH build on Linux, Windows and MacOS X

[David Holmes]

Volker, thanks for the report and the time spent on the patch. 
Unfortunately I am very reluctant to accept significant cpp changes for 
something that should be purely solvable by appropriate header file 
inclusion. So hopefully when this is examined more closely the "right" 
fix will become evident. Also there may be some recent uses of atomic 
functions that have now been reversed and so might fix some of the 
problems encountered.

And Volker is right that we should have a no PCH build as part of JPRT 
testing. How to do that is another matter :(

Anyone have a tool or tip/trick for generating a tree of the includes 
for a given cpp file?

David

On 26/02/2013 4:58 AM, Christian Thalinger wrote:
>
> On Feb 25, 2013, at 10:09 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
>
>> Recent changes have broken the no-PCH build again.
>
> Is this also broken for you?
>
> http://bugs.sun.com/view_bug.do?bug_id=8005084
>
> -- Chris
>
>>
>> The fix requires some bigger changes this time but before going into
>> the details - PLEASE, PLEASE add a no-PCH build to your JPRT (or
>> nightly build or whatever) system to catch such errors. On platforms
>> which don't support PCH (i.e. Solaris) the normal build 'naturally'
>> works and I would consider it 'good practice' for a project to be able
>> to build without such an esoteric:) feature like precompiled headers.
>> Actually this only means that all the dependencies of a compilation
>> unit are explicitly recorded in that compilation unit (which I'd
>> consider self-evident).
>>
>> Tested 'product' and 'jvmg' builds on Linux/x86_64, Linux/x86_32,
>> Windows/x86_64, Solaris/Sparc_64, Solaris/Sparc_32, and MacOS
>> X/x86_64.
>>
>> Please be so kind to open a BugID for this issue and review the change:
>>
>> http://cr.openjdk.java.net/~simonis/webrevs/fix_nonPCH_build/
>>
>> Regards,
>> Volker
>>
>> PS: following some details of this change:
>>
>> The following places were easy to fix (just include the appropriate
>> ".inline.hpp" files):
>>
>> src/share/vm/oops/symbol.hpp
>>
>> - uses Atomic inline functions (inc() and dec()) without including
>> "runtime/atomic.inline.hpp"
>>
>> src/share/vm/memory/allocation.inline.hpp
>>
>> - uses Atomic inline functions (load() and store()) without including
>> "runtime/atomic.inline.hpp"
>>
>> src/os/windows/vm/decoder_windows.cpp
>>
>> - uses Arguments::get_java_home() without including "runtime/arguments.hpp"
>>
>> src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp
>> src/os_cpu/bsd_x86/vm/orderAccess_bsd_x86.inline.hpp
>> src/os_cpu/solaris_x86/vm/orderAccess_solaris_x86.inline.hpp
>> src/os_cpu/solaris_sparc/vm/orderAccess_solaris_sparc.inline.hpp
>> src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp
>>
>> - use the Atomic inline functions (load() and store()) without
>> including "runtime/atomic.inline.hpp"
>>
>> src/os_cpu/bsd_x86/vm/atomic_bsd_x86.inline.hpp
>> src/os_cpu/linux_x86/vm/atomic_linux_x86.inline.hpp
>> src/os_cpu/linux_zero/vm/atomic_linux_zero.inline.hpp
>> src/os_cpu/linux_sparc/vm/atomic_linux_sparc.inline.hpp
>> src/os_cpu/windows_x86/vm/atomic_windows_x86.inline.hpp
>> src/os_cpu/bsd_zero/vm/atomic_bsd_zero.inline.hpp
>> src/os_cpu/solaris_sparc/vm/atomic_solaris_sparc.inline.hpp
>> src/os_cpu/solaris_x86/vm/atomic_solaris_x86.inline.hpp
>>
>> - include "orderAccess_<os>_<cpu>.inline.hpp" without using any
>> OrderAccess method. I removed this includes to avoid a cyclic
>> dependency between "orderAccess_<os>_<cpu>.inline.hpp" and
>> "atomic_<os>_<cpu>.inline.hpp" via "atomic.inline.hpp"
>>
>> src/os_cpu/linux_x86/vm/orderAccess_linux_x86.inline.hpp
>> src/os_cpu/bsd_x86/vm/orderAccess_bsd_x86.inline.hpp
>> src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp
>>
>> - use os::is_MP() without including os.hpp
>>
>> The following fix was a little more cumbersome:
>>
>> src/os/bsd/vm/os_bsd.hpp
>> src/os/linux/vm/os_linux.hpp
>>
>> - The methods "set_suspended()" and "clear_suspended()" use
>> Atomic::cmpxchg without "atomic.inline.hpp" being included.
>> Unfortunately "os_bsd.hpp" and "os_linux.hpp" are not self-contained
>> files. They are themselves included into "os.hpp" right into the class
>> definition of the "os" class. It is therefore not possible to simply
>> include "atomic.inline.hpp" into "os_bsd.hpp" and "os_linux.hpp".
>>
>> - The natural solution was to transfer the implementations of
>> "set_suspended()" and "clear_suspended()" to the corresponding
>> "os_bsd.inline.hpp" and "os_linux.inline.hpp" files. Unfortunately
>> this is not trivial as well, because both methods use preprocessor
>> defined constants from their enclosing class "SuspendResume" which are
>> defined right before there definition and undefined right thereafter.
>> They are therefore not available in "os_bsd.inline.hpp" and
>> "os_linux.inline.hpp".
>>
>> - The solution for the last problem is to change the mentioned
>> preprocessor constants into an C++ enumeration which also results in a
>> nicer overall code. The drawback is that all the users of the
>> preprocessor constants have to be updated to use the new, qualified
>> enum-values (i.e. 'os::Bsd::SuspendResume::SR_CONTINUE' instead of
>> 'SR_CONTINUE'). It's a little more verbose but definitely a lot
>> cleaner this way.
>>
>> I also did the following clean-up because I touched the corresponding
>> files anyway:
>>
>> src/os_cpu/windows_x86/vm/orderAccess_windows_x86.inline.hpp
>>
>> - remove "#pragma warning(disable: 4035)" because there don't seem to
>> be any missing return statement in this file anymore
>>
>> src/os/bsd/vm/os_bsd.inline.hpp
>> src/os/linux/vm/os_linux.inline.hpp
>> src/os/solaris/vm/os_solaris.inline.hpp
>> src/os/windows/vm/os_windows.inline.hpp
>>
>> - include both "atomic.hpp" and "atomic.inline.hpp". This is
>> unnecessary, because "XXX.inline.hpp" always includes "XXX.hpp"
>>
>> Finally, I've also updated the copyright line to 2013 in every file
>> touched by this change.
>