Re: ReversibleCollection proposal
On Fri, 23 Apr 2021 at 07:33, Stuart Marks wrote: > On 4/22/21 8:36 AM, Stephen Colebourne wrote: > Having these methods on Collection could lead to a situation where calling > addFirst > and then getFirst might result in getFirst returning a different element from > what > was passed to addFirst. This doesn't make any sense for methods that have > "first" in > the name. FWIW, I'm comfortable with that weirdness. The method could be called addPreferFirst() but that is a bit of a mouthful. I'd also note that it is much easier for developers of other Collection implementations to add a method with a matching name than it is to implement a new interface. Even libraries maintaining compatibility with Java 5 could do that. Your proposal has a similar issue BTW - addFirst() on a SortedSet. Your proposal is to throw UnsupportedOperationException, but I (like Remi) think that isn't a good answer when UnsupportedOperationException in collections is usually about immutability. It is a natural problem of the domain that adding first when the implementation is sorted is going to be confusing. I am simply suggesting embracing the weirdness, rather than trying to exception your way out of it. Stephen
New Collections interface - Sized
Hi all, While a discussion on ReversibleCollection is going on, I'd like to raise the interface I've always found most missing from the framework - Sized public interface Sized { int size(); default boolean isEmpty() { return size() == 0; } default boolean isNotEmpty() { return !isEmpty(); } } This would be applied to Collection and Map, providing a missing unification. Anything else that has a size could also be retrofitted, such as String. Ideally arrays too, but that could come later. Note that Iterable would not implement it. WDYT? Stephen
RFR: 8265746: Update java.time to use instanceof pattern variable (part II)
Hi, Could someone please review the second half of my update for the `java.time` package to make use of the `instanceof` pattern variable? This PR was split into two parts due to the large number of files affected. Kind regards, Patrick - Commit messages: - 8265746: Update java.time to use instanceof pattern variable (part II) Changes: https://git.openjdk.java.net/jdk/pull/3650/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3650&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265746 Stats: 152 lines in 20 files changed: 0 ins; 48 del; 104 mod Patch: https://git.openjdk.java.net/jdk/pull/3650.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3650/head:pull/3650 PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II)
On Fri, 23 Apr 2021 10:44:30 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review the second half of my update for the `java.time` > package to make use of the `instanceof` pattern variable? > > This PR was split into two parts due to the large number of files affected. > > Kind regards, > > Patrick LGTM! - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II)
On Fri, 23 Apr 2021 10:44:30 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review the second half of my update for the `java.time` > package to make use of the `instanceof` pattern variable? > > This PR was split into two parts due to the large number of files affected. > > Kind regards, > > Patrick Looks good. Thank you for the updates - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II)
On Fri, 23 Apr 2021 10:44:30 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review the second half of my update for the `java.time` > package to make use of the `instanceof` pattern variable? > > This PR was split into two parts due to the large number of files affected. > > Kind regards, > > Patrick Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v6]
On Wed, 21 Apr 2021 13:13:16 GMT, Jim Laskey wrote: >> Move makeXXXSpilterator from public (@hidden) to protected. No API ch > > Jim Laskey has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains eight additional commits since > the last revision: > > - Merge branch 'master' into 8265137 > - Remove @hidden > - Correct the hierarchy of Random > - Remove extraneous references to makeXXXSpliterator > - Move makeXXXSpliterator methods to RandomSupport > - change static final from 'proxy' to 'PROXY' > - Make makeXXXSpliterator final > - Move makeXXXSpilterator from public (@hidden) to protected. No API change. Hi, the overall setup is mch better than before! This also makes the problem we have seen in https://github.com/policeman-tools/forbidden-apis/issues/177 go away, because Random/ThreadLocalRandom no longer extend a public, but internal/hidden class (forbiddenapis is a classfile analyzer that tries to prevent user's from extending internal JDK APIs in one of its checks). When looking into superclasses of Random, it detected a "bad superclass from an internal package". So this alone makes me very happy with this. In addition, it no longer changes APIs of Random/ThreadLocalRandom, only the new interface is added. +1 to merge - Marked as reviewed by uschindler (Author). PR: https://git.openjdk.java.net/jdk/pull/3469
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II)
On Fri, 23 Apr 2021 10:44:30 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review the second half of my update for the `java.time` > package to make use of the `instanceof` pattern variable? > > This PR was split into two parts due to the large number of files affected. > > Kind regards, > > Patrick Marked as reviewed by chegar (Reviewer). src/java.base/share/classes/java/time/Clock.java line 623: > 621: return (obj instanceof FixedClock other > 622: && instant.equals(other.instant) > 623: && zone.equals(other.zone)); The outer set of braces is redundant. src/java.base/share/classes/java/time/Clock.java line 673: > 671: return (obj instanceof OffsetClock other > 672: && baseClock.equals(other.baseClock) > 673: && offset.equals(other.offset)); The outer set of braces is redundant. src/java.base/share/classes/java/time/ZonedDateTime.java line 2191: > 2189: && dateTime.equals(other.dateTime) > 2190: && offset.equals(other.offset) > 2191: && zone.equals(other.zone)); same here. - PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II) [v2]
> Hi, > > Could someone please review the second half of my update for the `java.time` > package to make use of the `instanceof` pattern variable? > > This PR was split into two parts due to the large number of files affected. > > Kind regards, > > Patrick Patrick Concannon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Removed redundant braces - Merge remote-tracking branch 'origin/master' into JDK-8265746 - 8265746: Update java.time to use instanceof pattern variable (part II) - Changes: - all: https://git.openjdk.java.net/jdk/pull/3650/files - new: https://git.openjdk.java.net/jdk/pull/3650/files/974ccbf0..7f32ddbe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3650&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3650&range=00-01 Stats: 519 lines in 11 files changed: 275 ins; 177 del; 67 mod Patch: https://git.openjdk.java.net/jdk/pull/3650.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3650/head:pull/3650 PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II) [v2]
On Fri, 23 Apr 2021 15:38:53 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review the second half of my update for the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> This PR was split into two parts due to the large number of files affected. >> >> Kind regards, >> >> Patrick > > Patrick Concannon has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Removed redundant braces > - Merge remote-tracking branch 'origin/master' into JDK-8265746 > - 8265746: Update java.time to use instanceof pattern variable (part II) Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3650
Re: New Collections interface - Sized
This has come up before. For example, during an early iteration of the Stream design, before parallelism entered the picture. The first scrawled-on-a-napkin prototype for streams was based on Iterator, and it took about a minute to realize that we could do a much better job if we had a slightly broader interface to work with, essentially Iterator+Sized. When you pull on this string, you end up with a lot of new interfaces, such as SizedIterator, SizedIterable, etc, in part because ... we have no intersection types. Having lots of fine-grained interfaces for "has X" and "has Y" is nice from a "bucket of lego bricks" library-design perspective, but when the user goes to express "I need an aggregate that has sizes, iterators, and encounter order", you end up with code like: &Sized> void foo(X x) { ... } and then you run into the wall of "but I can only use intersection types in these places in the language." The idiom of having fine-grained mix-in-ish interfaces really wants a language with intersection types. Additionally, I am having a hard time imagining how Sized would be usable by a client; no method will *take* a Sized (it's just not broad enough), and I can't immediately imagine what would even *return* a Sized. If the type is not suitable for use by clients, then it serves only to organize the library itself, and that's a weaker motivation. Is there a compelling example of where this would be used by clients? On 4/23/2021 5:23 AM, Stephen Colebourne wrote: Hi all, While a discussion on ReversibleCollection is going on, I'd like to raise the interface I've always found most missing from the framework - Sized public interface Sized { int size(); default boolean isEmpty() { return size() == 0; } default boolean isNotEmpty() { return !isEmpty(); } } This would be applied to Collection and Map, providing a missing unification. Anything else that has a size could also be retrofitted, such as String. Ideally arrays too, but that could come later. Note that Iterable would not implement it. WDYT? Stephen
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II) [v2]
On Fri, 23 Apr 2021 15:38:53 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review the second half of my update for the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> This PR was split into two parts due to the large number of files affected. >> >> Kind regards, >> >> Patrick > > Patrick Concannon has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Removed redundant braces > - Merge remote-tracking branch 'origin/master' into JDK-8265746 > - 8265746: Update java.time to use instanceof pattern variable (part II) Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II) [v2]
On Fri, 23 Apr 2021 15:38:53 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review the second half of my update for the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> This PR was split into two parts due to the large number of files affected. >> >> Kind regards, >> >> Patrick > > Patrick Concannon has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Removed redundant braces > - Merge remote-tracking branch 'origin/master' into JDK-8265746 > - 8265746: Update java.time to use instanceof pattern variable (part II) Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8265746: Update java.time to use instanceof pattern variable (part II) [v2]
On Fri, 23 Apr 2021 15:38:53 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review the second half of my update for the `java.time` >> package to make use of the `instanceof` pattern variable? >> >> This PR was split into two parts due to the large number of files affected. >> >> Kind regards, >> >> Patrick > > Patrick Concannon has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Removed redundant braces > - Merge remote-tracking branch 'origin/master' into JDK-8265746 > - 8265746: Update java.time to use instanceof pattern variable (part II) Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3650
Re: RFR: 8264208: Console charset API [v12]
On Thu, 22 Apr 2021 17:38:43 GMT, Naoto Sato wrote: >> Please review the changes for the subject issue. This has been suggested in >> a recent discussion thread for the JEP 400 >> [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. >> A CSR has also been drafted, and comments are welcome >> [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Revived charset() in JavaIOAccess interface. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3419
Integrated: 8264208: Console charset API
On Fri, 9 Apr 2021 16:47:55 GMT, Naoto Sato wrote: > Please review the changes for the subject issue. This has been suggested in > a recent discussion thread for the JEP 400 > [[1](https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-March/075214.html)]. > A CSR has also been drafted, and comments are welcome > [[2](https://bugs.openjdk.java.net/browse/JDK-8264209)]. This pull request has now been integrated. Changeset: bebfae48 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/bebfae48 Stats: 214 lines in 10 files changed: 184 ins; 12 del; 18 mod 8264208: Console charset API Reviewed-by: joehw, rriggs, alanb - PR: https://git.openjdk.java.net/jdk/pull/3419
Re: RFR: 4926314: Optimize Reader.read(CharBuffer) [v9]
On Sat, 13 Mar 2021 14:28:25 GMT, Philippe Marschall wrote: >> Implement three optimiztations for Reader.read(CharBuffer) >> >> * Add a code path for heap buffers in Reader#read to use the backing array >> instead of allocating a new one. >> * Change the code path for direct buffers in Reader#read to limit the >> intermediate allocation to `TRANSFER_BUFFER_SIZE`. >> * Implement `InputStreamReader#read(CharBuffer)` and delegate to >> `StreamDecoder`. >> * Implement `StreamDecoder#read(CharBuffer)` and avoid buffer allocation. > > Philippe Marschall has updated the pull request with a new target base due to > a merge or a rebase. The pull request now contains 15 commits: > > - Merge master > - Fix bug in CharArrayReader and add unit test > - Clean up unit tests > - Revert off-heap code path > - Replace c-style array declarations > - Share work buffer between #skip and #read > - Update copyright year > - Implement review comment > - Revert StreamDecoder changes > - Implement CharArrayReader#read(CharBuffer) > - ... and 5 more: > https://git.openjdk.java.net/jdk/compare/d339320e...c4c859e0 This PR is on hold pending the author's issuing the `/integrate` command. - PR: https://git.openjdk.java.net/jdk/pull/1915
Re: RFR: 8200559: Java agents doing instrumentation need a means to define auxiliary classes [v2]
On Apr 21, 2021, at 11:40 AM, Alan Bateman wrote: > > Sure, if you are using native code then you have the full power of JVM TI and > JNI available. Project Panama is exploring how to restrict access to native > code, I think too early to say how this might extend to JNI. I looked at some of the Panama documents and saw no hint that it might be extended to JNI. It seems to be positioned as an (partial) alternative to JNI. What I do see is that native code has no direct access to the JDK via Panama, only the ability to invoke provided upcalls, which can only access objects and methods that the caller has access to. That is indeed much more restricted than JNI. Even though it is “too early”, can you explain why you think Panama’s restrictions might apply to JNI? As a library developer, I have often made use of JNI to work around limitations in current and older JDKs. I would hate to lose that ability. Alan
Re: New Collections interface - Sized
On Fri, 23 Apr 2021 at 17:08, Brian Goetz wrote: > This has come up before. For example, during an early iteration of the > Stream design, before parallelism entered the picture. The first > scrawled-on-a-napkin prototype for streams was based on Iterator, and it took > about a minute to realize that we could do a much better job if we had a > slightly broader interface to work with, essentially Iterator+Sized. > > When you pull on this string, you end up with a lot of new interfaces, such > as SizedIterator, SizedIterable, etc, in part because ... we have no > intersection types. Having lots of fine-grained interfaces for "has X" and > "has Y" is nice from a "bucket of lego bricks" library-design perspective, > but when the user goes to express "I need an aggregate that has sizes, > iterators, and encounter order", you end up with code like: > > &Sized> void foo(X x) { ... } > > and then you run into the wall of "but I can only use intersection types in > these places in the language." The idiom of having fine-grained mix-in-ish > interfaces really wants a language with intersection types. > > Additionally, I am having a hard time imagining how Sized would be usable by > a client; no method will *take* a Sized (it's just not broad enough), and I > can't immediately imagine what would even *return* a Sized. If the type is > not suitable for use by clients, then it serves only to organize the library > itself, and that's a weaker motivation. > > Is there a compelling example of where this would be used by clients? Here are some examples: https://stackoverflow.com/questions/10988634/java-global-isempty-method https://github.com/OpenGamma/Strata/blob/main/modules/collect/src/main/java/com/opengamma/strata/collect/ArgChecker.java#L404 ie. the ability to spot an "empty" object, or ensure input is non-empty. Use cases are generally in low-level/framework code like this, where the actual input data type is not known. Stephen
Re: New Collections interface - Sized
Is there a compelling example of where this would be used by clients? Here are some examples: https://stackoverflow.com/questions/10988634/java-global-isempty-method Without passing judgment on the sort of dynamically typed programs that need a method like this, or wondering what monstrosities such code uses to actually _get_ the data out of this weird ad-hoc union, note that this code becomes dramatically simpler with pattern matching: return switch (c) { case null -> true; case CharSequence cs -> cs.length() == 0; case Collection c -> c.isEmpty(); case Object[] os -> os.length == 0; default -> { weird reflective thingy } } Note also that a Sized abstraction only addresses two of these five cases -- CharSequence and Collection. The others would still require such ad-hoc treatment. And while it may seem like I have pattern matching on the brain, there's a point to bringing up pattern matching -- which is that real code often ends up dealing with ad-hoc unions, and a linguistic mechanism for abstracting computations over ad-hoc unions is often a better solution if you find yourself in ad-hoc land than trying to partially unify it. But again, if you are treating these things as containers, then a Sized doesn't get you very far, because if you conclude the thing isn't empty, you're going to want to get stuff out, and Sized has no methods for that. So presumably there's some companion to Sized for accessing elements by index: interface HasStuff extends Sized { T get(int index); } And note that in order for HasStuff to be useful, it has to extend Sized, because, no intersection types. Which suggests Sized is not the abstraction you are looking for. And again, pattern matching: Object getElement(Object thingWithStuff, int index) { return switch (thingWithStuff) { case null -> null; case Object[] os -> os[index]; case Collection c -> c.get(index); ... more if you can think of them ... };
Re: New Collections interface - Sized
On Fri, 23 Apr 2021 at 23:07, Brian Goetz wrote: > >> Is there a compelling example of where this would be used by clients? > > Here are some examples: > > https://stackoverflow.com/questions/10988634/java-global-isempty-method > Without passing judgment on the sort of dynamically typed programs that > need a method like this The other example is better as it benefits from declaring an API that only accepts instances of `Sized` and does not need to get the contents. > But again, if you are treating these things as containers, then a Sized > doesn't get you very far, because if you conclude the thing isn't empty, > you're going to want to get stuff out, and Sized has no methods for > that. So presumably there's some companion to Sized for accessing > elements by index: > > interface HasStuff extends Sized { > T get(int index); > } I don't think there has to be. the more useful interface would be this one, but to date there has been strong resistance in unifying the Collection and Map interfaces: interface Stuff extends Sized { int size(); int isEmpty(); int isNotEmpty(); Iterator iterator(); } Stephen
Re: New Collections interface - Sized
- Mail original - > De: "Stephen Colebourne" > À: "core-libs-dev" > Envoyé: Samedi 24 Avril 2021 00:14:51 > Objet: Re: New Collections interface - Sized > On Fri, 23 Apr 2021 at 23:07, Brian Goetz wrote: >> >> Is there a compelling example of where this would be used by clients? >> > Here are some examples: >> > https://stackoverflow.com/questions/10988634/java-global-isempty-method >> Without passing judgment on the sort of dynamically typed programs that >> need a method like this > > The other example is better as it benefits from declaring an API that > only accepts instances of `Sized` and does not need to get the > contents. > >> But again, if you are treating these things as containers, then a Sized >> doesn't get you very far, because if you conclude the thing isn't empty, >> you're going to want to get stuff out, and Sized has no methods for >> that. So presumably there's some companion to Sized for accessing >> elements by index: >> >> interface HasStuff extends Sized { >> T get(int index); >> } > > I don't think there has to be. the more useful interface would be this > one, but to date there has been strong resistance in unifying the > Collection and Map interfaces: > > interface Stuff extends Sized { > int size(); > int isEmpty(); > int isNotEmpty(); >Iterator iterator(); > } This is basically Spliterator, an iterator + a size, with the iterator is "push" instead of "pull" because it's more efficient. In details a Spliterator is either - an Iterable (with no SIZED characteristic) - an Iterable + size (if SIZED and estimateSize() != Long.MAX_VALUE) - an Iterable + comparator (if SORTED and comparator != null) - an Iterable + split (if trySplit != null) and all combinations of the above. > > Stephen Rémi
Re: New Collections interface - Sized
This is basically Spliterator, an iterator + a size, with the iterator is "push" instead of "pull" because it's more efficient. In details a Spliterator is either - an Iterable (with no SIZED characteristic) - an Iterable + size (if SIZED and estimateSize() != Long.MAX_VALUE) - an Iterable + comparator (if SORTED and comparator != null) - an Iterable + split (if trySplit != null) and all combinations of the above. So, you're asking for Spliterable, no? :)
Re: New Collections interface - Sized
- Mail original - > De: "Stephen Colebourne" > À: "core-libs-dev" > Envoyé: Vendredi 23 Avril 2021 11:23:03 > Objet: New Collections interface - Sized > Hi all, > While a discussion on ReversibleCollection is going on, I'd like to > raise the interface I've always found most missing from the framework > - Sized > > public interface Sized { > int size(); > default boolean isEmpty() { > return size() == 0; > } > default boolean isNotEmpty() { > return !isEmpty(); > } > } > > This would be applied to Collection and Map, providing a missing > unification. Anything else that has a size could also be retrofitted, > such as String. Ideally arrays too, but that could come later. Note > that Iterable would not implement it. > > WDYT? There are 3 ways to have instances of different classes to have a common behavior in Java, - either thy implement a common super-type - or you use a structural type conversion (a method ref on an instance) here: Sized sized = list::size; - or you use pattern matching (or a cascade of if/else if you are like the old fashion) int size(Object o) { return switch(o) { case List list -> list.size(); case Object[] array -> array.length(); case String s -> s.length(); default -> throw ... }; } The advantage of pattern matching is that it does not have a global impact, out of where you want to use it, by example, here, you can use s.length() instead of s.codepoints().count() without having to think what the default size of a String means for all program that can be written. > > Stephen Rémi
Re: New Collections interface - Sized
- Mail original - > De: "Brian Goetz" > À: "Remi Forax" , "Stephen Colebourne" > > Cc: "core-libs-dev" > Envoyé: Samedi 24 Avril 2021 00:40:54 > Objet: Re: New Collections interface - Sized >> This is basically Spliterator, an iterator + a size, with the iterator is >> "push" >> instead of "pull" because it's more efficient. >> >> In details a Spliterator is either >> - an Iterable (with no SIZED characteristic) >> - an Iterable + size (if SIZED and estimateSize() != Long.MAX_VALUE) >> - an Iterable + comparator (if SORTED and comparator != null) >> - an Iterable + split (if trySplit != null) >> >> and all combinations of the above. oops, i've written Iterable instead of Iterator. >> > So, you're asking for Spliterable, no? :) a Supplier> is a Spliterable, as in StreamSupport.stream() [1]. Rémi [1] https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/stream/StreamSupport.html#stream(java.util.function.Supplier,int,boolean)
Re: RFR: 8265356: need code example for getting canonical constructor of a Record [v2]
> I decided to show a complete static method in the example, so it could be > copied to user utility class as is. Not sure if it's reasonable to add > `assert cls.isRecord();` there. Also I don't know whether there's a > limitation on max characters in the sample code. Probable a line break in > `static \nConstructor getCanonicalConstructor(Class > cls)` is unnecessary. > > --- > Aside from this PR, I've found a couple of things to clean up in > `java.lang.Class`: > 1. There's erroneous JavaDoc link in `getSimpleName()` JavaDoc (introduced by > @jddarcy in #3038). It should be `#isArray()` instead of `isArray()`. > 2. Methods Atomic::casAnnotationType and Atomic::casAnnotationData have > unused type parameters ``. > 3. Probably too much but AnnotationData can be nicely converted to a record! > Not sure, probably nobody wants to have `java.lang.Record` initialized too > early or increasing the footprint of such a basic class in the metaspace, so > I don't insist on this. > > > private record AnnotationData( > Map, Annotation> annotations, > Map, Annotation> declaredAnnotations, > // Value of classRedefinedCount when we created this AnnotationData > instance > int redefinedCount) { > } > > > Please tell me if it's ok to fix 1 and 2 along with this PR. Tagir F. Valeev has updated the pull request incrementally with three additional commits since the last revision: - Trailing space removed - Add a reference from java.lang.Record to related Class methods - Fix cosmetic issues - Changes: - all: https://git.openjdk.java.net/jdk/pull/3556/files - new: https://git.openjdk.java.net/jdk/pull/3556/files/5b631cfe..1133ab7d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3556&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3556&range=00-01 Stats: 13 lines in 2 files changed: 4 ins; 1 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/3556.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3556/head:pull/3556 PR: https://git.openjdk.java.net/jdk/pull/3556
Re: RFR: 8265356: need code example for getting canonical constructor of a Record
On Tue, 20 Apr 2021 17:42:27 GMT, Stuart Marks wrote: >> I decided to show a complete static method in the example, so it could be >> copied to user utility class as is. Not sure if it's reasonable to add >> `assert cls.isRecord();` there. Also I don't know whether there's a >> limitation on max characters in the sample code. Probable a line break in >> `static \nConstructor getCanonicalConstructor(Class >> cls)` is unnecessary. >> >> --- >> Aside from this PR, I've found a couple of things to clean up in >> `java.lang.Class`: >> 1. There's erroneous JavaDoc link in `getSimpleName()` JavaDoc (introduced >> by @jddarcy in #3038). It should be `#isArray()` instead of `isArray()`. >> 2. Methods Atomic::casAnnotationType and Atomic::casAnnotationData have >> unused type parameters ``. >> 3. Probably too much but AnnotationData can be nicely converted to a record! >> Not sure, probably nobody wants to have `java.lang.Record` initialized too >> early or increasing the footprint of such a basic class in the metaspace, so >> I don't insist on this. >> >> >> private record AnnotationData( >> Map, Annotation> annotations, >> Map, Annotation> declaredAnnotations, >> // Value of classRedefinedCount when we created this AnnotationData >> instance >> int redefinedCount) { >> } >> >> >> Please tell me if it's ok to fix 1 and 2 along with this PR. > > Thanks for writing this example. > > I think that the example lines can be longer. I'd suggest putting the main > part of the method declaration on the same line as `static Record>`, but leaving the `throws` clause on the next line. > > I think including the small cleanups (1) and (2) in this PR is fine. Changing > `AnnotationData` to be a record seems like it might have other effects, so > I'd leave that one out. > > One other thing I'd like to see is a link to this example code from places > where people are likely to look for it. The class doc for `java.lang.Record` > has a definition for "canonical constructor" so it would be nice to link to > the example here. Something like "For further information about how to find > the canonical constructor reflectively, see Class::getRecordComponents." > (With appropriate javadoc markup.) This could either be a parenthetical > comment somewhere in the "canonical constructor" discussion, or possibly a > separate paragraph in the `@apiNote` section below. @stuart-marks thank you for review. How about this note in Record class? I wrote it in a more general manner, without mentioning canonical constructors explicitly. Is it enough? - PR: https://git.openjdk.java.net/jdk/pull/3556