[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-compiler-dev
Subject: Re: RFR: JDK-8216261: Javap ignores default modifier on interfaces
From: Vicente Romero <vicente.romero () oracle ! com>
Date: 2019-06-05 14:33:26
Message-ID: 5dd42de5-7f2b-4bb3-0d37-65e2b4715333 () oracle ! com
[Download RAW message or body]
OK I will fix that before pushing,
Thanks for the review,
Vicente
On 6/5/19 9:46 AM, Jonathan Gibbons wrote:
>
> That's OK, but it might be slightly better to do:
>
> 71 String output = new JavapTask(tb)
> 72 .options("-p", \
> testClassesPath.resolve(this.getClass().getSimpleName() + \
> "$SimpleInterface.class").toString()) 73 .run()
> + .writeAll()
> 74 .getOutput(Task.OutputKind.DIRECT);
> On 6/5/19 6:43 AM, Vicente Romero wrote:
> > Hi Jon,
> >
> > Thanks for your comments, what about [1]?
> >
> > Vicente
> >
> > [1] http://cr.openjdk.java.net/~vromero/8216261/webrev.01/
> >
> > On 6/4/19 7:24 PM, Jonathan Gibbons wrote:
> > >
> > >
> > > On 06/04/2019 04:08 PM, Vicente Romero wrote:
> > > > Please review fix for [1] at [2]. This is an enhancement request
> > > > asking to make javap show if a method has the default modifier or not.
> > > >
> > > > Thanks,
> > > > Vicente
> > > >
> > > > [1] https://bugs.openjdk.java.net/browse/JDK-8216261
> > > > [2] http://cr.openjdk.java.net/~vromero/8216261/webrev.00/
> > >
> > > The main source change looks OK,
> > >
> > > The test could be improved:
> > >
> > > 1. If the test fails, nothing is printed to show what went wrong
> > > because a basic assertion of "incorrect output". What was the
> > > incorrect output? It should *always* be the case for *all* tests
> > > that a test should try and give as much output as is reasonable when
> > > the test fails, to give the person analyzing the test failure as
> > > much as possible.
> > >
> > > 2. (Less important) Personally, I think it is bad style to
> > > construct pathnames with string bashing. (I've been burnt too
> > > often!) Generally it is better to use the File or Path API to
> > > construct filenames, and then use .toString().
> > >
> > > -- Jon
> >
[Attachment #3 (text/html)]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
OK I will fix that before pushing,<br>
<br>
Thanks for the review,<br>
Vicente<br>
<br>
<div class="moz-cite-prefix">On 6/5/19 9:46 AM, Jonathan Gibbons
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:5b286fe1-c7ee-165d-c1b9-750ad9898290@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<p>That's OK, but it might be slightly better to do:</p>
<pre style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; \
font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: \
auto; text-align: start; text-indent: 0px; text-transform: none; widows: auto; \
word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; \
background-color: rgb(238, 238, 238); text-decoration: none;"> 71 String \
output = new JavapTask(tb) 72 .options("-p", \
testClassesPath.resolve(this.getClass().getSimpleName() + \
"$SimpleInterface.class").toString()) 73 .run()
+ .writeAll()
74 .getOutput(Task.OutputKind.DIRECT);</pre>
<div class="moz-cite-prefix">On 6/5/19 6:43 AM, Vicente Romero
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:21b83c7d-c753-f153-f9e7-90a8a672c952@oracle.com">Hi
Jon, <br>
<br>
Thanks for your comments, what about [1]? <br>
<br>
Vicente <br>
<br>
[1] <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~vromero/8216261/webrev.01/"
moz-do-not-send="true">http://cr.openjdk.java.net/~vromero/8216261/webrev.01/</a>
<br>
<br>
On 6/4/19 7:24 PM, Jonathan Gibbons wrote: <br>
<blockquote type="cite"> <br>
<br>
On 06/04/2019 04:08 PM, Vicente Romero wrote: <br>
<blockquote type="cite">Please review fix for [1] at [2]. This
is an enhancement request asking to make javap show if a
method has the default modifier or not. <br>
<br>
Thanks, <br>
Vicente <br>
<br>
[1] <a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8216261"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8216261</a>
<br>
[2] <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~vromero/8216261/webrev.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~vromero/8216261/webrev.00/</a>
<br>
</blockquote>
<br>
The main source change looks OK, <br>
<br>
The test could be improved: <br>
<br>
1. If the test fails, nothing is printed to show what went
wrong because a basic assertion of "incorrect output". What
was the incorrect output? It should *always* be the case for
*all* tests that a test should try and give as much output as
is reasonable when the test fails, to give the person
analyzing the test failure as much as possible. <br>
<br>
2. (Less important) Personally, I think it is bad style to
construct pathnames with string bashing. (I've been burnt too
often!) Generally it is better to use the File or Path API to
construct filenames, and then use .toString(). <br>
<br>
-- Jon <br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic