[stdlib] Set, Dictionary: Clarify assert message when a duplicate key is found #15569
+4
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… is found When a duplicate key is found during rehashing, it is usually either because a key was mutated after insertion, or because the dictionary itself was mutated from multiple threads at the same time. Both of these are serious programmer errors. Promote the sanity check for duplicate keys to a full precondition and improve the error message to point out the most probable cause of the failure.
|
@swift-ci test |
|
@swift-ci smoke benchmark |
|
This is just a quick patch to help people debug these issues. We aren't doing a great job at detecting them, though; we should try to catch them more often. (rdar://problem/36437535) |
Build comment file:Optimized (O)Regression (21)
Improvement (15)
No Changes (389)
Unoptimized (Onone)Regression (16)
Improvement (17)
No Changes (392)
Hardware Overview |
milseman
reviewed
Mar 28, 2018
|
Precondition checks can be done at any time; there is no requirement to do them in any particular order wrt other operations. ( (The Set and Dictionary call |
|
Blërg, smoke benchmarks are noisy. I don't see an obviously relevant regression. |
|
So, it's no longer "unsafe"? Preconditions and debug preconditions are part of the standard library's interface. Sanity checks are for internal assertions. We need some kind of delineation between checks as part of the interface and internal invariant checks. Lacking features, my understanding is that we've done so through names such as "unsafe" or "unchecked", which we call after performing interface checks. |
|
My changing of Mutating a hash key is a subtle but serious programming error. It is special in that
This particular function, deep inside the guts of our hash tables, is one of the few places where hashed collections are sometimes able to detect mutated keys. Leaving the check as a |
|
LGTM! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a duplicate key is found during rehashing, it is usually either because a key was mutated after insertion, or because the dictionary itself was mutated from multiple threads at the same time.
Both of these are serious programmer errors. Promote the sanity check for duplicate keys to a full precondition and improve the error message to point out the most probable cause of the failure.