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. 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.
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? Thanks, — Igor On Aug 20, 2018, at 6:43 PM, David Holmes <david.hol...@oracle.com <mailto:david.hol...@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/~iignatyev//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 >> <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/~iignatyev//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/~iignatyev//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/~iignatyev//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
