RFR (L, tedious) 8199809: Don't include frame.inline.hpp and other.inline.hpp from .hpp files

[Thomas Stüfe]

On Wed, Mar 21, 2018 at 11:34 PM, David Holmes <david.holmes at oracle.com>
wrote:

> <trimming>
>
> On 22/03/2018 1:35 AM, Thomas Stüfe wrote:
>
>> On Wed, Mar 21, 2018 at 2:02 PM, David Holmes <david.holmes at oracle.com
>>    On 21/03/2018 10:39 PM, coleen.phillimore at oracle.com
>>         Hm so I need to add the #include for macroAssembler.hpp
>>         somewhere new like nativeInst_ppc.hpp or does just removing it
>>         from interpreterRT_ppc.hpp fix the problem?
>>
>>
>>     Whatever code is in the included platform specific header still
>>     needs to ensure the definitions that it needs have been included. If
>>     those are shared files then you may just be able to move them into
>>     the shared cpp file, but any platform specific headers must still be
>>     included in the platform specific headers.
>>
>>
>> I disagree in this particular case. In my opinion, headers whose purpose
>> is to be included into class declarations should not include other headers.
>>
>
> ??? If the code you are including relies on things from other header files
> then you have no choice else it won't compile!
>
>
Is this a misunderstanding? I am not saying not to provide the
dependencies. But they cannot be provided from within this header, if this
header gets dropped in right in the middle of a class definition (like e.g.
os_aix.hpp), right?

If, for an easy example, my header uses pthread_t, I cannot just simply
include pthread.h, because then all declarations of pthread.h appear in
class scope in the surrounding class. So I have to make sure
the platform header is included elsewhere, preferably at the start of the
surrounding header, or of the cpp file.

Thomas




> David
>
> Thanks, Thomas
>>
>>     David
>>     -----
>>
>>         thanks,
>>         Coleen
>>
>>
>>
>>             Thanks, Thomas
>>
>>
>>
>>
>>
>>             On Wed, Mar 21, 2018 at 11:41 AM, Thomas Stüfe
>>             <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>
>>             <mailto:thomas.stuefe at gmail.com
>>
>>             <mailto:thomas.stuefe at gmail.com>>> wrote:
>>
>>                  Hi Coleen,
>>
>>                  linuxs390 needs this:
>>
>>                  - .../source $ hg diff
>>                  diff -r daf3abb9031f
>>             src/hotspot/cpu/s390/interpreterRT_s390.cpp
>>                  --- a/src/hotspot/cpu/s390/interpreterRT_s390.cpp
>>             Wed Mar 21
>>                  08:37:04 2018 +0100
>>                  +++ b/src/hotspot/cpu/s390/interpreterRT_s390.cpp
>>             Wed Mar 21
>>                  11:12:03 2018 +0100
>>                  @@ -65,7 +65,7 @@
>>                   }
>>
>>                   // Implementation of SignatureHandlerGenerator
>>                             -InteprerterRuntime::Signature
>> HandlerGenerator::SignatureHandlerGenerator(
>>
>>                             +InterpreterRuntime::Signature
>> HandlerGenerator::SignatureHandlerGenerator(
>>
>>                       const methodHandle& method, CodeBuffer* buffer) :
>>                  NativeSignatureIterator(method) {
>>                     _masm = new MacroAssembler(buffer);
>>                     _fp_arg_nr = 0;
>>
>>                  (typo). Otherwise it builds fine.
>>
>>                  I'm getting build errors on AIX which are a bit more
>>             complicated,
>>                  still looking..
>>
>>                  Thanks, Thomas
>>
>>
>>                  On Wed, Mar 21, 2018 at 1:08 AM,
>>             <coleen.phillimore at oracle.com
>>             <mailto:coleen.phillimore at oracle.com>
>>                  <mailto:coleen.phillimore at oracle.com
>>
>>             <mailto:coleen.phillimore at oracle.com>>> wrote:
>>
>>                      Summary: Remove frame.inline.hpp,etc from header
>>             files and
>>                      adjust transitive includes.
>>
>>                      Tested with mach5 tier1 on Oracle platforms:
>> linux-x64,
>>                      solaris-sparc, windows-x64.  Built with open-only
>>             sources
>>                      using --disable-precompiled-headers on linux-x64,
>>             built with
>>                      zero (also disable precompiled headers). Roman
>>             built with
>>                      aarch64, and have request to build ppc, etc.
>>      (Please test
>>                      this patch!)
>>
>>                      Semi-interesting details:  moved
>>             SignatureHandlerGenerator
>>                      constructor to cpp file, moved
>>                      interpreter_frame_stack_direction() to target
>>             specific hpp
>>                      files (even though they're all -1), pd_last_frame to
>>                      thread_<os_cpu>.cpp because there isn't a
>>                      thread_<os_cpu>.inline.hpp file, lastly moved
>>                      InterpreterRuntime::LastFrameAccessor into
>>                      interpreterRuntime.cpp file, and a few other
>>             functions moved
>>                      in shared code.
>>
>>                      This is the last of this include file technical
>>             debt cleanup
>>                      that I'm going to do.  See bug for more information.
>>
>>                      open webrev at
>>             http://cr.openjdk.java.net/~coleenp/8199809.01/webrev
>>             <http://cr.openjdk.java.net/~coleenp/8199809.01/webrev>
>>                                 <http://cr.openjdk.java.net/%7
>> Ecoleenp/8199809.01/webrev
>>             <http://cr.openjdk.java.net/%7Ecoleenp/8199809.01/webrev>>
>>                      bug link
>>             https://bugs.openjdk.java.net/browse/JDK-8199809
>>             <https://bugs.openjdk.java.net/browse/JDK-8199809>
>>                      <https://bugs.openjdk.java.net/browse/JDK-8199809
>>             <https://bugs.openjdk.java.net/browse/JDK-8199809>>
>>
>>                      I'll update the copyrights when I commit.
>>
>>                      Thanks,
>>                      Coleen
>>
>>
>>
>>
>>
>>