On Mon, 21 Jul 2025 12:10:51 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
> `NoRepl`-suffixed `String` methods denote methods that do not replace invalid > characters, but throw `CharacterCodingException` on encounter. This behavior > cannot easily be derived from the method footprints, has been a source of > confusion for maintainers, and is not uniformly adopted, e.g., > `newStringUTF8NoRepl()` and `getBytesUTF8NoRepl()` does *not* throw `CCE`. > This PR removes `NoRepl` suffix from method names and consistently uses > `throws CCE` in method footprints. (b4845109e18 passes `tier1,2`.) newStringNoRepl has confused several maintainers as it's not immediately obvious that it means "no replace", or more specifically CodingErrorAction.REPORT behavior. So it's good to see this getting attention. Anyone working on String expects CodingErrorAction.REPLACE behavior so having String methods use methods that don't throw would feel right. There are "far away" APIs, Files.readXXX was mentioned, that require CodingErrorAction.REPORT behavior methods so having JLA methods throw CharacterCodingException is helpful. There are other far away APIs such as UUID and ZipCoder that specify a Charset and know that no exception will be throw. They want CodingErrorAction.REPORT behavior except that it's a bug if CharacterCodingException is thrown. These cases catch CCE and throw AssertionError, which is okay, and hopefully we have enough tests in all these area to ensure that it doesn't happen. Anyway, from a first read of the changes then I think we need to make sure that the method descriptions are accurate. There are several places where "IAE" is mentioned but the methods are changed to throw CCE. IAE is awkward in this area because throwing it, instead of CCE, makes it difficult to know if the handling is correct. There is constant code churn in this area and too easy to introduce a regression and "unexpected exception" if IAE is used to in place of a CharacterCodingException. Another high level comment from a first read is that it feels like there are two forms needed. One form is REPLACE action and doesn't throw. The other is REPORT action and throws CharacterCodingException. That is what we have already, it's just the naming is awkward. So it may be that it's more about finding good names so that it's clear from all the use sites. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26413#issuecomment-3103589181
