Re: RFR: 8217216: Launcher does not defend itself against LD_LIBRARY_PATH_64 (Solaris)
Thanks, Roger. Cheers, Henry > On Mar 1, 2019, at 10:27 AM, Roger Riggs wrote: > > Hi Henry, > > The change looks ok, Reviewed. > > (I'm not that familiar with Solaris). > > Thanks, Roger > > > > On 3/1/19 10:45 AM, Henry Jen wrote: >> Ping! >> >> Any thought on this webrev restore the way for Solaris LD_LIBRARY_PATH_64 >> handling? >> >> Cheers, >> Henry >> >> >>> On Feb 13, 2019, at 9:37 AM, Henry Jen wrote: >>> >>> Hi, >>> >>> Please review the webrev[1] for 8217216. The fix makes sure on Solaris, >>> when LD_LIBRARY_PATH_64 is set, we setup LD_LIBRARY_PATH based on that >>> value and unset LD_LIBRARY_PATH_64 in the relaunched process. >>> >>> Same approach was used before JDK-6367077, and the override is expected >>> behavior on Solaris 64-bit as stated in Solaris 64-bit Developer's Guide[2], >>> "The 64-bit dynamic linker's search path can be completely overridden using >>> the LD_LIBRARY_PATH_64 environment variable." >>> >>> Cheers, >>> Henry >>> >>> [1] http://cr.openjdk.java.net/~henryjen/8217216/0/webrev/ >>> [2] https://docs.oracle.com/cd/E19455-01/806-0477/dev-env-7/index.html >>> >
RFR: JDK-8214566 : --win-dir-chooser does not prompt if destination folder is not empty
Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). - Added custom action to check installation folder and display confirmation dialog to ask user to continue installation if destination folder is not empty. This is same behavior and confirmation message as for .exe. [1] https://bugs.openjdk.java.net/browse/JDK-8214566 [2] http://cr.openjdk.java.net/~almatvee/8214566/webrev.00/ Thanks, Alexander
Re: [13] RFR 8217254, 8217721: CompactNumberFormat() constructor does not comply with spec and format() method spec for IAEx is not complaint
Hi Nishit, Just one comment on j.t.CompactNumberFormat.java. At line 425, Null check can be done at the top of the method, as a parameter check, so that all the unnecessary "if-elseif" can be avoided. Others look good. Naoto On 3/6/19 3:56 AM, Nishit Jain wrote: Hi, Please review the fix for JDK-8217254 and JDK-8217721 Bug: https://bugs.openjdk.java.net/browse/JDK-8217254 https://bugs.openjdk.java.net/browse/JDK-8217721 Webrev: http://cr.openjdk.java.net/~nishjain/8217254_8217721/webrev.00/ Issue: The exception thrown by constructor and format() method was not compliant with the specification Fix: Updated the constructor and format method to throw exception as per the specification Regards, Nishit Jain
Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails to throw IllegalAccessException for final fields
Hi Mandy, The webrev has been updated with the new test: http://cr.openjdk.java.net/~afarley/8216558/webrev/ Note that I included a handful of small improvements, and that the final fields are all setAccessible by default, because (a) it seemed cleaner than adding a new control bit, and (b) nobody is meant to be changing final fields anyway. Open for review. Best Regards Adam Farley IBM Runtimes Mandy Chung wrote on 01/03/2019 17:50:49: > From: Mandy Chung > To: Adam Farley8 > Cc: core-libs-dev > Date: 01/03/2019 17:50 > Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > to throw IllegalAccessException for final fields > > Hi Adam, > > You can add a new test for this specific test case. > MethodHandlesGeneralTest is a general test that you either update > it to fit in existing testing framework or add a new test which > will be simpler for you. Please include the test in the webrev > for easier review. > > Mandy > > On 3/1/19 2:31 AM, Adam Farley8 wrote: > > Hi Mandy, > > > > Historically, the bigger the change I propose, the more months it takes > > the OpenJDK community to approve. > > > > I'm not sure I can justify adding an entire class to test a very > > specific case. > > > > Additionally, HasFields seems excessively complex. I don't understand > > why it feels the > > need to spend 34 lines of code parsing, processing, and storing 20 > > minimal variables it already has. > > > > Seems like an informative comment, followed by a littany of > > "cases.add..." would have been a simpler choice. > > > > Could you tell me if I'm missing something? > > > > Best Regards > > > > Adam Farley > > IBM Runtimes > > > > > > Mandy Chung wrote on 21/02/2019 17:37:54: > > > >> From: Mandy Chung > >> To: Adam Farley8 > >> Cc: core-libs-dev > >> Date: 21/02/2019 17:41 > >> Subject: Re: RFR: JDK-8216558: Lookup.unreflectSetter(Field) fails > >> to throw IllegalAccessException for final fields > >> > >> Hi Adam, > >> > >> On 2/14/19 3:16 AM, Adam Farley8 wrote: > >> > Hi Mandy, > >> > > >> > Apologies for the delay. > >> > >> Same here as I returned from vacation yesterday. > >> > >> > Could you review this cdiff as a proposal for the jtreg test? > >> > > >> > Made sense to modify the existing test set for MethodHandle rather than > >> > add a new one. > >> > >> Yes it'd be better if you extend the MethodHandlesGeneralTest and > >> MethodHandlesTest to test the write access. > >> > >> To add the test cases properly, MethodHandlesTest.HasFields > >> should be modified to include the read-only fields. It might > >> be cleaner to add a new HasReadOnlyFields class and maybe a new > >> TEST_SET_ACCESSIBLE bit that requests to call setAccessible(true) > >> and testSetter expects unreflectSetter to throw exception on > >> static final fields (possibly additional bits might be needed). > >> > >> Mandy > >> > > > > Unless stated otherwise above: > > IBM United Kingdom Limited - Registered in England and Wales with number > > 741598. > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU > Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
Hi Stuart,
According to Liskov substitution principle:
Subtype Requirement: Let ϕ ( x ) be a property provable about
objects x of type T. Then ϕ ( y ) should be true for objects y of type S
where S is a subtype of T.
Let ϕ ( x ) for objects x of type Iterable be: "x.iterator() may be
invoked multiple times, each time starting new iteration".
This clearly holds.
Does ϕ ( y ) hold for objects y of type IterableOnce? Clearly not.
In this respect Iterable should be a subtype of IterableOnce and foreach
loop should be retrofitted to work with IterableOnce.
What do you think?
Regards, Peter
On 3/1/19 3:43 AM, Stuart Marks wrote:
Hi all,
Please review and comment on this proposal to allow Stream instances
to be used in enhanced-for ("for-each") loops.
Abstract
Occasionally it's useful to iterate a Stream using a conventional
loop. However, the Stream interface doesn't implement Iterable, and
therefore streams cannot be used with the enhanced-for statement. This
is a proposal to remedy that situation by introducing a new interface
IterableOnce that is a subtype of Iterable, and then retrofitting the
Stream interface to implement it. Other JDK classes will also be
retrofitted to implement IterableOnce.
Full Proposal:
http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
Bug report:
https://bugs.openjdk.java.net/browse/JDK-8148917
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
Note, this changeset isn't ready to push yet. In particular, it has no
tests yet. However, the implementation is so simple that I figured I
should include it. Comments on the specification wording are also
welcome.
Thanks,
s'marks
Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
I don’t mean any offence, but I have to say, I strongly disagree with nearly
everything you’ve written below. To me, the idea of making a stream of integers
for a simple loop counter is hackish, confusing, verbose, and basically abusing
the stream concept. The only part I agree with is that it is an obvious
performance drawback as well. A counter and a stream of integers are
completely different concepts and should not be confused in this manner.
Scott
> On Mar 6, 2019, at 5:10 AM, Tagir Valeev wrote:
>
> Hello!
>
> By the way one of the painful code patterns in modern Java is `for(int
> i = 0; i newbies and prone to errors as the variable need to be repeated three
> times. Also the variable is not effectively final, despite it never
> changes within the loop body, so could have been considered as
> redeclared on every loop iteration (like in for-each). With the new
> proposal it's possible to write `for(int i : range(0, bound).boxed())`
> (assuming import static j.u.s.IntStream.range), which looks much
> better, though it has obvious performance drawback. Moving
> IterableOnce to BaseStream would enable to use `for(int i : range(0,
> bound))` which looks even better, though again we have plenty of
> garbage (but considerably less than in previous case!). I wonder
> whether Java could evolve to the point where such kind of code would
> be a recommended way to iterate over subsequent integer values without
> any performance handicap.
>
> With best regards,
> Tagir Valeev.
>
> On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks wrote:
>>
>> Hi all,
>>
>> Please review and comment on this proposal to allow Stream instances to be
>> used
>> in enhanced-for ("for-each") loops.
>>
>> Abstract
>>
>> Occasionally it's useful to iterate a Stream using a conventional loop.
>> However,
>> the Stream interface doesn't implement Iterable, and therefore streams
>> cannot be
>> used with the enhanced-for statement. This is a proposal to remedy that
>> situation by introducing a new interface IterableOnce that is a subtype of
>> Iterable, and then retrofitting the Stream interface to implement it. Other
>> JDK
>> classes will also be retrofitted to implement IterableOnce.
>>
>> Full Proposal:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>>
>> Bug report:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8148917
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>>
>> Note, this changeset isn't ready to push yet. In particular, it has no tests
>> yet. However, the implementation is so simple that I figured I should include
>> it. Comments on the specification wording are also welcome.
>>
>> Thanks,
>>
>> s'marks
Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
Hi Tagir,
try to do it now and you will see that you can't, because you can not write
Iterable yet.
Once we will get generics over value types, it will be a no-brainer.
Rémi
- Mail original -
> De: "Tagir Valeev"
> À: "Stuart Marks"
> Cc: "core-libs-dev"
> Envoyé: Mercredi 6 Mars 2019 11:10:41
> Objet: Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
> Hello!
>
> By the way one of the painful code patterns in modern Java is `for(int
> i = 0; i newbies and prone to errors as the variable need to be repeated three
> times. Also the variable is not effectively final, despite it never
> changes within the loop body, so could have been considered as
> redeclared on every loop iteration (like in for-each). With the new
> proposal it's possible to write `for(int i : range(0, bound).boxed())`
> (assuming import static j.u.s.IntStream.range), which looks much
> better, though it has obvious performance drawback. Moving
> IterableOnce to BaseStream would enable to use `for(int i : range(0,
> bound))` which looks even better, though again we have plenty of
> garbage (but considerably less than in previous case!). I wonder
> whether Java could evolve to the point where such kind of code would
> be a recommended way to iterate over subsequent integer values without
> any performance handicap.
>
> With best regards,
> Tagir Valeev.
>
> On Fri, Mar 1, 2019 at 9:47 AM Stuart Marks wrote:
>>
>> Hi all,
>>
>> Please review and comment on this proposal to allow Stream instances to be
>> used
>> in enhanced-for ("for-each") loops.
>>
>> Abstract
>>
>> Occasionally it's useful to iterate a Stream using a conventional loop.
>> However,
>> the Stream interface doesn't implement Iterable, and therefore streams
>> cannot be
>> used with the enhanced-for statement. This is a proposal to remedy that
>> situation by introducing a new interface IterableOnce that is a subtype of
>> Iterable, and then retrofitting the Stream interface to implement it. Other
>> JDK
>> classes will also be retrofitted to implement IterableOnce.
>>
>> Full Proposal:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>>
>> Bug report:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8148917
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>>
>> Note, this changeset isn't ready to push yet. In particular, it has no tests
>> yet. However, the implementation is so simple that I figured I should include
>> it. Comments on the specification wording are also welcome.
>>
>> Thanks,
>>
> > s'marks
[13] RFR 8217254, 8217721: CompactNumberFormat() constructor does not comply with spec and format() method spec for IAEx is not complaint
Hi, Please review the fix for JDK-8217254 and JDK-8217721 Bug: https://bugs.openjdk.java.net/browse/JDK-8217254 https://bugs.openjdk.java.net/browse/JDK-8217721 Webrev: http://cr.openjdk.java.net/~nishjain/8217254_8217721/webrev.00/ Issue: The exception thrown by constructor and format() method was not compliant with the specification Fix: Updated the constructor and format method to throw exception as per the specification Regards, Nishit Jain
Re: Proposal: JDK-8148917 Enhanced-For Statement Should Allow Streams
Hello!
By the way one of the painful code patterns in modern Java is `for(int
i = 0; i wrote:
>
> Hi all,
>
> Please review and comment on this proposal to allow Stream instances to be
> used
> in enhanced-for ("for-each") loops.
>
> Abstract
>
> Occasionally it's useful to iterate a Stream using a conventional loop.
> However,
> the Stream interface doesn't implement Iterable, and therefore streams cannot
> be
> used with the enhanced-for statement. This is a proposal to remedy that
> situation by introducing a new interface IterableOnce that is a subtype of
> Iterable, and then retrofitting the Stream interface to implement it. Other
> JDK
> classes will also be retrofitted to implement IterableOnce.
>
> Full Proposal:
>
> http://cr.openjdk.java.net/~smarks/reviews/8148917/IterableOnce0.html
>
> Bug report:
>
> https://bugs.openjdk.java.net/browse/JDK-8148917
>
> Webrev:
>
> http://cr.openjdk.java.net/~smarks/reviews/8148917/webrev.0/
>
> Note, this changeset isn't ready to push yet. In particular, it has no tests
> yet. However, the implementation is so simple that I figured I should include
> it. Comments on the specification wording are also welcome.
>
> Thanks,
>
> s'marks
