RFR: 8324799: Use correct extension for C++ test headers
Coleen Phillimore
coleenp at openjdk.org
Wed Feb 28 14:29:54 UTC 2024
On Wed, 28 Feb 2024 01:18:50 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
> Please review this change that renames some test .h files to .hpp. These
> files contain C++ code and should be named accordingly. Some of them contain
> uses of NULL, which we change to nullptr.
>
> The renamed files are:
>
> test/hotspot/jtreg/vmTestbase/nsk/share/aod/aod.h
> test/hotspot/jtreg/vmTestbase/nsk/share/jni/jni_tools.h
> test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.h
> test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/JVMTITools.h
> test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h
>
> test/lib/jdk/test/lib/jvmti/jvmti_thread.h
> test/lib/jdk/test/lib/jvmti/jvmti_common.h
> test/lib/native/testlib_threads.h
>
> The #include updates were performed mostly mechanically, and builds would fail
> if there were mistakes. The exception is test
> test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp,
> which was added after I'd done the mechanical update, so was updated by hand.
>
> The copyright updates were similarly performed mechanically. All but a handful
> of the including files have already had their copyrights updated recently,
> likely as part of JDK-8324681.
>
> Thus, the only "interesting" changes are to the renamed files.
>
> Testing: mach5 tier1
I asked for these to be done together since it's the same scrolling for each of these.
I added suggestions, but please feel free to ignore them since it would be prudent pull down and to rebuild after making them and it's not worth it.
test/hotspot/jtreg/serviceability/jvmti/DynamicCodeGenerated/libDynamicCodeGenerated.cpp line 25:
> 23:
> 24: #include <string.h>
> 25: #include <jvmti.h>
Is jvmti.h a C file? Sometimes it has <> sometimes it's included with "". I don't expect to see a change. I was just curious.
test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp line 26:
> 24: #include <jni.h>
> 25: #include <jvmti.h>
> 26: #include <jvmti_common.hpp>
Should this be in quotes rather than <> ?
test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/libGetThreadStateMountedTest.cpp line 26:
> 24: #include <jni.h>
> 25: #include <jvmti.h>
> 26: #include <jvmti_common.hpp>
Another.
test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassStatus/getclstat007/getclstat007.cpp line 28:
> 26: #include "jvmti.h"
> 27: #include "agent_common.hpp"
> 28: #include "JVMTITools.hpp"
There's a lower case jvmti_tools.hpp and an uppercase JVMTITools.hpp now. Maybe someone could do a cleanup of these tests (please!)
test/hotspot/jtreg/vmTestbase/nsk/jvmti/RetransformClasses/retransform004/retransform004.cpp line 29:
> 27: #include <jvmti.h>
> 28: #include "agent_common.hpp"
> 29: #include <jvmti_tools.hpp>
Suggestion:
#include "jvmti_tools.hpp"
-------------
Marked as reviewed by coleenp (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/18035#pullrequestreview-1906370538
PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506026145
PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506031049
PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506031853
PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506038573
PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506045478
More information about the serviceability-dev
mailing list