[stdlib/Sema] Minor improvements to synthesized hashValue

[xwu]

This PR makes two minor improvements to the synthesized implementation of hashValue.

• As mentioned in [stdlib/Sema/AST] Adopt a commonly used implementation for combining hashes #13056 and explained in [WIP] Conditional conformance to Hashable for Optional, Dictionary and Array types. #14247, the extraneous final _mixInt call is removed.
• In _combineHashValues, a 64-bit magic number is used instead of a 32-bit magic number on 64-bit architectures.

The manual implementation of DoubleWidth.hashValue is adjusted to match its synthesized counterparts.

[xwu]

cc @allevato

[xwu]

@swift-ci Please smoke test

[allevato]

LGTM!

[natecook1000]

Would it make sense to have compiler directives like #if archbits(32) ... #elseif archbits(64) ... #endif? Seems problematic/error prone to list individual architectures.

[xwu]

We can already write this as if MemoryLayout<Int>.size == 4, etc., and the compiler takes care of optimizing it. But here I'm following the prevailing style in this particular file.

Edit: Alternatively, a simplification which could be done down the road is to align ourselves with the assumption (already made elsewhere in the project) that only i386 and arm are 32-bit.

[natecook1000]

Is it smart enough not to complain that the 64-bit value is out of bounds on 32-bit platforms if you write that?

I'm not critiquing what you did here — more just wondering how this could be more clear. Totally possible that this is basically only an issue in places like this in the standard library.

[xwu]

That’s a good point; the code may not actually compile on a 32-bit system depending on compiler smarts. I do know it’s optimized down to a constant on my 64-bit system.

I think, ultimately, the utility of another compilation conditional will come down to whether or not we realistically expect Swift to support 32–bit architectures other than i386 and arm. It’s not entirely out of the question but does seem pretty unlikely in the near term.

[lorentey]

A dedicated directive would certainly express the underlying intention better, even if we never add support for additional architectures. We already have _endian(little)/_endian(big) for endianness.

In any case, for now, we could replace the #elseif with #else here; the compiler will complain if the constant doesn't fit in an UInt on some new 32-bit architecture. (It seems unnecessarily fragile/verbose to list all supported 64-bit architectures, although it is arguably the right choice in the rest of the file. We did go with #else in the new String implementation, although that may have just been due to laziness on my part. 🙈)

[xwu]

I agree that it's a pragmatic solution for the time being. I'll create a follow-up PR with that change.

[lorentey]

Nice!

[lorentey]

A dedicated directive would certainly express the underlying intention better, even if we never add support for additional architectures. We already have _endian(little)/_endian(big) for endianness.

In any case, for now, we could replace the #elseif with #else here; the compiler will complain if the constant doesn't fit in an UInt on some new 32-bit architecture. (It seems unnecessarily fragile/verbose to list all supported 64-bit architectures, although it is arguably the right choice in the rest of the file. We did go with #else in the new String implementation, although that may have just been due to laziness on my part. 🙈)