RFR(M/L) : 8209611 : use C++ compiler for hotspot tests

[Igor Ignatyev]

Hi David,

thanks for looking at the webrev and all your comments. my answers are inlined.

enjoy your vacation!

-- Igor 

> On Aug 21, 2018, at 9:28 PM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Hi Igor,
> 
> On 22/08/2018 9:04 AM, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev//8209611/webrev.02/index.html <http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.02/index.html> is a new version of patch, which moves only vmTestbase tests.
> 
> This seems okay in principle. I didn't verify all the mechanical changes individually.
> 
> make/common/TestFilesCompilation.gmk
> 
> I suspect you can just find all .c and .cpp files and process them together, rather than duplicate all the logic. But I'll leave that to Magnus to comment on.
I had to duplicate this foreach cycle to specify a correct TOOLCHAIN, w/o it linking fails on linux-slowdebug. 
> 
> make/test/JtregNativeHotspot.gmk
> 
> + BUILD_HOTSPOT_JTREG_LIBRARIES_CFLAGS := -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS
> 
> Just a note that defining these is obsolete once we use C11/C++11 - which I think is an intent for JDK 12 if possible.
initially I wanted to use <cinttypes> instead of <inttype.h> + -D__STDC_FORMAT_MACROS, but solaris studio apparently doesn't have cinttypes header file.

hotspot makefiles also use -D__STDC_, so I hope both places will be fixed together.

> 
> test/hotspot/jtreg/vmTestbase/gc/g1/unloading/libdefine.cpp
> 
>  #ifdef __cplusplus
> ! #define JNI_ENV_ARG(x, y) y
>  #define JNI_ENV_PTR(x) x
>  #else
>  #define JNI_ENV_ARG(x, y) x , y
>  #define JNI_ENV_PTR(x) (*x)
>  #endif
> 
> If this is now a C++ source file why do you a still have the code that allows for either C or C++? Is this stuff that will be cleaned up in the later RFE?
yes, 8209547 will clean up JNI_ENV_ARG macros.

> 
> ---
> 
> The switch to C++ means a lot of code now needs:
> 
> + #ifdef __cplusplus
> + extern "C" {
> + #endif

that's true, and this patch adds 'extern "C"' very conservatively (basically to everywhere). this, if needed, can be cleaned up later.

> 
> I still have to wonder whether it better to leave these as C tests and only convert to C++ as and when needed ... seems like so much work.
> 
> Anyway I'm off on vacation after today so I'll leave it to others to take this up.
> 
> Thanks,
> David
> 
>> Thanks,
>> -- Igor
>>> On Aug 20, 2018, at 11:07 PM, Igor Ignatyev <igor.ignatyev at oracle.com <mailto:igor.ignatyev at oracle.com>> wrote:
>>> 
>>>>> It has been discussed (not widely enough and I accept that) in 8209547 and the related email thread b/w JC(cc'ed) and myself.
>>>>> as I said, I might went a way too far, so I'll revert changes in the non-vmTestbase tests and made appropriate changes in makefiles. what do you think?
>>>> 
>>>> I think I need to see what you mean exactly :)
>>> sure, it will take some time for me to do that, hopefully will upload new webrevs tomorrow morning PT. but the basic idea is to leave files in test/hotspot/jtreg/compiler, runtime, gc, native_sanity, serviceability, testlibrary as .c files, exactly as they were before, and restore corresponding filenames in make/test/JtregNativeHotspot.gmk.