Skip to content

8357063: Document preconditions for DecimalDigits methods #25246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

8357063: Document preconditions for DecimalDigits methods #25246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
66f71c5
Document preconditions for DecimalDigits methods
wenshao May 15, 2025
381979d
document add warnings
wenshao May 16, 2025
ddb44aa
assert
wenshao May 16, 2025
f88d235
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
121695a
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
a31d965
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
c281623
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
27e883c
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
aa9d7a3
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
bcb9af2
Update src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
wenshao May 16, 2025
da865e4
warning
wenshao May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/java.base/share/classes/java/lang/AbstractStringBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,9 +924,9 @@ public AbstractStringBuilder append(int i) {
int spaceNeeded = count + DecimalDigits.stringSize(i);
byte[] value = ensureCapacitySameCoder(this.value, coder, spaceNeeded);
if (isLatin1(coder)) {
DecimalDigits.getCharsLatin1(i, spaceNeeded, value);
DecimalDigits.uncheckedGetCharsLatin1(i, spaceNeeded, value);
} else {
DecimalDigits.getCharsUTF16(i, spaceNeeded, value);
DecimalDigits.uncheckedGetCharsUTF16(i, spaceNeeded, value);
}
this.value = value;
this.count = spaceNeeded;
Expand All @@ -951,9 +951,9 @@ public AbstractStringBuilder append(long l) {
int spaceNeeded = count + DecimalDigits.stringSize(l);
byte[] value = ensureCapacitySameCoder(this.value, coder, spaceNeeded);
if (isLatin1(coder)) {
DecimalDigits.getCharsLatin1(l, spaceNeeded, value);
DecimalDigits.uncheckedGetCharsLatin1(l, spaceNeeded, value);
} else {
DecimalDigits.getCharsUTF16(l, spaceNeeded, value);
DecimalDigits.uncheckedGetCharsUTF16(l, spaceNeeded, value);
}
this.value = value;
this.count = spaceNeeded;
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/lang/Integer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,11 +432,11 @@ public static String toString(int i) {
int size = DecimalDigits.stringSize(i);
if (COMPACT_STRINGS) {
byte[] buf = new byte[size];
DecimalDigits.getCharsLatin1(i, size, buf);
DecimalDigits.uncheckedGetCharsLatin1(i, size, buf);
return new String(buf, LATIN1);
} else {
byte[] buf = new byte[size * 2];
DecimalDigits.getCharsUTF16(i, size, buf);
DecimalDigits.uncheckedGetCharsUTF16(i, size, buf);
return new String(buf, UTF16);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/lang/Long.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,11 +462,11 @@ public static String toString(long i) {
int size = DecimalDigits.stringSize(i);
if (COMPACT_STRINGS) {
byte[] buf = new byte[size];
DecimalDigits.getCharsLatin1(i, size, buf);
DecimalDigits.uncheckedGetCharsLatin1(i, size, buf);
return new String(buf, LATIN1);
} else {
byte[] buf = new byte[size * 2];
DecimalDigits.getCharsUTF16(i, size, buf);
DecimalDigits.uncheckedGetCharsUTF16(i, size, buf);
return new String(buf, UTF16);
}
}
Expand Down
16 changes: 8 additions & 8 deletions src/java.base/share/classes/java/lang/StringConcatHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,12 +315,12 @@ static long prepend(long indexCoder, byte[] buf, char value, String prefix) {
static long prepend(long indexCoder, byte[] buf, int value, String prefix) {
int index = (int)indexCoder;
if (indexCoder < UTF16) {
index = DecimalDigits.getCharsLatin1(value, index, buf);
index = DecimalDigits.uncheckedGetCharsLatin1(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.LATIN1);
return index;
} else {
index = DecimalDigits.getCharsUTF16(value, index, buf);
index = DecimalDigits.uncheckedGetCharsUTF16(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.UTF16);
return index | UTF16;
Expand All @@ -341,12 +341,12 @@ static long prepend(long indexCoder, byte[] buf, int value, String prefix) {
static long prepend(long indexCoder, byte[] buf, long value, String prefix) {
int index = (int)indexCoder;
if (indexCoder < UTF16) {
index = DecimalDigits.getCharsLatin1(value, index, buf);
index = DecimalDigits.uncheckedGetCharsLatin1(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.LATIN1);
return index;
} else {
index = DecimalDigits.getCharsUTF16(value, index, buf);
index = DecimalDigits.uncheckedGetCharsUTF16(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.UTF16);
return index | UTF16;
Expand Down Expand Up @@ -713,11 +713,11 @@ static int prepend(int index, byte coder, byte[] buf, char value, String prefix)
*/
static int prepend(int index, byte coder, byte[] buf, int value, String prefix) {
if (coder == String.LATIN1) {
index = DecimalDigits.getCharsLatin1(value, index, buf);
index = DecimalDigits.uncheckedGetCharsLatin1(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.LATIN1);
} else {
index = DecimalDigits.getCharsUTF16(value, index, buf);
index = DecimalDigits.uncheckedGetCharsUTF16(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.UTF16);
}
Expand All @@ -737,11 +737,11 @@ static int prepend(int index, byte coder, byte[] buf, int value, String prefix)
*/
static int prepend(int index, byte coder, byte[] buf, long value, String prefix) {
if (coder == String.LATIN1) {
index = DecimalDigits.getCharsLatin1(value, index, buf);
index = DecimalDigits.uncheckedGetCharsLatin1(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.LATIN1);
} else {
index = DecimalDigits.getCharsUTF16(value, index, buf);
index = DecimalDigits.uncheckedGetCharsUTF16(value, index, buf);
index -= prefix.length();
prefix.getBytes(buf, index, String.UTF16);
}
Expand Down
4 changes: 2 additions & 2 deletions src/java.base/share/classes/java/math/BigDecimal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4138,9 +4138,9 @@ private String layoutChars(boolean sci) {
int highInt = (int)intCompact / 100;
int highIntSize = DecimalDigits.stringSize(highInt);
byte[] buf = new byte[highIntSize + 3];
DecimalDigits.getCharsLatin1(highInt, highIntSize, buf);
DecimalDigits.uncheckedGetCharsLatin1(highInt, highIntSize, buf);
buf[highIntSize] = '.';
DecimalDigits.putPairLatin1(buf, highIntSize + 1, lowInt);
DecimalDigits.uncheckedPutPairLatin1(buf, highIntSize + 1, lowInt);
try {
return JLA.newStringNoRepl(buf, StandardCharsets.ISO_8859_1);
} catch (CharacterCodingException cce) {
Expand Down
87 changes: 49 additions & 38 deletions src/java.base/share/classes/jdk/internal/util/DecimalDigits.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,15 @@ public static int stringSize(long x) {
* values, to cover the Integer.MIN_VALUE case. Converting otherwise
* (negative to positive) will expose -Integer.MIN_VALUE that overflows
* integer.
* <p>
* <b>WARNING: This method does not perform any bound checks. </b>
*
* @param i value to convert
* @param index next index, after the least significant digit
* @param buf target buffer, Latin1-encoded
* @return index of the most significant digit or minus sign, if present
*/
public static int getCharsLatin1(int i, int index, byte[] buf) {
// Used by trusted callers. Assumes all necessary bounds checks have been done by the caller.
public static int uncheckedGetCharsLatin1(int i, int index, byte[] buf) {
int q;
int charPos = index;

Expand All @@ -163,20 +164,20 @@ public static int getCharsLatin1(int i, int index, byte[] buf) {
while (i <= -100) {
q = i / 100;
charPos -= 2;
putPairLatin1(buf, charPos, (q * 100) - i);
uncheckedPutPairLatin1(buf, charPos, (q * 100) - i);
i = q;
}

// We know there are at most two digits left at this point.
if (i <= -10) {
charPos -= 2;
putPairLatin1(buf, charPos, -i);
uncheckedPutPairLatin1(buf, charPos, -i);
} else {
putCharLatin1(buf, --charPos, '0' - i);
uncheckedPutCharLatin1(buf, --charPos, '0' - i);
}

if (negative) {
putCharLatin1(buf, --charPos, '-');
uncheckedPutCharLatin1(buf, --charPos, '-');
}
return charPos;
}
Expand All @@ -193,14 +194,15 @@ public static int getCharsLatin1(int i, int index, byte[] buf) {
* values, to cover the Long.MIN_VALUE case. Converting otherwise
* (negative to positive) will expose -Long.MIN_VALUE that overflows
* long.
* <p>
* <b>WARNING: This method does not perform any bound checks. </b>
*
* @param i value to convert
* @param index next index, after the least significant digit
* @param buf target buffer, Latin1-encoded
* @return index of the most significant digit or minus sign, if present
*/
public static int getCharsLatin1(long i, int index, byte[] buf) {
// Used by trusted callers. Assumes all necessary bounds checks have been done by the caller.
public static int uncheckedGetCharsLatin1(long i, int index, byte[] buf) {
long q;
int charPos = index;

Expand All @@ -213,7 +215,7 @@ public static int getCharsLatin1(long i, int index, byte[] buf) {
while (i < Integer.MIN_VALUE) {
q = i / 100;
charPos -= 2;
putPairLatin1(buf, charPos, (int)((q * 100) - i));
uncheckedPutPairLatin1(buf, charPos, (int)((q * 100) - i));
i = q;
}

Expand All @@ -223,36 +225,37 @@ public static int getCharsLatin1(long i, int index, byte[] buf) {
while (i2 <= -100) {
q2 = i2 / 100;
charPos -= 2;
putPairLatin1(buf, charPos, (q2 * 100) - i2);
uncheckedPutPairLatin1(buf, charPos, (q2 * 100) - i2);
i2 = q2;
}

// We know there are at most two digits left at this point.
if (i2 <= -10) {
charPos -= 2;
putPairLatin1(buf, charPos, -i2);
uncheckedPutPairLatin1(buf, charPos, -i2);
} else {
putCharLatin1(buf, --charPos, '0' - i2);
uncheckedPutCharLatin1(buf, --charPos, '0' - i2);
}

if (negative) {
putCharLatin1(buf, --charPos, '-');
uncheckedPutCharLatin1(buf, --charPos, '-');
}
return charPos;
}


/**
* This is a variant of {@link DecimalDigits#getCharsLatin1(int, int, byte[])}, but for
* This is a variant of {@link DecimalDigits#uncheckedGetCharsLatin1(int, int, byte[])}, but for
* UTF-16 coder.
* <p>
* <b>WARNING: This method does not perform any bound checks.</b>
*
* @param i value to convert
* @param index next index, after the least significant digit
* @param buf target buffer, UTF16-coded.
* @return index of the most significant digit or minus sign, if present
*/
public static int getCharsUTF16(int i, int index, byte[] buf) {
// Used by trusted callers. Assumes all necessary bounds checks have been done by the caller.
public static int uncheckedGetCharsUTF16(int i, int index, byte[] buf) {
int q;
int charPos = index;

Expand All @@ -265,36 +268,37 @@ public static int getCharsUTF16(int i, int index, byte[] buf) {
while (i <= -100) {
q = i / 100;
charPos -= 2;
putPairUTF16(buf, charPos, (q * 100) - i);
uncheckedPutPairUTF16(buf, charPos, (q * 100) - i);
i = q;
}

// We know there are at most two digits left at this point.
if (i <= -10) {
charPos -= 2;
putPairUTF16(buf, charPos, -i);
uncheckedPutPairUTF16(buf, charPos, -i);
} else {
putCharUTF16(buf, --charPos, '0' - i);
uncheckedPutCharUTF16(buf, --charPos, '0' - i);
}

if (negative) {
putCharUTF16(buf, --charPos, '-');
uncheckedPutCharUTF16(buf, --charPos, '-');
}
return charPos;
}


/**
* This is a variant of {@link DecimalDigits#getCharsLatin1(long, int, byte[])}, but for
* This is a variant of {@link DecimalDigits#uncheckedGetCharsLatin1(long, int, byte[])}, but for
* UTF-16 coder.
* <p>
* <b>WARNING: This method does not perform any bound checks.</b>
*
* @param i value to convert
* @param index next index, after the least significant digit
* @param buf target buffer, UTF16-coded.
* @return index of the most significant digit or minus sign, if present
*/
public static int getCharsUTF16(long i, int index, byte[] buf) {
// Used by trusted callers. Assumes all necessary bounds checks have been done by the caller.
public static int uncheckedGetCharsUTF16(long i, int index, byte[] buf) {
long q;
int charPos = index;

Expand All @@ -307,7 +311,7 @@ public static int getCharsUTF16(long i, int index, byte[] buf) {
while (i < Integer.MIN_VALUE) {
q = i / 100;
charPos -= 2;
putPairUTF16(buf, charPos, (int)((q * 100) - i));
uncheckedPutPairUTF16(buf, charPos, (int)((q * 100) - i));
i = q;
}

Expand All @@ -317,26 +321,26 @@ public static int getCharsUTF16(long i, int index, byte[] buf) {
while (i2 <= -100) {
q2 = i2 / 100;
charPos -= 2;
putPairUTF16(buf, charPos, (q2 * 100) - i2);
uncheckedPutPairUTF16(buf, charPos, (q2 * 100) - i2);
i2 = q2;
}

// We know there are at most two digits left at this point.
if (i2 <= -10) {
charPos -= 2;
putPairUTF16(buf, charPos, -i2);
uncheckedPutPairUTF16(buf, charPos, -i2);
} else {
putCharUTF16(buf, --charPos, '0' - i2);
uncheckedPutCharUTF16(buf, --charPos, '0' - i2);
}

if (negative) {
putCharUTF16(buf, --charPos, '-');
uncheckedPutCharUTF16(buf, --charPos, '-');
}
return charPos;
}

/**
* This is a variant of {@link DecimalDigits#getCharsUTF16(long, int, byte[])}, but for
* This is a variant of {@link DecimalDigits#uncheckedGetCharsUTF16(long, int, byte[])}, but for
* UTF-16 coder.
*
* @param i value to convert
Expand All @@ -345,7 +349,6 @@ public static int getCharsUTF16(long i, int index, byte[] buf) {
* @return index of the most significant digit or minus sign, if present
*/
public static int getChars(long i, int index, char[] buf) {
// Used by trusted callers. Assumes all necessary bounds checks have been done by the caller.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Shaojin, I think this was a misplaced comment previously. Looking at the implementation of this method, there's no "unsafe" access happening in this method's implementation. It ends up calling putChar which does a Java style array access and thus is backed by the language's bounds checking.

Removing this comment I believe is the right thing. Having said that, I am unsure the javadoc comment of this method should refer to DecimalDigits#uncheckedGetCharsUTF16 because that is confusing and misleading.

Should we change the javadoc text of this method to:

Places characters representing the long i into the character array buf. The characters are placed into the buffer backwards starting with the least significant digit at the specified index (exclusive), and working backwards from there.

Would that accurately describe what this method's implementation currently does?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method has the same algorithm as uncheckedGetCharsUTF16, the only difference is the safe array access of char[] and the unsafe access of byte[] by uncheckedPutPairUTF16.

This method was also copied from uncheckedGetCharsUTF16 and then modified when the code was written, so I think the reference here is OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/integrate

long q;
int charPos = index;

Expand Down Expand Up @@ -402,34 +405,42 @@ public static void putPair(char[] buf, int charPos, int v) {
/**
* Insert the 2-bytes integer into the buf as 2 decimal digit ASCII bytes,
* only least significant 16 bits of {@code v} are used.
* <p>
* <b>WARNING: This method does not perform any bound checks.</b>
*
* @param buf byte buffer to copy into
* @param charPos insert point
* @param v to convert
*/
public static void putPairLatin1(byte[] buf, int charPos, int v) {
public static void uncheckedPutPairLatin1(byte[] buf, int charPos, int v) {
int packed = DIGITS[v & 0x7f];
putCharLatin1(buf, charPos, packed & 0xFF);
putCharLatin1(buf, charPos + 1, packed >> 8);
uncheckedPutCharLatin1(buf, charPos, packed & 0xFF);
uncheckedPutCharLatin1(buf, charPos + 1, packed >> 8);
}

/**
* Insert the 2-chars integer into the buf as 2 decimal digit UTF16 bytes,
* only least significant 16 bits of {@code v} are used.
* <p>
* <b>WARNING: This method does not perform any bound checks.</b>
*
* @param buf byte buffer to copy into
* @param charPos insert point
* @param v to convert
*/
public static void putPairUTF16(byte[] buf, int charPos, int v) {
public static void uncheckedPutPairUTF16(byte[] buf, int charPos, int v) {
int packed = DIGITS[v & 0x7f];
putCharUTF16(buf, charPos, packed & 0xFF);
putCharUTF16(buf, charPos + 1, packed >> 8);
uncheckedPutCharUTF16(buf, charPos, packed & 0xFF);
uncheckedPutCharUTF16(buf, charPos + 1, packed >> 8);
}

private static void putCharLatin1(byte[] buf, int charPos, int c) {
private static void uncheckedPutCharLatin1(byte[] buf, int charPos, int c) {
assert charPos >= 0 && charPos < buf.length;
UNSAFE.putByte(buf, ARRAY_BYTE_BASE_OFFSET + charPos, (byte) c);
}

private static void putCharUTF16(byte[] buf, int charPos, int c) {
private static void uncheckedPutCharUTF16(byte[] buf, int charPos, int c) {
assert charPos >= 0 && charPos < (buf.length >> 1);
UNSAFE.putCharUnaligned(buf, ARRAY_BYTE_BASE_OFFSET + ((long) charPos << 1), (char) c);
}
}
4 changes: 2 additions & 2 deletions test/hotspot/jtreg/compiler/patches/java.base/java/lang/Helper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,14 @@ public static int codePointCountSB(byte[] val, int beginIndex, int endIndex) {

public static int getChars(int i, int begin, int end, byte[] value) {
StringUTF16.checkBoundsBeginEnd(begin, end, value);
int pos = DecimalDigits.getCharsUTF16(i, end, value);
int pos = DecimalDigits.uncheckedGetCharsUTF16(i, end, value);
assert begin == pos;
return pos;
}

public static int getChars(long l, int begin, int end, byte[] value) {
StringUTF16.checkBoundsBeginEnd(begin, end, value);
int pos = DecimalDigits.getCharsUTF16(l, end, value);
int pos = DecimalDigits.uncheckedGetCharsUTF16(l, end, value);
assert begin == pos;
return pos;
}
Expand Down