On 11/14/13 2:04 AM, David Holmes wrote:
Sorry for the delay (been enroute). Only issue I have remains the subSequence
change - you can't weaken the post-condition of CharSequence.subSequence without
breaking subtype substitutability.
Hi David,
Yes, in general, what you say about weakening post-conditions is true. In this
case, though, I can't see how it can cause any practical harm.
I think the CharSequence spec for subSequence() is written with the assumption
that the CharSequence is mutable. If I'm not mistaken, all the CharSequence
implementations except for String are mutable. Thus, in the statement
Returns a new CharSequence that is a subsequence of this sequence.
the "new" really means that if the value of this sequence is later mutated, it
will not affect the value of the returned sequence. Well, strictly speaking,
it's ambiguous; the "new" could refer to either the sequences' values, the
reference identity of this vs the returned sequence, or both. I'd say that for
the mutable CharSequences, keeping the values independent is the important part,
and having independent values pretty much implies that the returned sequence is
a distinct object.
Since Strings are immutable, there is no concern about mutation of the value of
the original String affecting the value of the returned String. That leaves
reference identity. Obviously identity of mutable objects is really important.
For immutable objects, identity is much less important, and so I'm trying to
relax the spec to allow more discretion to the implementation about when to
create new objects.
I guess our choices are as follows:
0) Current webrev: remove "new" from String's spec for subSequence(). Allows
'this' to be returned in certain cases, but weakens the post-condition of its
definining interface, which is usually a no-no.
1) Leave String.subSequence() as it stands, requiring a new CharSequence. This
fixes the relationship with the CS interface, but the current implementation
violates this spec, as it returns 'this' in some cases (where the subSequence
represents the entire String).
2) Leave String.subSequence() as it stands, requiring a new CharSequence, and
change the implementation so that it always creates a new String, even if the
subSequence represents the entire original String.
3) Change the CharSequence.subSequence() spec to be more precise about what it
requires (mutation of one doesn't affect the other), allowing
String.subSequence() to return 'this' when it can.
Anything else?
I don't like (1) as it leaves our implementation in violation of a close reading
of the spec. I don't like (2) as it de-optimizes the current implementation. I
could live with (3) but it's more work, as new wording will have to be
generated, and its impact on the other CS implementations will need to be
assessed. It would probably result in the "best" (as in, most precise and
self-consistent) specification, but I have to question whether it's worth the
effort.
I still prefer (0), though I acknowledge it does have this issue of weakening
the interface's post-condition. I note that there are other places where an
implementation doesn't strictly conform to its interface, such as
IdentityHashMap and Map. Another possibility, I guess, would be to do something
like adding an @apiNote to String.subSequence() explaining why it's different
from CS.subSequence().
s'marks
Thanks,
David
On 12/11/2013 8:43 AM, Stuart Marks wrote:
Hi all,
Here's an updated version of the String spec change. Changes from the
previous version address comments made by Brent Christian and David
Holmes. Specifically:
- Specify copyValueOf() overloads as "equivalent to" corresponding
valueOf() overloads.
- Remove extranous changes to subSequence() method
- Clarify wording of concat() method doc.
Bug report:
https://bugs.openjdk.java.net/browse/JDK-7174936
Webrev:
http://cr.openjdk.java.net/~smarks/reviews/7174936/webrev.1/
Specdiff:
http://cr.openjdk.java.net/~smarks/reviews/7174936/specdiff.1/overview-summary.html
Thanks!
s'marks
On 11/7/13 2:31 AM, Stuart Marks wrote:
Hi all,
Please review the following spec changes to java.lang.String.
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.
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.
This change is spec only, no code changes.
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
