Skip to content
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.

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

Restore CharSequences.newAsciiString(String) #1503

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

bondolo
Copy link
Contributor

@bondolo bondolo commented Apr 19, 2021

Motivation:
In ServiceTalk 0.39.0 the method
CharSequences.newAsciiString(CharSequence) was introduced which
replaced CharSequences.newAsciiString(String). This was a source
compatible, but binary incompatible change. All existing uses of the
String overload could be recompiled to use the CharSequence
overload. Existing binary code expected the String overload to be present
and fails with NoSuchMethodError at runtime.
Modifications:
For cases where the libraries and applications using ServiceTalk cannot
be recompiled conveniently the CharSequences.newAsciiString(String)
overload is restored.
Result:
Binary compatibility with libraries and applications which use
CharSequences.newAsciiString(String) and are compiled against
versions of ServiceTalk before 0.39.0 is restored.

Motivation:
In ServiceTalk 0.39.0 the method
`CharSequences.newAsciiString(CharSequence)` was introduced which
replaced `CharSequences.newAsciiString(String)`. This was a source
compatible, but binary incompatible change. All existing uses of the
`String` overload could be recompiled to use the `CharSequence`
overload. Existing binary code expected the `String` overload to Existing
and fails with `NoSuchMethodError` at runtime.
Modifications:
For cases where the libraries and applications using ServiceTalk cannot
be recompiled conveniently the `CharSequences.newAsciiString(String)`
overload is restored.
Result:
Binary compatibility with libraries and applications which use
`CharSequences.newAsciiString(String)` and are compiled against
versions of ServiceTalk before 0.39.0 is restored.
@bondolo bondolo self-assigned this Apr 19, 2021
*
* @param input a character sequence containing only 8-bit ASCII characters.
* @return a {@link CharSequence}
*/
Copy link
Member

Choose a reason for hiding this comment

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

add @Deprecated annotation and javaodc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't want to add deprecation because the only fix for user code is to add an explicit upcast to CharSequence which will be redundant once the String method is actually removed. We should investigate solutions to add an invisible bridge method for the String overload so that source compiles to the CharSequence method but binary compatibility is maintained.

@bondolo bondolo merged commit 053873a into apple:main Apr 19, 2021
@bondolo bondolo deleted the charsequences-string branch April 20, 2021 00:01
idelpivnitskiy pushed a commit that referenced this pull request Apr 20, 2021
Motivation:
In ServiceTalk 0.39.0 the method
`CharSequences.newAsciiString(CharSequence)` was introduced which
replaced `CharSequences.newAsciiString(String)`. This was a source
compatible, but binary incompatible change. All existing uses of the
`String` overload could be recompiled to use the `CharSequence`
overload. Existing binary code expected the `String` overload to exist
and fails with `NoSuchMethodError` at runtime.
Modifications:
For cases where the libraries and applications using ServiceTalk cannot
be recompiled conveniently the `CharSequences.newAsciiString(String)`
overload is restored.
Result:
Binary compatibility with libraries and applications which use
`CharSequences.newAsciiString(String)` and are compiled against
versions of ServiceTalk before 0.39.0 is restored.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants