RFR: 8259894: refactor parts of jvm.h into jvm_io.h and jvm_constants.h [v2]

Ioi Lam iklam at openjdk.java.net
Thu Jan 21 03:09:17 UTC 2021


On Wed, 20 Jan 2021 05:51:11 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   David Holmes comments
>
> src/hotspot/share/include/jvm_constants.h line 8:
> 
>> 6:  * under the terms of the GNU General Public License version 2 only, as
>> 7:  * published by the Free Software Foundation.  Oracle designates this
>> 8:  * particular file as subject to the "Classpath" exception as provided
> 
> Classpath exception is wrong here - that is only for (mainly Java) sources that applications may link against. It's existing use in jvm.h is incorrect.

I think the classpath exception is intended for other JVM implementations that might include jvm.h. I'll leave it as is for now.

> src/hotspot/share/include/jvm_constants.h line 26:
> 
>> 24:  */
>> 25: 
>> 26: #ifndef _JAVASOFT_JVM_CONSTANTS_H_
> 
> We don't need to keep the legacy JAVASOFT defines.

Changed to `_JVM_CONSTANTS_H_` and `_JVM_IO_H_`.

> src/hotspot/share/logging/logSelection.cpp line 25:
> 
>> 23:  */
>> 24: #include "precompiled.hpp"
>> 25: #include "jvm.h"
> 
> Not jvm_io.h ??

Changed to jvm_io.h

> src/hotspot/share/include/jvm.h line 34:
> 
>> 32: #include "jvm_md.h"
>> 33: #include "jvm_constants.h"
>> 34: #include "jvm_io.h"
> 
> Note that the comment block starting at line 40 needs to be updated now you have refactored things.

I fixed the comments.

> src/hotspot/share/classfile/javaClasses.cpp line 26:
> 
>> 24: 
>> 25: #include "precompiled.hpp"
>> 26: #include "jvm.h"
> 
> Does this really need jvm.h or just jvm_constants.h and jvm_io.h?

jvm.h is needed because this file uses the JVM_XXX() functions.

> src/hotspot/share/classfile/vmIntrinsics.cpp line 26:
> 
>> 24: 
>> 25: #include "precompiled.hpp"
>> 26: #include "jvm.h"
> 
> Does this really need jvm.h or just jvm_constants.h and jvm_io.h?

Changed to jvm_constants.h and jvm_io.h.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2154


More information about the hotspot-dev mailing list