On 11/6/13 6:15 PM, David Holmes wrote:
On 7/11/2013 11:31 AM, Stuart Marks wrote:
In several places the specs mention returning "new" strings. This is
overspecified; it's sufficient to return "a" string that satisfies the
required properties. In some cases the current implementation doesn't
create a new string -- for example, substring(0) returns 'this' -- which
is strictly a violation of the spec. The solution is to relax the spec
requirement to create new strings.

Or modify it as per concat - which is spec'd to return this when you concat an
empty string.

But I suppose in principle this allows for reuse of existing String objects.

I'm mainly interested in relaxing the spec, allowing the implementation maximum freedom to choose whether to create a new string or return a pre-existing instance such as 'this', instead of requiring one way or the other.

The concat() method is weird in that if the arg is empty, it requires returning 'this'; but if 'this' is empty, it requires creating a new String instead of just returning the arg. I think this is overspecified. But the implementation follows this exactly, so I didn't want to change the spec in this case. Maybe later.

Also, this change cleans up the specs for the copyValueOf() overloads to
make them identical to the corresponding valueOf() overloads.
Previously, they were gratuitously different. I think that some time in
the dim, distant past, probably before JDK 1.0, they had different
semantics, but now they're identical. The specs should say they're
identical.

Don't we normally express this as "Equivalent to ..." rather than "Identical to
..." - the latter seems like an overspecification.

Yes, "Equivalent to..." is better; I'll change it.

Surely this change invalidates any tests that check for "new" strings as per the
spec? They won't start failing but they will no longer be valid test.

For methods where the implementation violates the spec by not returning a new instance, any tests that exist for these cases would be failing, so I'd think we'd have heard about this already. So perhaps such tests don't exist.

For methods where the implementation does return a new string, relaxing the spec may indeed invalidate tests that test for a new string. Such a test wouldn't actually fail until some future time when the implementation is changed.

But yes, this change might have some impact on the JCK. This is going through the normal internal review process (CCC) though, so the JCK team should be properly informed of the change.

That aside subSequence is specified by CharSequence to return a new
CharSequence. So I don't think you can simply remove that requirement.

Oh yes, good catch. I had originally modified subSequence to change all the mentions of CharSequence to String -- since String.subSequence() merely calls String.substring(). After talking to Mike Duigou about this we decided that it would be better to leave these all as CharSequence. I changed them all back, but I missed this one in the @return clause. I'll fix this too.

Thanks,

s'marks



Cheers,
David
-----

Bug report:

     https://bugs.openjdk.java.net/browse/jdk-7174936

Webrev:

     http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.0/

Specdiff:


http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.0/overview-summary.html



Thanks!

s'marks

Reply via email to