Re: RFR: 8273682: Upgrade Jline to 3.20.0
On Thu, 23 Sep 2021 14:43:21 GMT, Jan Lahoda wrote: > I'd like to upgrade the internal JLine to 3.20.0, to support the rxvt > terminal (see JDK-8270943), and to generally use a newer version of the > library. This patch is basically a application of relevant parts of the diff > between JLine 3.14.0 and 3.20.0, with merge fixes as needed. > > Thanks! LGTM src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/CompletionMatcher.java line 32: > 30: * > 31: * @param candidates list of candidates > 32: * @return a map of candidates that completion matcher matches a list of ..? src/jdk.internal.le/share/classes/jdk/internal/org/jline/reader/EndOfFileException.java line 2: > 1: /* > 2: * Copyright (c) 2002-2020, the original author or authors. 2021 - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5655
Re: RFR: 8274879: Replace uses of StringBuffer with StringBuilder within java.base classes
On Thu, 7 Oct 2021 16:48:06 GMT, Naoto Sato wrote: >> StringBuffer is a legacy synchronized class. There are more modern >> alternatives which perform better: >> 1. Plain String concatenation should be preferred >> 2. StringBuilder is a direct replacement to StringBuffer which generally >> have better performance >> >> In [JDK-8264029](https://bugs.openjdk.java.net/browse/JDK-8264029) I >> migrated only usages which were automatically detected by IDEA. It turns out >> there are more usages which can be migrated. > > src/java.base/share/classes/java/lang/Character.java line 123: > >> 121: * than U+ are called supplementary characters. The Java >> 122: * platform uses the UTF-16 representation in {@code char} arrays and >> 123: * in the {@code String} and {@code StringBuilder} classes. In > > Not sure simple replacement applies here, as `StringBuffer` still uses > `UTF-16` representation. You may add `StringBuilder` as well here, but I > think you might want to file a CSR to clarify. reverted changes in this spec. - PR: https://git.openjdk.java.net/jdk/pull/5432
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v3]
> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE avoid link in private javadoc - Changes: - all: https://git.openjdk.java.net/jdk/pull/5878/files - new: https://git.openjdk.java.net/jdk/pull/5878/files/4ba785b3..d35afde5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5878=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5878=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5878.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5878/head:pull/5878 PR: https://git.openjdk.java.net/jdk/pull/5878
RFR: JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior
Classes compiled prior to the nestmate support will generate `REF_invokeSpecial` if the implementation method is a private instance method. Since a lambda proxy class is a hidden class, a nestmate of the host class, it can invoke the private implementation method but it has to use `REF_invokeVirtual` or `REF_invokeInterface`. In order to support the old classes running on the new runtime, LMF implementation adjusts the reference kind from `REF_invokeSpecial` to `REF_invokeVirtual/REF_invokeInterface`. This PR fixes the check properly to ensure the adjustment is only made if the implementation method is private method in the host class. - Commit messages: - JDK-8274848: LambdaMetaFactory::metafactory on REF_invokeSpecial impl method has incorrect behavior Changes: https://git.openjdk.java.net/jdk/pull/5901/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5901=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8274848 Stats: 379 lines in 4 files changed: 378 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/5901.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5901/head:pull/5901 PR: https://git.openjdk.java.net/jdk/pull/5901
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]
On Mon, 11 Oct 2021 18:52:07 GMT, Andrey Turbanov wrote: >> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE > update javadoc of 'newCapacity' method to refer > ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead We generally avoid in javadoc. Especially in private javadoc. I would write this more simply/readably as * {@code SOFT_MAX_ARRAY_LENGTH >> coder} - Marked as reviewed by martin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8271737: Only normalize the cached user.dir property once
On Thu, 7 Oct 2021 14:05:19 GMT, Evgeny Astigeevich wrote: > The change completes the fix of > [JDK-8198997](https://bugs.openjdk.java.net/browse/JDK-8198997) which has > added normalisation in a constructor but not removed it from the get method. Lgtm. - Marked as reviewed by phh (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5850
Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
I agree with the idea of a try() syntax, but i don't think we need more interfaces. Yes, John is right about the fact that the TWR Aucloseable does not work well with checked exceptions, but the issue is more that there is nothing that works well with checked exceptions because Java has no way to compose them correctly (remainder: we should remove the idea of checked exceptions from the language given that it's a backward compatible change and Kotlin, C# are safer than Java because users of those languages are not used to write a try/catch/printStackTrace in those languages unlike in Java). So for me, AutoCloseable is enough. The problem is more than a fluent API are expression oriented and a try-with-resources is statement oriented, hence the mismatch. Like we have introduced the switch expression, here we need a try-with-resources expression. There are good reasons to have a try-with-resources expression 1) it can be used to track a fluent chain that should close() the resources 2) the compiler can guarantee that the stream/reader/etc around the resources does not leak outside. In term of syntax, i don't think that the arrow syntax (the one used for a case of a switch case or a lambda) should be used here, because using a block expression seems to be always a mistake. I like the proposal from John, a try(expression) or a try expression (without the parenthesis). regards, Rémi - Original Message - > From: "John Rose" > To: "Paul Sandoz" , "Brian Goetz" > , "Tagir F.Valeev" > > Cc: "core-libs-dev" > Sent: Lundi 11 Octobre 2021 20:42:20 > Subject: Re: RFR: 8274412: Add a method to Stream API to consume and close > the stream without using try-with-resources > So the purpose of TWR is to hold an object with a “close-debt” > (debt of a future call to close) and pay it at the end of a block, > sort of like C++ RAII (but also sort of not). > > But fluent syntaxes (which I like very much and hope to see > more of in the future!) don’t play well with blocks, so if a > fluent chain (any part of that chain: It’s multiple objects) > incurs a “close-debt”, it’s hard to jam a TWR block into it. > > Hence the current proposal. I agree with Brian and Paul > that we haven’t examined all the corners of this problem > yet. And I’d like to poke at the concept of “close-debt” to > help with the examination. > > Just for brain storming, I think we could model “close-debt” > outside either fluent API structure or TWR block structure. > Java O-O APIs are the pre-eminent way to model things in > Java, and they work exceedingly well, when used with skill. > > AutoCloseable models close-debt of course. But it has two > weaknesses: It doesn’t model anything *other* than the > debt, and its (sole) method skates awkwardly around the > issue of checked exceptions. (It requires an override with > exception type narrowing to be used in polite company.) > AC is more of an integration hook with TWR, rather than > a general-purpose model for close-debt. Therefore it doesn’t > teach us much about close-debt in a fluent setting. > > Surely we can model close-debt better. Let’s say that an > operation (expression) with close-debt *also* has a return > value and (for grins) *also* has an exception it might throw. > This gets us to an API closer to Optional. (If you hear the > noise of a monad snuffling around in the dark outside > your tent, you are not wrong.) > > interface MustClose_1 { > T get() throws X; //or could throw some Y or nothing at all > void close() throws X; > } > > (I wish we had an easier way to associate such an X > with such a T, so that Stream could be more > interoperable with simple Stream. It’s a pain to > carry around those X arguments. So I’ll drop X now.) > > interface MustClose_2 { > T get(); > void close() throws Exception; > } > > An expression of this type requires (in general) two > operations to finish up: It must be closed, and the result > (type T) must be gotten. There’s an issue of coupling between > the two methods; I would say, decouple their order, so that > the “get” call, as with Optional, can be done any time, > and the “close” call can be done in any order relative > to “get”. Both calls should be idempotent, I think. > But that’s all second-order detail. > > A first-order detail is the apparent but incorrect 1-1 relation > between T values and close-debts. That’s very wrong; > a closable stream on 1,000 values has one close-debt, > not 1,000. So maybe we need: > > interface MustClose_3 { > S map(Function value); > void close() throws Exception; > } > > That “map” method looks a little like Remi’s apply > method. Did I mention this design requires skill > (as well as flexibility, with one hand already tied > by checked exceptions)? I’m at the edge of my own > skill here, but I think there’s good ground to explore > here. > > In a fluent setting, a Stream that incurs a close-debt > might be typed (after incurring the
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]
On Mon, 11 Oct 2021 18:52:07 GMT, Andrey Turbanov wrote: >> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE > > Andrey Turbanov has updated the pull request incrementally with one > additional commit since the last revision: > > 275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE > update javadoc of 'newCapacity' method to refer > ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead I'll run tests; if they pass, I'll sponsor the change. - PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]
> 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE Andrey Turbanov has updated the pull request incrementally with one additional commit since the last revision: 275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE update javadoc of 'newCapacity' method to refer ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead - Changes: - all: https://git.openjdk.java.net/jdk/pull/5878/files - new: https://git.openjdk.java.net/jdk/pull/5878/files/d679bd3a..4ba785b3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5878=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5878=00-01 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/5878.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5878/head:pull/5878 PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE [v2]
On Sun, 10 Oct 2021 22:13:29 GMT, Sergey Bylokhov wrote: >> Andrey Turbanov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE >> update javadoc of 'newCapacity' method to refer >> ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead > > src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 240: > >> 238: * OutOfMemoryError: Requested array size exceeds VM limit >> 239: */ >> 240: private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; > > Looks like the usage of this field was removed by the > https://github.com/openjdk/lanai/commit/03642a01, note that the doc for the > "newCapacity" is still mentioned this field. doc updated - PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
So the purpose of TWR is to hold an object with a “close-debt” (debt of a future call to close) and pay it at the end of a block, sort of like C++ RAII (but also sort of not). But fluent syntaxes (which I like very much and hope to see more of in the future!) don’t play well with blocks, so if a fluent chain (any part of that chain: It’s multiple objects) incurs a “close-debt”, it’s hard to jam a TWR block into it. Hence the current proposal. I agree with Brian and Paul that we haven’t examined all the corners of this problem yet. And I’d like to poke at the concept of “close-debt” to help with the examination. Just for brain storming, I think we could model “close-debt” outside either fluent API structure or TWR block structure. Java O-O APIs are the pre-eminent way to model things in Java, and they work exceedingly well, when used with skill. AutoCloseable models close-debt of course. But it has two weaknesses: It doesn’t model anything *other* than the debt, and its (sole) method skates awkwardly around the issue of checked exceptions. (It requires an override with exception type narrowing to be used in polite company.) AC is more of an integration hook with TWR, rather than a general-purpose model for close-debt. Therefore it doesn’t teach us much about close-debt in a fluent setting. Surely we can model close-debt better. Let’s say that an operation (expression) with close-debt *also* has a return value and (for grins) *also* has an exception it might throw. This gets us to an API closer to Optional. (If you hear the noise of a monad snuffling around in the dark outside your tent, you are not wrong.) interface MustClose_1 { T get() throws X; //or could throw some Y or nothing at all void close() throws X; } (I wish we had an easier way to associate such an X with such a T, so that Stream could be more interoperable with simple Stream. It’s a pain to carry around those X arguments. So I’ll drop X now.) interface MustClose_2 { T get(); void close() throws Exception; } An expression of this type requires (in general) two operations to finish up: It must be closed, and the result (type T) must be gotten. There’s an issue of coupling between the two methods; I would say, decouple their order, so that the “get” call, as with Optional, can be done any time, and the “close” call can be done in any order relative to “get”. Both calls should be idempotent, I think. But that’s all second-order detail. A first-order detail is the apparent but incorrect 1-1 relation between T values and close-debts. That’s very wrong; a closable stream on 1,000 values has one close-debt, not 1,000. So maybe we need: interface MustClose_3 { S map(Function value); void close() throws Exception; } That “map” method looks a little like Remi’s apply method. Did I mention this design requires skill (as well as flexibility, with one hand already tied by checked exceptions)? I’m at the edge of my own skill here, but I think there’s good ground to explore here. In a fluent setting, a Stream that incurs a close-debt might be typed (after incurring the debt, perhaps in a transform) as Stream>, and somehow all consumers of the MustClose, such as map and collect operations on the Stream, would correctly unpack each T from the MC, and then repack the result into the MC<.> wrapper. var res = openAFileStream().map(…).collect(…); Here the first method call returns a Stream with close-debt mixed into its type. The map and collect calls would wire both parts: The T values flowing through, and the close-debt. Who takes responsibility for paying the close debt? Maybe an extra call at the end: …map(…).collectAndClose(…). Or maybe the stream “knows” internally that since its type has a close debt, all of its terminal operations have to pay off that debt as they collect the payloads. So it would be automatic, somehow, inside of collect, forEach, etc. To make the parts hook up right, you might need reified generics, or maybe an amended type MustCloseStream <: Stream>, like the LongStream <: Stream we dislike. I’m only proposing as a thought exercise for now. Maybe the MustCloseStream takes an explicit close method which produces a regular stream over the base type. The explicit close method would release resources and buffer anything necessary to produce an in-memory Stream. You’d want to call it late in the fluent chain, after filtering and flat-mapping is done, just before a collect for forEach. Here’s a streamlined version of MustClose that I like, which sidesteps the problem of mutual ordering of two methods: interface MustClose_4 { R getAndClose() throws Exception; default void close() throws Exception { getAndClose(); } } Here, R is not an element type of a stream, but rather the final result (maybe Void) of some terminal operation. Such an interface could interact with syntax, too. For example, it might help with TWR expressions (if we wanted to think about that): var res
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Thu, 9 Sep 2021 08:25:44 GMT, Wu Yan wrote: >> Hi, >> Please help me review the change to enhance getting time zone ID from >> /etc/localtime on linux. >> >> We use `realpath` instead of `readlink` to obtain the link name of >> /etc/localtime, because `readlink` can only read the value of a symbolic of >> link, not the canonicalized absolute pathname. >> >> For example, the value of /etc/localtime is >> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by >> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of >> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which >> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you >> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. >> >> Thanks, >> wuyan > > Wu Yan has updated the pull request incrementally with one additional commit > since the last revision: > > replace realpath src/java.base/unix/native/libjava/TimeZone_md.c line 113: > 111: } > 112: } > 113: There are a few `*(right + 1)` references in the loops. Is there any possibility that it would run over the buffer? src/java.base/unix/native/libjava/path_util.h line 31: > 29: int collapsible(char *names); > 30: void splitNames(char *names, char **ix); > 31: void joinNames(char *names, int nc, char **ix); Are these functions, `collapsible`, `splitNames` and `joinNames` have to be non-static? - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE
On Sat, 9 Oct 2021 17:54:16 GMT, Andrey Turbanov wrote: > 8275002: Remove unused AbstractStringBuilder.MAX_ARRAY_SIZE JDK sources should not contain dead unused fields - thanks for fixing. The change to use newLength in this file should have adjusted the javadoc of newCapacity, perhaps simply to refer to ArraysSupport.SOFT_MAX_ARRAY_LENGTH instead. That sounds like a job for Jim Laskey as the author of commit 03642a01af7123298d6524a98c99a3934d35c11b Author: Jim Laskey Date: Thu Jun 11 10:08:23 2020 -0300 8230744: Several classes throw OutOfMemoryError without message Reviewed-by: psandoz, martin, bchristi, rriggs, smarks If that is fixed (perhaps in a different commit), then this commit is good. History has shown that capacity growth code is highly errorprone, so it's worth writing whitebox tests, as I did in e.g. ./java/util/concurrent/ConcurrentHashMap/WhiteBox.java ./java/util/ArrayDeque/WhiteBox.java ./java/util/HashMap/WhiteBoxResizeTest.java - Marked as reviewed by martin (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/5878
Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources
Hi Tagir, Do you mind if we slow down on this and let the idea bake somewhat, captured in this thread/issue/PR(draft). I am always a little wary of compact-only or fluent-only methods, as I find them harder to justify. In this case I think there might be something more with regards to a pattern/idiom, and making it easier to not forgot to close. But, I am not sure we are there yet. - we have had this “transform” idiom floating around for a while which is like your consumeAndClose, but without the try block. https://bugs.openjdk.java.net/browse/JDK-8140283 And such a method was added to String. There are likely other places where we could consider adding this idiom. - I think we should explore adding the method you propose on AutoCloseable, that likely has more leverage, but we would need to do some careful analysis of code to ensure the risk to incompatibly is low. The idioms “transform” and “transformAndClose” are I think related [*]. If we can nail down these I would feel much better about committing to them as methods on various classes. Paul. [*] If we ever get the notion of Haskell-like type classes in the platform I wonder if those would provide the hook, we could add later, that I am looking for so that we can corral these idioms. > On Oct 3, 2021, at 11:51 PM, Tagir F.Valeev wrote: > > Currently, when the stream holds a resource, it's necessary to wrap it with > try-with-resources. This undermines the compact and fluent style of stream > API calls. For example, if we want to get the `List` of files inside the > directory and timely close the underlying filehandle, we should use something > like this: > > > List paths; > try (Stream stream = Files.list(Path.of("/etc"))) { >paths = stream.toList(); > } > // use paths > > > I suggest to add a new default method to Stream interface named > `consumeAndClose`, which allows performing terminal stream operation and > closing the stream at the same time. It may look like this: > > >default R consumeAndClose(Function, ? extends R> > function) { >Objects.requireNonNull(function); >try(this) { >return function.apply(this); >} >} > > > Now, it will be possible to get the list of the files in the fluent manner: > > > List list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList); > > - > > Commit messages: > - Fix tests > - 8274412: Add a method to Stream API to consume and close the stream without > using try-with-resources > > Changes: https://git.openjdk.java.net/jdk/pull/5796/files > Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5796=00 > Issue: https://bugs.openjdk.java.net/browse/JDK-8274412 > Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod > Patch: https://git.openjdk.java.net/jdk/pull/5796.diff > Fetch: git fetch https://git.openjdk.java.net/jdk pull/5796/head:pull/5796 > > PR: https://git.openjdk.java.net/jdk/pull/5796
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v2]
On Mon, 11 Oct 2021 13:42:35 GMT, Ravi Reddy wrote: >> Hi all, >> >> Please review this fix for Infinite loop in ZipOutputStream.close(). >> The main issue here is when ever there is an exception during close >> operations on GZip we are not setting the deflator to a finished state which >> is leading to an infinite loop when we try writing on the same GZip >> instance( since we use while(!def.finished()) inside the write operation). >> >> Thanks, >> Ravi > > Ravi Reddy has updated the pull request incrementally with one additional > commit since the last revision: > > 8193682 : Infinite loop in ZipOutputStream.close() I have updated the review with the new fix. Instead of throwing an exception in close() method , Now when we get an exception during write inside deflate() , we will close the stream and throw the exception. Updated the test case in TestNG format. - PR: https://git.openjdk.java.net/jdk/pull/5522
Re: RFR: 8193682: Infinite loop in ZipOutputStream.close() [v2]
> Hi all, > > Please review this fix for Infinite loop in ZipOutputStream.close(). > The main issue here is when ever there is an exception during close > operations on GZip we are not setting the deflator to a finished state which > is leading to an infinite loop when we try writing on the same GZip instance( > since we use while(!def.finished()) inside the write operation). > > Thanks, > Ravi Ravi Reddy has updated the pull request incrementally with one additional commit since the last revision: 8193682 : Infinite loop in ZipOutputStream.close() - Changes: - all: https://git.openjdk.java.net/jdk/pull/5522/files - new: https://git.openjdk.java.net/jdk/pull/5522/files/5072b6c1..f6a678ed Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5522=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5522=00-01 Stats: 96 lines in 2 files changed: 48 ins; 28 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/5522.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5522/head:pull/5522 PR: https://git.openjdk.java.net/jdk/pull/5522
Re: RFR: 8177814: jdk/editpad is not in jdk TEST.groups
On Thu, 23 Sep 2021 08:54:48 GMT, Aleksey Shipilev wrote: > @prrace notices this here: > https://github.com/openjdk/jdk/pull/5544#issuecomment-925396869. And I think > it is the already open issue that this patch is fixing. While the original > patch added the test in `jdk_other`, Phil suggests it to be added to > `jdk_desktop`. > > Additional testing: > - [x] `jdk_editpad` is passing Anyhow, how do you want to proceed with this PR? - PR: https://git.openjdk.java.net/jdk/pull/5648
Re: RFR: 4511638: Double.toString(double) sometimes produces incorrect results [v2]
On Fri, 16 Apr 2021 11:30:32 GMT, Raffaello Giulietti wrote: >> Hello, >> >> here's a PR for a patch submitted on March 2020 >> [1](https://cr.openjdk.java.net/~bpb/4511638/webrev.04/) when Mercurial was >> a thing. >> >> The patch has been edited to adhere to OpenJDK code conventions about >> multi-line (block) comments. Nothing in the code proper has changed, except >> for the addition of redundant but clarifying parentheses in some expressions. >> >> >> Greetings >> Raffaello > > Raffaello Giulietti has updated the pull request incrementally with one > additional commit since the last revision: > > 4511638: Double.toString(double) sometimes produces incorrect results Hi Guy, while implementing the additional test recommended in your point (2), it occurred to me that the numbers of the form y 10^n, y in D_k (k = 1, 2, 3, 4) end up being of the form y' 10^n', where y' = y / 10^k, n' = n + k, plus the 2 * Y values around these. Such numbers do not seem to show any special structure worth a dedicated test, so I'm wondering if you mean something else instead. Perhaps you mean y to have at most 4 digits, i.e., 0 <= y < 10^4? Greetings Raffaello P.S. The test recommended in point (1) pass successfully. - PR: https://git.openjdk.java.net/jdk/pull/3402