Skip to content

[stdlib] fix another accidental infinite-recursion bug #38950

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
merged 1 commit into from
Aug 23, 2021

Conversation

glessard
Copy link
Contributor

@glessard glessard commented Aug 19, 2021

It's been known for years (see SR-6501) that it's easy to forget to implement the critical function replaceSubrange<C>(_: with: C) when conforming a type to RangeReplaceableCollection, because the compiler doesn't ask for it. At runtime, there is then an infinite-recursion crash on just about any RRC-specific function.

This adds an unavailable implementation to transform this issue into a compile-time error, in line with other recent similar fixes, such as #36969.

Resolves SR-6501

@glessard glessard marked this pull request as draft August 19, 2021 09:10
@glessard
Copy link
Contributor Author

@swift-ci please test macOS platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 8051a46d8c6681d939ba51f6e26f726c136cf761

@glessard
Copy link
Contributor Author

@swift-ci please test macOS platform

@glessard
Copy link
Contributor Author

@swift-ci please smoke test

@glessard
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility

2 similar comments
@glessard
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility

@glessard
Copy link
Contributor Author

@swift-ci Please Test Source Compatibility

@glessard glessard marked this pull request as ready for review August 21, 2021 22:44
@glessard glessard requested a review from stephentyrone August 21, 2021 22:46
Copy link
Contributor

@stephentyrone stephentyrone left a comment

Choose a reason for hiding this comment

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

LGTM

@stephentyrone stephentyrone merged commit 6b219a9 into swiftlang:main Aug 23, 2021
@glessard glessard deleted the sr6501 branch August 23, 2021 15:28
@dduan
Copy link
Contributor

dduan commented Jan 5, 2022

fwiw this broke source compatibility. For a good reason, ofc!

@glessard
Copy link
Contributor Author

glessard commented Jan 5, 2022

Yeah, breaking code that relies on a bug is deemed not to be an actual source break.

@stephentyrone
Copy link
Contributor

Expanding a little on what Guillaume said, code that was "broken" by this change merely compiled; it never actually worked because it had an invalid conformance to RangeReplaceableCollection. Thus, there is no source break--conformances must satisfy the semantic contract of the protocol in a Swift program*.

(*) Yes, the program used to compile, but it was not a valid Swift program; we simply were unable to detect the constraint violation at compile time. Now we can.

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.

4 participants