[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR (L) 8210481: Remove #ifdef cplusplus from vmTestbase
From: JC Beyler <jcbeyler () google ! com>
Date: 2018-09-18 2:38:08
Message-ID: CAF9BGBynJdkcDghpPiYwjQVfjA=2PzReegC+Uw2Vb0kg2H1RCg () mail ! gmail ! com
[Download RAW message or body]
Thanks all for the reviews, I know it was a bit big and tedious; this is
pushed :)
Jc
On Mon, Sep 17, 2018 at 12:46 AM <serguei.spitsyn@oracle.com> wrote:
> Hi Jc,
>
> It looks good to me.
> Thank you for taking care about it!
>
> Thanks,
> Serguei
>
>
>
> On 9/14/18 9:50 AM, JC Beyler wrote:
>
> Hi all,
>
> Could I get a review for the following webrev:
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8210481
>
> @Alex: I know we had said generally 50 files max but this one is really
> straight forward; if you still prefer 50 files, let me know and I'll cut it
> up into chunks.
>
> @all: A big webrev this time, to which I apologize but it is the last
> macro removal webrev from me in a while (there still is
> https://bugs.openjdk.java.net/browse/JDK-8210728 but I'm giving everyone
> a rest on macro removal and will work on a few code refactoring I promised
> to do ;-)).
>
> To perhaps help reviewing it:
> This webrev removes two #ifdef per file touched:
>
> -#ifdef __cplusplus
> extern "C" {
> -#endif
>
> and
>
> -#ifdef __cplusplus
> }
> -#endif
>
> -> The patch only has deletions (4 per file), no insertions
> -> I double checked that all files only have exactly those removals and no
> other removals EXCEPT:
>
> http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h.udiff.html
> which is still defining the NSK_STUB macros but was doing it for C/C++
> styles.
> -> Last easier check for reviewers:
> - Look for deletions but that are not exactly "-#endif" or "-#ifdef
> __cplusplus"
> grep "^-[^-]" jdk12-jvmti12.patch | grep -v "^-#endif$" | grep -v
> "^-#ifdef __cplusplus$"
>
> That command provides only the lines from the nsk_tools.h file:
> $ grep "^-[^-]" jdk12-jvmti12.patch | grep -v "^-#endif$" | grep -v
> "^-#ifdef __cplusplus$"
> -#define NSK_CPP_STUB1(Func,env) (*env)->Func(env)
> -#define NSK_CPP_STUB2(Func,env,a) (*env)->Func(env,a)
> -#define NSK_CPP_STUB3(Func,env,a,b) (*env)->Func(env,a,b)
> -#define NSK_CPP_STUB4(Func,env,a,b,c) (*env)->Func(env,a,b,c)
> -#define NSK_CPP_STUB5(Func,env,a,b,c,d) (*env)->Func(env,a,b,c,d)
> -#define NSK_CPP_STUB6(Func,env,a,b,c,d,e) (*env)->Func(env,a,b,c,d,e)
> -#define NSK_CPP_STUB7(Func,env,a,b,c,d,e,f) (*env)->Func(env,a,b,c,d,e,f)
> -#define NSK_CPP_STUB8(Func,env,a,b,c,d,e,f,g)
> (*env)->Func(env,a,b,c,d,e,f,g)
> -#define NSK_CPP_STUB9(Func,env,a,b,c,d,e,f,g,h)
> (*env)->Func(env,a,b,c,d,e,f,g,h)
> -#ifndef NSK_CPP_STUBS_ENFORCE_C
> -#undef NSK_CPP_STUB1
> -#undef NSK_CPP_STUB2
> -#undef NSK_CPP_STUB3
> -#undef NSK_CPP_STUB4
> -#undef NSK_CPP_STUB5
> -#undef NSK_CPP_STUB6
> -#undef NSK_CPP_STUB7
> -#undef NSK_CPP_STUB8
> -#undef NSK_CPP_STUB9
>
> Thanks for the reviews,
> Jc
>
>
>
--
Thanks,
Jc
[Attachment #3 (text/html)]
<div dir="ltr">Thanks all for the reviews, I know it was a bit big and tedious; this \
is pushed :)<br><div>Jc</div></div><br><div class="gmail_quote"><div dir="ltr">On \
Mon, Sep 17, 2018 at 12:46 AM <<a \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
Hi Jc,<br>
<br>
It looks good to me.<br>
Thank you for taking care about it!<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<br>
<div class="m_-170153123098883880moz-cite-prefix">On 9/14/18 9:50 AM, JC Beyler \
wrote:<br> </div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi all,
<div><br>
</div>
<div>Could I get a review for the following
webrev:</div>
<div>Webrev: <a \
href="http://cr.openjdk.java.net/%7Ejcbeyler/8210481/webrev.00/" \
target="_blank">http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/</a></div> \
<div>Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8210481" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8210481</a></div> <div><br>
</div>
<div>@Alex: I know we had said generally 50
files max but this one is really straight
forward; if you still prefer 50 files, let
me know and I'll cut it up into chunks.<br>
</div>
<div><br>
</div>
<div>@all: A big webrev this time, to which I
apologize but it is the last macro removal
webrev from me in a while (there still is <a \
href="https://bugs.openjdk.java.net/browse/JDK-8210728" \
target="_blank">https://bugs.openjdk.java.net/browse/JDK-8210728</a> but I'm \
giving everyone a rest on macro removal and will work on a few code
refactoring I promised to do ;-)).</div>
<div><br>
</div>
<div>To perhaps help reviewing it:</div>
<div>This webrev removes two #ifdef per file
touched:</div>
<div><br>
</div>
<div>-#ifdef __cplusplus<br>
</div>
<div>
<div>
<div> extern "C" {</div>
<div>-#endif</div>
</div>
<div><br>
</div>
and</div>
<div><br>
</div>
<div>
<div>-#ifdef __cplusplus<br>
</div>
<div> }</div>
<div>-#endif</div>
<div dir="ltr" \
class="m_-170153123098883880m_-4745492194429044958gmail_signature"> <div dir="ltr">
<div><br>
</div>
<div>-> The patch only has deletions
(4 per file), no insertions</div>
<div>-> I double checked that all
files only have exactly those removals
and no other removals EXCEPT:</div>
<div> <a \
href="http://cr.openjdk.java.net/%7Ejcbeyler/8210481/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h.udiff.html" \
target="_blank">http://cr.openjdk.java.net/~jcbeyler/8210481/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h.udiff.html</a></div>
<div>which is still defining the
NSK_STUB macros but was doing it for
C/C++ styles.<br>
</div>
<div>-> Last easier check for
reviewers:</div>
<div> - Look for deletions but that
are not exactly "-#endif" or \
"-#ifdef __cplusplus"</div>
<div>grep "^-[^-]" jdk12-jvmti12.patch |
grep -v "^-#endif$" | grep -v
"^-#ifdef __cplusplus$"<br>
</div>
<div><br>
</div>
<div>That command provides only the
lines from the nsk_tools.h file:</div>
<div>
<div>$ grep "^-[^-]"
jdk12-jvmti12.patch | grep -v
"^-#endif$" | grep -v "^-#ifdef
__cplusplus$"</div>
<div>-#define NSK_CPP_STUB1(Func,env)
(*env)->Func(env)</div>
<div>-#define
NSK_CPP_STUB2(Func,env,a)
(*env)->Func(env,a)</div>
<div>-#define
NSK_CPP_STUB3(Func,env,a,b)
(*env)->Func(env,a,b)</div>
<div>-#define
NSK_CPP_STUB4(Func,env,a,b,c)
(*env)->Func(env,a,b,c)</div>
<div>-#define
NSK_CPP_STUB5(Func,env,a,b,c,d)
(*env)->Func(env,a,b,c,d)</div>
<div>-#define
NSK_CPP_STUB6(Func,env,a,b,c,d,e)
(*env)->Func(env,a,b,c,d,e)</div>
<div>-#define
NSK_CPP_STUB7(Func,env,a,b,c,d,e,f)
(*env)->Func(env,a,b,c,d,e,f)</div>
<div>-#define
NSK_CPP_STUB8(Func,env,a,b,c,d,e,f,g)
(*env)->Func(env,a,b,c,d,e,f,g)</div>
<div>-#define
NSK_CPP_STUB9(Func,env,a,b,c,d,e,f,g,h)
(*env)->Func(env,a,b,c,d,e,f,g,h)</div>
<div>-#ifndef NSK_CPP_STUBS_ENFORCE_C</div>
<div>-#undef NSK_CPP_STUB1</div>
<div>-#undef NSK_CPP_STUB2</div>
<div>-#undef NSK_CPP_STUB3</div>
<div>-#undef NSK_CPP_STUB4</div>
<div>-#undef NSK_CPP_STUB5</div>
<div>-#undef NSK_CPP_STUB6</div>
<div>-#undef NSK_CPP_STUB7</div>
<div>-#undef NSK_CPP_STUB8</div>
<div>-#undef NSK_CPP_STUB9</div>
</div>
<div><br>
</div>
<div>Thanks for the reviews,<br>
</div>
<div>Jc</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" \
class="gmail_signature" data-smartmail="gmail_signature"><div \
dir="ltr"><div><br></div>Thanks,<div>Jc</div></div></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic