Hi Matthias, thanks for your careful review of this tedious change.
I think the change in jdk.internal.module.Checks.java is ok to backport. It is no public interface and regression tests show no issue. So I wouldn't modify the patch in this place. Any other opinions? Best regards Christoph From: Baesken, Matthias Sent: Dienstag, 18. Juni 2019 15:16 To: Langer, Christoph <[email protected]>; [email protected] Cc: Java Core Libs <[email protected]> Subject: RE: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base Hello Christoph, looks good with the exception of src/java.base/share/classes/jdk/internal/module/Checks.java http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/src/java.base/share/classes/jdk/internal/module/Checks.java.frames.html /** - * Returns {@code true} if the given name is a legal module name. - */ - public static boolean isModuleName(String name) { .... - private static boolean isJavaIdentifier(CharSequence cs) { - if (cs.length() == 0 || RESERVED.contains(cs)) + private static boolean isJavaIdentifier(String str) { + if (str.isEmpty() || RESERVED.contains(str)) ... where I am a bit unsure - should we really remove a method in jdk11 (and change signature of another) ? I know , it is from jdk/internal , but still I have a bad feeling about this . This was probably fine for the higher release but for jdk11 it might be better to keep what we have . Best regards, Matthias From: Langer, Christoph Sent: Dienstag, 18. Juni 2019 10:59 To: [email protected]<mailto:[email protected]> Cc: Java Core Libs <[email protected]<mailto:[email protected]>>; Baesken, Matthias <[email protected]<mailto:[email protected]>> Subject: [11u]: RFR: 8215281: Use String.isEmpty() when applicable in java.base Hi, please review the backport of the String.isEmpty() cleanups in java.base. The main reason to bring this down to JDK11u is that it'll ease backporting of later changes which otherwise run into conflicts and won't resolve. Furthermore, the original change is a nice cleanup and improves performance. Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8215281.11u/ Bug: https://bugs.openjdk.java.net/browse/JDK-8215281 Change in jdk/jdk: http://hg.openjdk.java.net/jdk/jdk/rev/8bf9268df0e2 The original patch merely applies (with a little fuzz in some places). There were 4 rejects: src/java.base/share/classes/java/lang/VersionProps.java.template --- VersionProps.java.template +++ VersionProps.java.template @@ -95,7 +95,7 @@ props.put("java.version.date", java_version_date); props.put("java.runtime.version", java_runtime_version); props.put("java.runtime.name", java_runtime_name); - if (VENDOR_VERSION_STRING.length() > 0) + if (!VENDOR_VERSION_STRING.isEmpty()) props.put("java.vendor.version", VENDOR_VERSION_STRING); props.put("java.class.version", CLASSFILE_MAJOR_MINOR); -> manually resolved that to the right location src/java.base/share/classes/java/text/CompactNumberFormat.java -> file does not exist in 11u, so dropping src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckMethodAdapter.java --- CheckMethodAdapter.java +++ CheckMethodAdapter.java @@ -1314,7 +1314,7 @@ * @param message the message to use in case of error. */ static void checkMethodIdentifier(final int version, final String name, final String message) { - if (name == null || name.length() == 0) { + if (name == null || name.isEmpty()) { throw new IllegalArgumentException(INVALID + message + MUST_NOT_BE_NULL_OR_EMPTY); } if ((version & 0xFFFF) >= Opcodes.V1_5) { @@ -1347,7 +1347,7 @@ * @param message the message to use in case of error. */ static void checkInternalName(final int version, final String name, final String message) { - if (name == null || name.length() == 0) { + if (name == null || name.isEmpty()) { throw new IllegalArgumentException(INVALID + message + MUST_NOT_BE_NULL_OR_EMPTY); } if (name.charAt(0) == '[') { @@ -1457,7 +1457,7 @@ * @param descriptor the string to be checked. */ static void checkMethodDescriptor(final int version, final String descriptor) { - if (descriptor == null || descriptor.length() == 0) { + if (descriptor == null || descriptor.isEmpty()) { throw new IllegalArgumentException("Invalid method descriptor (must not be null or empty)"); } if (descriptor.charAt(0) != '(' || descriptor.length() < 3) { -> file looks different in 11u due to a missing change. So, modified the right places in the 11u version src/java.base/share/classes/jdk/internal/org/objectweb/asm/util/CheckSignatureAdapter.java --- CheckSignatureAdapter.java +++ CheckSignatureAdapter.java @@ -365,7 +365,7 @@ } private void checkClassName(final String name, final String message) { - if (name == null || name.length() == 0) { + if (name == null || name.isEmpty()) { throw new IllegalArgumentException(INVALID + message + " (must not be null or empty)"); } for (int i = 0; i < name.length(); ++i) { @@ -377,7 +377,7 @@ } private void checkIdentifier(final String name, final String message) { - if (name == null || name.length() == 0) { + if (name == null || name.isEmpty()) { throw new IllegalArgumentException(INVALID + message + " (must not be null or empty)"); } for (int i = 0; i < name.length(); ++i) { -> file looks different in 11u due to a missing change. So, modified the right places in the 11u version Thanks Christoph
