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

[4.2][SE-0206][stdlib] Implement Hashable Enhancements proposal #16173

Merged
merged 19 commits into from
Apr 27, 2018

Conversation

lorentey
Copy link
Member

Implements SE-0206 for 4.2.

rdar://problem/35052153

Cherry-pick from #16066 & #16073, reviewed by @milseman & @lancep

lorentey added 19 commits April 26, 2018 14:47
This makes it easier to understand failure traces in test logs.

(cherry picked from commit d55b14b)
This is safe to do with hash(into:), because random hash collisions can be eliminated with awesome certainty by trying a number of different hash seeds. (Unless there is a weakness in SipHash.)

In some cases, we intentionally want hashing to produce looser equivalency classes than equality — to let those cases keep working, add an optional hashEqualityOracle parameter.

Review usages of checkHashable and add hash oracles as needed.

(cherry picked from commit 8d18d1a)
This prototype is not fully implemented, and it relies on specific hash values to not trigger unhandled cases.

To keep its test working, define and use a custom hashing interface that emulates hashValue behavior prior to SE-0206.

(cherry picked from commit 6984782)
(cherry picked from commit de66338)
As noted in the proposal’s revision, this allows us to get rid of finalization checks, improves API robustness, and paves the way for making Hasher move-only in the future.

(cherry picked from commit 87685aa)
(cherry picked from commit 641d9b1)
Newly internal declarations include Hasher._seed and the integer overloads of Hasher._combine(_:), as well as _SipHash13 and _SipHash24.

Unify the interfaces of these SipHash testing structs with Hasher. Update SipHash test to cover Hasher, too.

Add @usableFromInline to all newly internal stuff. In addition to its normal use, it also enables white box testing; compile tests that need to use these declarations with -disable-access-control.

(cherry picked from commit 97ec296)
This removes the default implementation of hash(into:), and replaces it with automatic synthesis built into the compiler. Hashable can now be implemented by defining either hashValue or hash(into:) -- the compiler supplies the missing half automatically, in all cases.

To determine which hash(into:) implementation to generate, the synthesizer resolves hashValue -- if it finds a synthesized definition for it, then the generated hash(into:) body implements hashing from scratch, feeding components into the hasher. Otherwise, the body implements hash(into:) in terms of hashValue.

(cherry picked from commit 23c1b24)
…or _hash(into:)

Without this change, the Hashable synthesizer attempts to add hash(into:) methods directly on CF types, which (somewhat unsurprisingly) fails with assertion below. (This situation is unique to CF types, which are imported with an implicit _CFObject conformance; we usually have an extension context or a non-imported type decl in which to put the derived methods.)

Assertion failed: (!decl->isForeign() && "Use getForeignMetadataLayout()"), function getClassMetadataLayout, file /Users/klorentey/Swift/swift/lib/IRGen/MetadataLayout.cpp, line 83.
0  swift                    0x000000010ccd29c8 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  swift                    0x000000010ccd30d6 SignalHandler(int) + 694
2  libsystem_platform.dylib 0x00007fff600e0d9a _sigtramp + 26
3  swift                    0x000000010e04f1ed cmark_strbuf__initbuf + 122440
4  libsystem_c.dylib        0x00007fff5ffa6f79 abort + 127
5  libsystem_c.dylib        0x00007fff5ff6f090 basename_r + 0
6  swift                    0x00000001096e8f95 swift::irgen::IRGenModule::getClassMetadataLayout(swift::ClassDecl*) + 53
7  swift                    0x00000001095b9e25 swift::irgen::emitVirtualMethodValue(swift::irgen::IRGenFunction&, llvm::Value*, swift::SILDeclRef, swift::CanTypeWrapper<swift::SILFunctionType>) + 133
8  swift                    0x00000001095ba252 swift::irgen::emitVirtualMethodValue(swift::irgen::IRGenFunction&, llvm::Value*, swift::SILType, swift::SILDeclRef, swift::CanTypeWrapper<swift::SILFunctionType>, bool) + 690
9  swift                    0x00000001096bef49 swift::SILInstructionVisitor<(anonymous namespace)::IRGenSILFunction, void>::visit(swift::SILInstruction*) + 33497
10 swift                    0x00000001096b3067 swift::irgen::IRGenModule::emitSILFunction(swift::SILFunction*) + 8055
11 swift                    0x00000001095c8d8b swift::irgen::IRGenerator::emitLazyDefinitions() + 1051
12 swift                    0x000000010968eb16 performIRGeneration(swift::IRGenOptions&, swift::ModuleDecl*, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >, llvm::StringRef, swift::PrimarySpecificPaths const&, llvm::LLVMContext&, swift::SourceFile*, llvm::GlobalVariable**, unsigned int) + 1382
13 swift                    0x000000010968f05e swift::performIRGeneration(swift::IRGenOptions&, swift::SourceFile&, std::__1::unique_ptr<swift::SILModule, std::__1::default_delete<swift::SILModule> >, llvm::StringRef, swift::PrimarySpecificPaths const&, llvm::LLVMContext&, unsigned int, llvm::GlobalVariable**) + 94
14 swift                    0x000000010952cfc0 performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 15488
15 swift                    0x0000000109528332 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 2962
16 swift                    0x00000001094e4848 main + 2328
17 libdyld.dylib            0x00007fff5feecc41 start + 1
Stack dump:
[…]
1.	While emitting IR SIL function "@$SSo10CGColorRefas8HashableSCsACP4hash4intoys6HasherVz_tFTW".
 for 'hash(into:)' in module 'CoreGraphics'

(cherry picked from commit 7d679f8)
hash(into:) needs to be included in expectations; tests looking at synthesized Hashable implementation bodies need to be updated for resilient hashing.

(cherry picked from commit 130bcfd)
…iated values

For enums with no associated values, it is better to move the hasher.combine call out of the switch statement, like this:

func hash(into hasher: inout Hasher) {
  let discriminator: Int
  switch self {
    case a: discriminator = 0
    case b: discriminator = 1
    case c: discriminator = 2
  }
  hasher.combine(discriminator)
}

This enables the optimizer to replace the switch statement with a simple integer promotion, restoring earlier behavior.

(cherry picked from commit fa26420)

# Conflicts:
#	test/IRGen/enum_derived.swift
@lorentey
Copy link
Member Author

@swift-ci please test

@lorentey lorentey requested a review from airspeedswift April 26, 2018 15:52
@tkremenek tkremenek merged commit 553ed1b into swiftlang:swift-4.2-branch Apr 27, 2018
@lorentey lorentey deleted the make-hasher-public-4.2 branch April 30, 2018 19:18
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