JDK-8302323 Add repeat methods to StringBuilder/StringBuffer #12728

JimLaskey · 2023-02-23T13:13:43Z

Add the ability to repeatedly append char and CharSequence data to StringBuilder/StringBuffer.

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Change requires CSR request JDK-8302686 to be approved
Commit message must refer to an issue

Issues

JDK-8302323: Add repeat methods to StringBuilder/StringBuffer
JDK-8302686: Add repeat methods to StringBuilder/StringBuffer (CSR)

Reviewers

Tagir F. Valeev (@amaembo - Committer) ⚠️ Review applies to 7d3ecc77
Claes Redestad (@cl4es - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/12728/head:pull/12728
$ git checkout pull/12728

Update a local copy of the PR:
$ git checkout pull/12728
$ git pull https://git.openjdk.org/jdk.git pull/12728/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 12728

View PR using the GUI difftool:
$ git pr show -t 12728

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/12728.diff

JimLaskey · 2023-02-23T13:13:56Z

/csr

bridgekeeper · 2023-02-23T13:14:41Z

👋 Welcome back jlaskey! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2023-02-23T13:17:39Z

@JimLaskey this pull request will not be integrated until the CSR request JDK-8302686 for issue JDK-8302323 has been approved.

openjdk · 2023-02-23T13:19:42Z

@JimLaskey The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2023-02-23T13:23:03Z

Webrevs

src/java.base/share/classes/java/lang/AbstractStringBuilder.java

src/java.base/share/classes/java/lang/String.java

src/java.base/share/classes/java/lang/AbstractStringBuilder.java

src/java.base/share/classes/java/lang/StringBuffer.java

src/java.base/share/classes/java/lang/AbstractStringBuilder.java

JimLaskey · 2023-03-03T14:57:10Z

Can I get some final reviews?

AlanBateman · 2023-03-03T15:22:02Z

The repeat codePoint throws IAE if the input is not a valid Unicode code point. Is there a test for that? The tests for IAE seem to be the negative repeat count but maybe I missed it? Also wondering if this needs tests for legacy StringBuffer.

JimLaskey · 2023-03-03T15:34:37Z

I agree. Will add IAE tests for bad code points and create as second set of tests for StringBuffer.

stsypanov · 2023-03-14T09:20:31Z

Should we also have overloaded method accepting position to insert repeated value? Smth like repeat(int pos, char c, int count)?

cl4es

It seems reasonable to consider analogous insert methods. Perhaps a more consistent naming scheme would be appendRepeated and insertRepeated? (The precedent for repeat in String::repeat is there, but is kind of weak since it's clear in that case that we're not mutating internal state)

src/java.base/share/classes/java/lang/AbstractStringBuilder.java

cl4es · 2023-03-28T10:48:48Z

test/jdk/java/lang/StringBuilder/StringBuilderRepeat.java

+        }
+
+        try {
+            sb.repeat(0x10FFFF + 1, -1);


Should these tests that test invalid codepoints use a valid repeat count as to verify the IAE is correctly thrown for the out-of-bound codepoint? Tests where both params are illegal are fine, too, of course.

cl4es

Looks ok. I don't have a strong opinion on insert-repeat.

I think tests that check for illegal code points should use a legal repeat value.

openjdk · 2023-03-28T10:53:33Z

@JimLaskey This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8302323: Add repeat methods to StringBuilder/StringBuffer

Reviewed-by: tvaleev, redestad

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 133 new commits pushed to the master branch:

cddaf68: 8304962: sun/net/www/http/KeepAliveCache/B5045306.java: java.lang.RuntimeException: Failed: Initial Keep Alive Connection is not being reused
a06f461: 8303214: Typo in java.util.Collections#synchronizedNavigableMap javadoc
7987ad4: 8304412: Serial: Refactor old generation cards update after Full GC
3c4cd50: 8304963: HttpServer closes connection after processing HEAD after JDK-7026262
4f625c0: 8304543: Modernize debugging jvm args in test/hotspot/jtreg/vmTestbase/nsk/jdi/Argument/value/value004.java
426025a: 8303526: Changing "arbitrary" Name.compareTo() ordering breaks the regression suite
6aec6f3: 8304931: vm/concepts/methods/methods001/methods00101m1/methods00101m1 failures with already pending exception
63ce88b: 8304147: JVM crash during shutdown when dumping dynamic archive
554bccf: 8304448: Kitchensink failed: assert(!thread->is_in_any_VTMS_transition()) failed: class prepare events are not allowed in any VTMS transition
3b88b2a: 8304761: Update IANA Language Subtag Registry to Version 2023-03-22
... and 123 more: https://git.openjdk.org/jdk/compare/7277bb19f128b84094400cb4262b2e0432e559c5...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

JimLaskey · 2023-04-03T15:27:07Z

/integrate

openjdk · 2023-04-03T15:29:34Z

Going to push as commit 9b9b5a7.
Since your change was applied there have been 211 commits pushed to the master branch:

dd7ca75: 8305478: [REDO] disable gtest/NMTGtests.java sub-tests failing due to JDK-8305414
f9827ad: 8288109: HttpExchangeImpl.setAttribute does not allow null value after JDK-8266897
6010de0: 8305417: disable gtest/NMTGtests.java sub-tests failing due to JDK-8305414
127afd3: 8241613: Suspicious calls to MacroAssembler::null_check(Register, offset)
33d09e5: 8305247: On RISC-V generate_fixed_frame() sometimes generate a relativized locals value which is way too large
790aced: 8305100: [REDO] Clean up JavadocTokenizer
2e91585: 8303123: Add line break opportunity to single type parameters
094e03d: 8299718: JavaDoc: Buttons to copy specific documentation URL are not accessible
4de24cd: 8303210: [linux, Windows] Make UseSystemMemoryBarrier available as product flag
336a23e: 8303229: JFR: Preserve disk repository after exit
... and 201 more: https://git.openjdk.org/jdk/compare/7277bb19f128b84094400cb4262b2e0432e559c5...master

Your commit was automatically rebased without conflicts.

openjdk · 2023-04-03T15:29:54Z

@JimLaskey Pushed as commit 9b9b5a7.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JimLaskey added 3 commits February 16, 2023 12:22

AbstractStringBuilder repeat

10ba7b6

Add synchronized to StringBuffer repeats

5acdf75

Clean up fro CSR

c4c1e76

openjdk bot added rfr Pull request is ready for review csr Pull request needs approved CSR before integration labels Feb 23, 2023

openjdk bot added the core-libs core-libs-dev@openjdk.org label Feb 23, 2023

rgiulietti reviewed Feb 23, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

RogerRiggs reviewed Feb 23, 2023

View reviewed changes

src/java.base/share/classes/java/lang/String.java Show resolved Hide resolved

RogerRiggs reviewed Feb 23, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

PR Cleanup

530f554

rgiulietti reviewed Feb 23, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

amaembo reviewed Feb 26, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Show resolved Hide resolved

amaembo reviewed Feb 26, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

Optimize for empty CharSequence

01ffb19

amaembo reviewed Feb 28, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Show resolved Hide resolved

amaembo reviewed Feb 28, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

AlanBateman reviewed Feb 28, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Show resolved Hide resolved

AlanBateman reviewed Feb 28, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

JimLaskey added 2 commits February 28, 2023 09:05

Change error report to use "is negative"

9da9ec9

Remove @throws IndexOutOfBoundsException

9a157ce

amaembo approved these changes Feb 28, 2023

View reviewed changes

RogerRiggs suggested changes Feb 28, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Show resolved Hide resolved

src/java.base/share/classes/java/lang/StringBuffer.java Show resolved Hide resolved

JimLaskey added 2 commits March 1, 2023 08:47

Move @SInCE

22a993b

Move @SInCE in subclasses

34c7948

turbanoff reviewed Mar 2, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

JimLaskey added 2 commits March 2, 2023 16:00

Support code points

02a4cc7

Add snippet

7d3ecc7

openjdk bot removed the csr Pull request needs approved CSR before integration label Mar 2, 2023

amaembo approved these changes Mar 3, 2023

View reviewed changes

Expand test for StringBuffer and illegal code points

1a77988

RogerRiggs mentioned this pull request Mar 13, 2023

8300818: Reduce complexity of padding with DateTimeFormatter #12131

Closed

3 tasks

cl4es reviewed Mar 14, 2023

View reviewed changes

src/java.base/share/classes/java/lang/AbstractStringBuilder.java Outdated Show resolved Hide resolved

JimLaskey added 2 commits March 27, 2023 15:23

Merge branch 'master' into 8302323

54a34cf

Use Arrays.fill

796406b

cl4es reviewed Mar 28, 2023

View reviewed changes

cl4es approved these changes Mar 28, 2023

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Mar 28, 2023

openjdk bot added the integrated Pull request has been integrated label Apr 3, 2023

openjdk bot closed this Apr 3, 2023

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Apr 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JDK-8302323 Add repeat methods to StringBuilder/StringBuffer #12728

JDK-8302323 Add repeat methods to StringBuilder/StringBuffer #12728

JimLaskey commented Feb 23, 2023 •

edited by openjdk bot

JimLaskey commented Feb 23, 2023

bridgekeeper bot commented Feb 23, 2023

openjdk bot commented Feb 23, 2023

openjdk bot commented Feb 23, 2023

mlbridge bot commented Feb 23, 2023 •

edited

JimLaskey commented Mar 3, 2023

AlanBateman commented Mar 3, 2023

JimLaskey commented Mar 3, 2023

stsypanov commented Mar 14, 2023

cl4es left a comment

cl4es Mar 28, 2023

cl4es left a comment

openjdk bot commented Mar 28, 2023

JimLaskey commented Apr 3, 2023

openjdk bot commented Apr 3, 2023

openjdk bot commented Apr 3, 2023

JDK-8302323 Add repeat methods to StringBuilder/StringBuffer #12728

JDK-8302323 Add repeat methods to StringBuilder/StringBuffer #12728

Conversation

JimLaskey commented Feb 23, 2023 • edited by openjdk bot

Progress

Issues

Reviewers

Reviewing

JimLaskey commented Feb 23, 2023

bridgekeeper bot commented Feb 23, 2023

openjdk bot commented Feb 23, 2023

openjdk bot commented Feb 23, 2023

mlbridge bot commented Feb 23, 2023 • edited

Webrevs

JimLaskey commented Mar 3, 2023

AlanBateman commented Mar 3, 2023

JimLaskey commented Mar 3, 2023

stsypanov commented Mar 14, 2023

cl4es left a comment

Choose a reason for hiding this comment

cl4es Mar 28, 2023

Choose a reason for hiding this comment

cl4es left a comment

Choose a reason for hiding this comment

openjdk bot commented Mar 28, 2023

JimLaskey commented Apr 3, 2023

openjdk bot commented Apr 3, 2023

openjdk bot commented Apr 3, 2023

JimLaskey commented Feb 23, 2023 •

edited by openjdk bot

mlbridge bot commented Feb 23, 2023 •

edited