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

[David Holmes]

Hi Igor,

On 21/08/2018 1:58 PM, Igor Ignatev wrote:
> Hi David,
> 
> It is necessary for vmTestbase tests (if we of course want to clean them 
> up) as they are coupled, and moving only part of them will require (non 
> trivial) updates in the rest of code (or at least in the shared part) to 
> properly serve both compilers.

Can you elaborate on the problem please as I don't see why the 
vmTestbase tests are special here.

I would expect a .c file to be compiled as C and a .cpp to be compiled 
as C++.

> It absolutely not necessary for other 
> tests, but I’d prefer to have some level of unification in the test 
> base. That being said, I agree I might went too far and moved all the 
> tests which might or might not compromise their purpose. I personally 
> don’t think we should relay on our jtreg testbase to verify if C linked 
> libraries can be used with hotspot. it must be verified by JCK which 
> should be compiled/linked with carefully chosen compilers/linkers and 
> their flags.

Sure but JCK comes into play much later and I'd rather spot issues 
during developer testing than promotion testing.

> 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 :)

Thanks,
David

> Thanks,
> — Igor
> 
> On Aug 20, 2018, at 6:43 PM, David Holmes <david.holmes at oracle.com 
> <mailto:david.holmes at oracle.com>> wrote:
> 
>> Hi Igor,
>>
>> On 21/08/2018 8:59 AM, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8209611/webrev.01/index.html 
>>> <http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.01/index.html>
>>>> 11160 lines changed: 879 ins; 61 del; 10220 mod;
>>> Hi all,
>>> could you please review the patch which moves all hotspot native test 
>>> code to C++? this will guarantee that we always use C++ compilers for 
>>> them (as an opposite to either C or C++ compiler depending on 
>>> configuration), as a result we will be able to get rid of 
>>> JNI_ENV_ARG[1] macros, perform other clean ups and improve overall 
>>> quality of the test code.
>>
>> Sorry but I don't see why this is necessary. If people want to be able 
>> to write C++ tests then we should enable that if not currently 
>> enabled, but I don't see why everything should be forced to C++. What 
>> if we did something that broke the C linkage and we didn't detect it 
>> because we only ever tested C++?
>>
>> Was the motivation previously discussed somewhere?
>>
>> Thanks,
>> David
>>
>>> the patch consists of two parts:
>>>  - automatic: renaming .c files to .cpp, updating #include, changing 
>>> JNI/JVMTI calls
>>>  - semi-manual: adding extern "C" , fixing a number of compiler 
>>> warnings (mostly types inconsistency), updating makefiles
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8209611
>>> webrevs:
>>>  - automatic: 
>>> http://cr.openjdk.java.net/~iignatyev//8209611/webrev.00/index.html 
>>> <http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.00/index.html>
>>>> 9394 lines changed: 0 ins; 0 del; 9394 mod;
>>>  - semi-manual: 
>>> http://cr.openjdk.java.net/~iignatyev//8209611/webrev.0-1/index.html 
>>> <http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.0-1/index.html>
>>>> 1899 lines changed: 879 ins; 61 del; 959 mod
>>>  - whole: 
>>> http://cr.openjdk.java.net/~iignatyev//8209611/webrev.01/index.html 
>>> <http://cr.openjdk.java.net/%7Eiignatyev//8209611/webrev.01/index.html>
>>>> 11160 lines changed: 879 ins; 61 del; 10220 mod;
>>> testing: all hotspot tests + tier[1-3]
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8209547
>>> Thanks,
>>> -- Igor