[jdk11u-dev] RFR: 8299254: Support dealing with standard assert macro [v2]

[Antonio Vieiro]

On Tue, 16 Jul 2024 10:36:14 GMT, George Adams <gdams at openjdk.org> wrote:

>> Antonio Vieiro has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reorganization of include files as per review
>
> test/hotspot/gtest/gc/shared/test_memset_with_concurrent_readers.cpp line 27:
> 
>> 25: #include "gc/shared/memset_with_concurrent_readers.hpp"
>> 26: #include "utilities/globalDefinitions.hpp"
>> 27: #include "unittest.hpp"
> 
> Suggestion:

Done in commit, thanks!

> test/hotspot/gtest/gc/shared/test_memset_with_concurrent_readers.cpp line 32:
> 
>> 30: #include <string.h>
>> 31: #include <sstream>
>> 32: #include "utilities/vmassert_reinstall.hpp"
> 
> Suggestion:
> 
> #include "utilities/vmassert_reinstall.hpp"
> 
> #include "unittest.hpp"

Done in commit. Thanks!

> test/hotspot/gtest/jfr/test_networkUtilization.cpp line 45:
> 
>> 43: #include "utilities/growableArray.hpp"
>> 44: 
>> 45: #include "unittest.hpp"
> 
> Suggestion:

Done in commit.

> test/hotspot/gtest/unittest.hpp line 31:
> 
>> 29: 
>> 30: #define GTEST_DONT_DEFINE_TEST 1
>> 31: #include "gtest/gtest.h"
> 
> Suggestion:
> 
> #include "utilities/vmassert_uninstall.hpp"
> #include "gtest/gtest.h"

Done in commit.

> test/hotspot/gtest/unittest.hpp line 34:
> 
>> 32: #include "utilities/vmassert_reinstall.hpp"
>> 33: 
>> 34: // gtest/gtest.h includes assert.h which will define the assert macro, but hotspot has its
> 
> This block needs removing. See https://github.com/openjdk/jdk17u-dev/pull/2689/files#diff-ff1dd449619fcf441d7a63b01c6c4b6bf3e6fd5dc1149ffa48f38d5e6d82e5baL63-L74

Thanks!

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

PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2842#discussion_r1680652505
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2842#discussion_r1680652745
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2842#discussion_r1680652024
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2842#discussion_r1680651822
PR Review Comment: https://git.openjdk.org/jdk11u-dev/pull/2842#discussion_r1680653226