> On Aug 20, 2018, at 10:06 PM, David Holmes <david.hol...@oracle.com> wrote:
>
> 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++.
currently our build system supports only 1-1 relation b/w tests lib/exec and
source file; most of native libraries in vmTestbase have more than one source
file. to work that around, we introduced a .c file for each library and theses
.c files include all other required .c files.
yes, a .c file will be compiled as C and a .cpp file as C++, but as we have .c
files which include .c files, moving only part of .c files, we will end up w/
.c files being included into both .c and .cpp files and thus compiled as both C
and C++. let's take
test/hotspot/jtreg/vmTestbase/nsk/jvmti/scenarios/hotswap/HS301/hs301t004, if
we want its native part to be compiled as C++, we rename libhs301t004.c to
libhs301t004.cpp, but libhs301t004.c includes 11 other files, there only one is
a test specific fill and all the other are used by other tests and included by
their .c files. so we have 10 files which now will be compiled by both C and
C++ 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.
>
> Sure but JCK comes into play much later and I'd rather spot issues during
> developer testing than promotion testing.
agree, but if we want to verify how C/C++ linked libraries work w/ hotspot, it
should be done by the tests specifically written for that purpose rather that
relaying on indirect coverage from other tests, and I guess it hasn't been done
as we always relied on JCK to test that.
>
>> 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.
Cheers,
-- Igor
>
> Thanks,
> David
>
>> 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/%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
