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

Thomas Stüfe thomas.stuefe at gmail.com
Thu Mar 22 05:52:55 UTC 2018

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

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 nativeInstppc.hpp or does just removing it from interpreterRTppc.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/interpreterRTs390.cpp --- a/src/hotspot/cpu/s390/interpreterRTs390.cpp Wed Mar 21 08:37:04 2018 +0100 +++ b/src/hotspot/cpu/s390/interpreterRTs390.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); fpargnr = 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 interpreterframestackdirection() to target specific hpp files (even though they're all -1), pdlastframe to thread.cpp because there isn't a thread.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

More information about the hotspot-dev mailing list