Skip to content

[5.7] Fix algorithms overload resolution issues #425

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

Merged

Conversation

natecook1000
Copy link
Member

This change addresses two overload resolution problems with the
collection-based algorithm methods. First, when RegexBuilder is
imported, String gains RegexComponent conformance, which means
the RegexComponent-based overloads win for strings, which is
undesirable. Second, if a collection has an element type that can
be expressed as an array literal, collection-based methods get
selected ahead of any standard library counterpart. These two problems
combine in a tricky way for split and contains.

For split, both the collection-based and regex-based versions need
to be marked as @_disfavoredOverload so that the problems above can
be resolved. Unfortunately, this sets up an ambiguity once String
has RegexComponent conformance, so the RegexBuilder module
includes separate overloads for String and Substring that act
as tie-breakers. If introduced in the standard library, these would
be a source-breaking change, as they would win over the Element-
based split when referencing the split method, as with
let splitFunction = myString.split.

For contains, the same requirements hold, with the added
complication that the Foundation overlay defines its own
String.contains(_:) method with different behavior than included
in these additions. For this reason, the more specific overloads for
String and Substring can't live in the RegexBuilder module,
which creates a problem for source compatibility. As it stands now,
this existing code does not compile with the new algorithm methods
added, as the type of vowelPredicate changes from (Character) -> Bool to (String) -> Bool:

let str = "abcde"
let vowelPredicate = "aeiou".contains
print(str.filter(vowelPredicate))

This change addresses two overload resolution problems with the
collection-based algorithm methods. First, when RegexBuilder is
imported, `String` gains `RegexComponent` conformance, which means
the `RegexComponent`-based overloads win for strings, which is
undesirable. Second, if a collection has an element type that can
be expressed as an array literal, collection-based methods get
selected ahead of any standard library counterpart. These two problems
combine in a tricky way for `split` and `contains`.

For `split`, both the collection-based and regex-based versions need
to be marked as `@_disfavoredOverload` so that the problems above can
be resolved. Unfortunately, this sets up an ambiguity once `String`
has `RegexComponent` conformance, so the `RegexBuilder` module
includes separate overloads for `String` and `Substring` that act
as tie-breakers. If introduced in the standard library, these would
be a source-breaking change, as they would win over the `Element`-
based split when referencing the `split` method, as with
`let splitFunction = myString.split`.

For `contains`, the same requirements hold, with the added
complication that the Foundation overlay defines its own
`String.contains(_:)` method with different behavior than included
in these additions. For this reason, the more specific overloads for
`String` and `Substring` can't live in the `RegexBuilder` module,
which creates a problem for source compatibility. As it stands now,
this existing code does not compile with the new algorithm methods
added, as the type of `vowelPredicate` changes from `(Character) ->
Bool` to `(String) -> Bool`:

```
let str = "abcde"
let vowelPredicate = "aeiou".contains
print(str.filter(vowelPredicate))
```
@natecook1000
Copy link
Member Author

@swift-ci Please test

@natecook1000 natecook1000 changed the title [5.7] Fix algorithms overload resolution issues (#402) [5.7] Fix algorithms overload resolution issues May 21, 2022
@natecook1000 natecook1000 added the r5.7 5.7 Release Cherry Picks label May 23, 2022
@natecook1000 natecook1000 merged commit 3be51c1 into swiftlang:swift/release/5.7 May 24, 2022
@natecook1000 natecook1000 deleted the 5.7/contains_overloads branch May 24, 2022 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
r5.7 5.7 Release Cherry Picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants