On Wed, 25 Jan 2023 18:00:33 GMT, Weijun Wang wrote:
>> The translation tool didn't seem to translate this. Either because it
>> couldn't or because it somehow missed it. I'm not sure which, but I'm open
>> to replacing this with a translation suggestion you have. Or I can leave it
>> as is.
>
On Tue, 21 Feb 2023 20:39:53 GMT, Tagir F. Valeev wrote:
>> For cleanup and dogfooding the new method, it would be nice to use
>> Math.clamp where possible in java.base. See PR #12428.
>>
>> As Math.clamp performs an additional check that min is not greater than max,
>> I conservatively replac
On Tue, 21 Feb 2023 20:39:53 GMT, Tagir F. Valeev wrote:
>> For cleanup and dogfooding the new method, it would be nice to use
>> Math.clamp where possible in java.base. See PR #12428.
>>
>> As Math.clamp performs an additional check that min is not greater than max,
>> I conservatively replac
Please review this superficial documentation cleanup that was triggered by
unrelated analysis of doc comments in JDK API.
The only effect that this multi-area PR has on the JDK API Documentation (i.e.
the observable effect on the generated HTML pages) can be summarized as follows:
diff -ur
On Thu, 2 Mar 2023 16:23:17 GMT, Alexey Ivanov wrote:
>> Please review this superficial documentation cleanup that was triggered by
>> unrelated analysis of doc comments in JDK API.
>>
>> The only effect that this multi-area PR has on the JDK API Documentation
>> (i.e. the observable effect on
On Fri, 3 Mar 2023 08:15:49 GMT, Alexey Ivanov wrote:
>> Please review this superficial documentation cleanup that was triggered by
>> unrelated analysis of doc comments in JDK API.
>>
>> The only effect that this multi-area PR has on the JDK API Documentation
>> (i.e. the observable effect on
seem to occur in infrequently updated third-party code (e.g.
> javax.xml), which I assume we shouldn't touch unless necessary.
> * I will update copyright years after (and if) the fix had been approved, as
> required.
Pavel Rappo has updated the pull request with a new target bas
On Thu, 2 Mar 2023 12:03:44 GMT, Pavel Rappo wrote:
> Please review this superficial documentation cleanup that was triggered by
> unrelated analysis of doc comments in JDK API.
>
> The only effect that this multi-area PR has on the JDK API Documentation
> (i.e. the observabl
Please review this simple fix.
-
Commit messages:
- Fix typos
Changes: https://git.openjdk.org/jdk/pull/14136/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14136&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8308735
Stats: 9 lines in 3 files changed: 0 ins; 0 del
On Wed, 24 May 2023 23:32:18 GMT, Pavel Rappo wrote:
> Please review this simple fix.
This pull request has now been integrated.
Changeset: 38367d3c
Author: Pavel Rappo
URL:
https://git.openjdk.org/jdk/commit/38367d3c3ad9292b7c581917c89e9f07fac3dd31
Stats: 9 lines in 3 fi
On Wed, 1 Feb 2023 10:36:12 GMT, Sergey Tsypanov wrote:
>> `ResourceBundle.CacheKey.equals()` and `Bundles.CacheKey.equals()` are quire
>> outdated. This simple clean-up modernizes them.
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last r
On Fri, 23 Jun 2023 13:46:04 GMT, Sergey Tsypanov wrote:
>> src/java.base/share/classes/java/util/ResourceBundle.java line 743:
>>
>>> 741: return ((module != null) &&
>>> (module.equals(otherEntry.getModule())) &&
>>> 742: (caller != null) &&
>>> (calle
On Fri, 23 Jun 2023 17:05:19 GMT, Naoto Sato wrote:
>> I'll run our CI, and if all good, I'll approve this PR. If nothing else,
>> this change seems reasonable and correct.
>
> Thanks Pavel for jumping in. I too think this change looks good.
Yep; all good.
-
PR Review Comment: htt
On Wed, 1 Feb 2023 10:36:12 GMT, Sergey Tsypanov wrote:
>> `ResourceBundle.CacheKey.equals()` and `Bundles.CacheKey.equals()` are quire
>> outdated. This simple clean-up modernizes them.
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last r
On Fri, 23 Jun 2023 21:59:21 GMT, Pavel Rappo wrote:
> NPE was seemingly caught to cover for other being null.
Unlike that of other file in this PR.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/12328#discussion_r1240446275
On Wed, 1 Feb 2023 10:36:12 GMT, Sergey Tsypanov wrote:
>> `ResourceBundle.CacheKey.equals()` and `Bundles.CacheKey.equals()` are quire
>> outdated. This simple clean-up modernizes them.
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last r
On Sun, 25 Jun 2023 18:17:31 GMT, Sergey Tsypanov wrote:
>> `ResourceBundle.CacheKey.equals()` and `Bundles.CacheKey.equals()` are quire
>> outdated. This simple clean-up modernizes them.
>
> Sergey Tsypanov has updated the pull request with a new target base due to a
> merge or a rebase. The i
On Sun, 25 Jun 2023 18:42:14 GMT, Pavel Rappo wrote:
> I'll re-run our CI, and if all good, I'll sponsor this PR.
The CI tests I started have just passed. While this PR is already good, I
wonder if we make it even better.
I doubt highly that we need null-checks for CacheKey'
On Sun, 25 Jun 2023 22:32:34 GMT, Pavel Rappo wrote:
>> I'll re-run our CI, and if all good, I'll sponsor this PR.
>
>> I'll re-run our CI, and if all good, I'll sponsor this PR.
>
> The CI tests I started have just passed. While this PR is already go
On Mon, 26 Jun 2023 09:46:08 GMT, Sergey Tsypanov wrote:
>> `ResourceBundle.CacheKey.equals()` and `Bundles.CacheKey.equals()` are quire
>> outdated. This simple clean-up modernizes them.
>
> Sergey Tsypanov has updated the pull request incrementally with one
> additional commit since the last
Please review this cleanup PR to normalize names of identifiers which are Java
variables/fields or tokens in text files. Those names either contain a pronoun
that is very rarely used in code, or seem like they contain such a pronoun,
which, in fact, they don't. Either way, the goal is to improve
On Mon, 26 Jun 2023 18:21:07 GMT, Roger Riggs wrote:
>> Please review this cleanup PR to normalize names of identifiers which are
>> Java variables/fields or tokens in text files. Those names either contain a
>> pronoun that is very rarely used in code, or seem like they contain such a
>> pron
On Mon, 26 Jun 2023 18:31:06 GMT, Jens Lidestrom wrote:
>> Please review this cleanup PR to normalize names of identifiers which are
>> Java variables/fields or tokens in text files. Those names either contain a
>> pronoun that is very rarely used in code, or seem like they contain such a
>> p
On Mon, 26 Jun 2023 18:44:42 GMT, Pavel Rappo wrote:
>> make/data/charsetmapping/charsets line 149:
>>
>>> 147: package sun.nio.cs
>>> 148: typesbcs
>>> 149: histname ISO8859_2
>>
>> Should this column be re-aligned with the lo
On Mon, 26 Jun 2023 14:07:03 GMT, Pavel Rappo wrote:
> Please review this cleanup PR to normalize names of identifiers which are
> Java variables/fields or tokens in text files. Those names either contain a
> pronoun that is very rarely used in code, or seem like they contain such a
On Wed, 28 Jun 2023 11:05:11 GMT, Sergey Tsypanov wrote:
> Double-checked locking should rely on local variable to avoid racy reads from
> volatile field.
To put this into perspective, the race you are trying to fix seems benign, so
the PR qualifies as a performance improvement, not a bug fix.
On Wed, 28 Jun 2023 11:05:11 GMT, Sergey Tsypanov wrote:
> Double-checked locking should rely on local variable to avoid racy reads from
> volatile field.
While I still cannot see any malignancy in that race, the proposed change makes
the code simpler to reason about and probably faster to exe
Please review this PR to use modern APIs and language features to simplify
`equals` and `hashCode` in the java.text area.
* Some changes to `equals` and `hashCode` are refactoring rather than
modernization. Such changes can be as trivial as rearranging, adding, or
commenting checks.
* java.tex
On Mon, 3 Jul 2023 13:22:27 GMT, Chen Liang wrote:
>> Please review this PR to use modern APIs and language features to simplify
>> `equals` and `hashCode` in the java.text area.
>>
>> * Some changes to `equals` and `hashCode` are refactoring rather than
>> modernization. Such changes can be a
On Tue, 4 Jul 2023 00:40:14 GMT, Chen Liang wrote:
> Hmm, I think that issue refers to code that have explicit non-Object
> parameter types (like `X::equals(Object)boolean` in the issue's sample). This
> method already have both arguments as `Object`, so I don't think there's any
> type-specif
On Wed, 5 Jul 2023 15:47:56 GMT, Roger Riggs wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Adhere to surrounding code style
>
> src/java.base/share/classes/java/text/DecimalFor
tests. Since writing new tests in this case seems
> unreasonable, I **excluded** those classes from this PR.
Pavel Rappo has updated the pull request incrementally with one additional
commit since the last revision:
Adhere to surrounding code style
-
Changes:
- all: https://
On Wed, 5 Jul 2023 15:46:56 GMT, Roger Riggs wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Adhere to surrounding code style
>
> src/java.base/share/classes/java/text/CompactNumberFor
tests. Since writing new tests in this case seems
> unreasonable, I **excluded** those classes from this PR.
Pavel Rappo 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 pu
On Wed, 5 Jul 2023 17:39:49 GMT, Roger Riggs wrote:
>> Do you mean we could use `obj instanceof DecimalFormat other` in front of
>> that 30-line `&&` expression, or that we could collapse the list to exactly
>> what you wrote?
>>
>> Separately, `super.equals` already performs the class check;
tests. Since writing new tests in this case seems
> unreasonable, I **excluded** those classes from this PR.
Pavel Rappo has updated the pull request incrementally with one additional
commit since the last revision:
Re-arrange checks
-
Changes:
- all: https://git.openjdk.org
On Wed, 5 Jul 2023 17:44:52 GMT, Roger Riggs wrote:
>> You are right, I have no stats. Performance-wise, it's already better than
>> what was there before. Before, there was no short-circuit check. But I can
>> go either way; I don't have a strong opinion.
>>
>> Reusing superclass' equals is n
On Wed, 5 Jul 2023 15:33:13 GMT, Roger Riggs wrote:
> I'd suggest replacing the calls to `valuesMatch` with `Objects.equals` and
> remove the `valuesMatch` method as unnecessary.
I'll do something about them soon, Roger. But first I need to understand
JDK-8015417 better, as it also affects oth
On Thu, 6 Jul 2023 14:46:59 GMT, Pavel Rappo wrote:
>> I'd suggest replacing the calls to `valuesMatch` with `Objects.equals` and
>> remove the `valuesMatch` method as unnecessary.
>
>> I'd suggest replacing the calls to `valuesMatch` with `Objects.equals`
On Thu, 6 Jul 2023 23:38:01 GMT, Stuart Marks wrote:
> For me the issue here is that there is a bunch of lore about avoiding
> `Objects::equals` and it's embodied in comments like this:
>
> > NB: Do not replace with Object.equals until JDK-8015417 is resolved.
>
> These comments are almost exa
tests. Since writing new tests in this case seems
> unreasonable, I **excluded** those classes from this PR.
Pavel Rappo has updated the pull request incrementally with one additional
commit since the last revision:
Feedback
-
Changes:
- all: https://git.openjdk.org
On Wed, 12 Jul 2023 22:26:14 GMT, John R Rose wrote:
> Yes, it would be, because `rp` has a statically observable type,
> `BigInteger`. And that observation will flow across the call to the helper
> method.
After all, my mental model is incorrect; sigh.
I read your write-up as though once we
On Mon, 3 Jul 2023 11:12:32 GMT, Pavel Rappo wrote:
> Please review this PR to use modern APIs and language features to simplify
> `equals` and `hashCode` in the java.text area.
>
> * Some changes to `equals` and `hashCode` are refactoring rather than
> modernization. Such ch
On Thu, 27 Jul 2023 06:46:46 GMT, John Jiang wrote:
> if (i < savingsInstantTransitions.length) {
> // javazic writes the last GMT offset into index 0!
> if (i < savingsInstantTransitions.length) {
> offsets[0] = standardOffsets[standardOffsets.length - 1] * 1000;
> nOffse
Please review this simple PR.
-
Commit messages:
- Initial commit
Changes: https://git.openjdk.org/jdk/pull/15382/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15382&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8314738
Stats: 124 lines in 28 files changed: 0 ins
On Tue, 22 Aug 2023 12:23:18 GMT, Mark Reinhold wrote:
> I wouldn’t update the copyright year as you have in some of these files.
I was taught to do it every time when I change a file. Would you like me to
revert those changes to copyright years in this case?
-
PR Comment: https:/
On Tue, 22 Aug 2023 08:42:32 GMT, Pavel Rappo wrote:
> Please review this simple PR.
This pull request has now been integrated.
Changeset: f39fc0aa
Author: Pavel Rappo
URL:
https://git.openjdk.org/jdk/commit/f39fc0aa2de19332fa51af605ece0660891d8c7a
Stats: 124 lines in 28 fi
Please review this trivial PR to reorder the `sealed` class or interface
modifier. For context of this change see:
https://github.com/openjdk/jdk/pull/17242#issuecomment-1887338396.
-
Commit messages:
- Initial commit
Changes: https://git.openjdk.org/jdk/pull/17468/files
Webrev:
On Thu, 18 Jan 2024 09:29:11 GMT, Magnus Ihse Bursie wrote:
> Thanks! When this has been integrated, I can take a shot at the missorted
> `default`s.
Thanks, I could've done that myself, but decided not to. You see, `default`
should ideally be a [sole] modifier on a method: all other modifiers
On Wed, 17 Jan 2024 21:22:07 GMT, Pavel Rappo wrote:
> Please review this trivial PR to reorder the `sealed` class or interface
> modifier. For context of this change see:
> https://github.com/openjdk/jdk/pull/17242#issuecomment-1887338396.
This pull request has now been i
On Thu, 18 Apr 2024 20:44:00 GMT, Jonathan Gibbons wrote:
> Please review a set of updates to clean up use of `/**` comments in the
> vicinity of declarations.
>
> There are various categories of update:
>
> * "Box comments" beginning with `/**`
> * Misplaced doc comments before package or imp
On Sat, 27 Apr 2024 10:33:42 GMT, Nizar Benalla wrote:
> Also have you looked at the output documentation? Without the `@inheritDoc`
> tags the content will only have a since tag, which is definitely wrong.
This is not how I remember it. Unless written around, `{@inheritDoc}` in a main
descrip
On Mon, 29 Apr 2024 22:54:47 GMT, Chen Liang wrote:
> would be nice if you can share more about these (also about the behaviors of
> inheriting `@throws` etc.
I hope this document explains it well; if it doesn't, we should fix it:
https://docs.oracle.com/en/java/javase/22/docs/specs/javadoc/do
On Thu, 25 Apr 2024 14:29:27 GMT, Nizar Benalla wrote:
> Please review this PR that aims to add all the remaining needed `@since` tags
> in `java.base`, and group them into a single fix.
> This is related to #18934 and my work around the `@since` checker feature.
> Explicit `@since` tags are nee
On Mon, 29 Apr 2024 17:46:24 GMT, Nizar Benalla wrote:
> Pavel, can I simply change the PR/issue title to be more descriptive? As I
> want to include the the inherit doc because of the unchecked exceptions,
> rather than clean this up in a different PR
I suggest dropping all the changes that a
Please review this PR, which supersedes a now withdrawn
https://github.com/openjdk/jdk/pull/14831.
This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more
user-friendly methods. Here's a summary:
- Made the operand constants (i.e. `T_BOOLEAN` and friends) and the
`vectorizedHash
t side, since the method
> is now private, it's no longer callable by clients of `ArraysSupport`, thus a
> problem of an inaccurate name is less severe.
>
> - Made the `ArraysSupport.utf16HashCode` method private
>
> - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport`
Pavel Ra
On Tue, 28 May 2024 20:38:21 GMT, Claes Redestad wrote:
>> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 301:
>>
>>> 299: return switch (length) {
>>> 300: case 0 -> initialValue;
>>> 301: case 1 -> 31 * initialValue + JLA.getUTF16Char(a,
On Tue, 28 May 2024 22:08:06 GMT, Pavel Rappo wrote:
>> Yes, should be `2` (`a` is semantically a `char[]`). This typo likely pass
>> functional testing since `1` can never happen in practice, and the default
>> case should work for any value. There might be a String
On Tue, 28 May 2024 20:21:34 GMT, Claes Redestad wrote:
>> test/hotspot/jtreg/compiler/intrinsics/TestArraysHashCode.java line 88:
>>
>>> 86: private static int testIntrinsic(byte[] bytes, int type)
>>> 87: throws InvocationTargetException, IllegalAccessException {
>>> 88:
On Wed, 29 May 2024 03:21:27 GMT, Chen Liang wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix incorrect utf16 hashCode adaptation
>
> src/java.base/share/classes/jdk/internal/ut
On Tue, 28 May 2024 20:40:30 GMT, Claes Redestad wrote:
>> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 275:
>>
>>> 273: return switch (length) {
>>> 274: case 0 -> initialValue;
>>> 275: case 1 -> 31 * initialValue + (a[fromIndex] & 0xff)
On Tue, 28 May 2024 19:13:50 GMT, Jorn Vernee wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix incorrect utf16 hashCode adaptation
>
> src/java.base/share/classes/jdk/internal/ut
t side, since the method
> is now private, it's no longer callable by clients of `ArraysSupport`, thus a
> problem of an inaccurate name is less severe.
>
> - Made the `ArraysSupport.utf16HashCode` method private
>
> - Moved tiny cases (i.e. 0, 1, 2) to `ArraysSupport`
Pa
On Wed, 29 May 2024 12:53:42 GMT, Pavel Rappo wrote:
>> src/java.base/share/classes/jdk/internal/util/ArraysSupport.java line 252:
>>
>>> 250: return switch (length) {
>>> 251: case 0 -> initialValue;
>>> 252: case
On Wed, 29 May 2024 12:44:45 GMT, Pavel Rappo wrote:
>> I don't care as long as microbenchmarks don't get a hiccup.
>
> @cl4es, here are some results from my machine (macosx-aarch64):
>
> Name (size) Cnt BaseError Te
On Wed, 29 May 2024 15:50:05 GMT, Pavel Rappo wrote:
>> @cl4es, here are some results from my machine (macosx-aarch64):
>>
>> Name (size) Cnt BaseError TestError
>> Unit Change
>> ArraysHashCode.bytes1
On Thu, 30 May 2024 08:34:59 GMT, Claes Redestad wrote:
>> @cl4es, do you want me to delete that test file altogether?
>
> I thought you verified that the non-constant type test still provoke a crash
> (on x86) if you back out the code changes in
> https://github.com/openjdk/jdk/commit/969f6a37
On Mon, 27 May 2024 16:28:31 GMT, Pavel Rappo wrote:
> Please review this PR, which supersedes a now withdrawn
> https://github.com/openjdk/jdk/pull/14831.
>
> This PR replaces `ArraysSupport.vectorizedHashCode` with a set of more
> user-friendly methods. Here's a summ
On Wed, 13 Sep 2023 17:38:28 GMT, Justin Lu wrote:
>> JDK .properties files still use ISO-8859-1 encoding with escape sequences.
>> It would improve readability to see the native characters instead of escape
>> sequences (especially for the L10n process). The majority of files changed
>> are l
On Fri, 13 Sep 2024 07:29:00 GMT, ExE Boss wrote:
> The old version of the doc comments had a `.` at the end of the first
> sentence:
The new version has it too, but in the final, generated form:
https://docs.oracle.com/en/java/javase/22/docs/specs/javadoc/doc-comment-spec.html#return
`{@retu
On Fri, 13 Sep 2024 02:47:18 GMT, Joe Darcy wrote:
> Candidates for this refactoring were found programmatically; the program to
> find candidates is in a comment on the bug.
Thanks, Joe. This looks good, especially considering that the change was
produced with the help of a quick-and-dirty, r
On Fri, 13 Sep 2024 13:07:03 GMT, Chen Liang wrote:
> This patch only captures one-line returns without extra sentences. Is it
> possible for us to handle slightly more complex documentations like
> `ParameterizedType::getActualTypeArguments`?
Sure, it is possible. However, there's a tradeoff
73 matches
Mail list logo
