Equatable conformance to the DiagnosticSeverity typepublic enum DiagnosticSeverity: Sendable, Equatable {
case error
case warning
case note
case remark
}
if case before but using == is more convenientMaybe widely used when macros become more mainstream or when even more functionality/complexity is unlocked for them. Regardless, I see conforming DiagnosticSeverity to Equatable as a beneficial addition since I'd rather use == instead of if case to check equality.
Would you ever see a future situation where you would wish this type was also Hashable? Would it make sense to go ahead with Hashable now?
I do think it being Hashable now would be better as I do see future situations where I would use it indirectly and directly depending on what the macro's functionality and complexity should be.
Aren't payload-less enums automatically Hashable?
That looks to be true implicitly, and is present in the generated assembly.
I think enums are only automatically Hashable if they are internal (maybe package), not if they are public.
That’s a great suggestion, @RandomHashTags. I’ll update the PR.
I know that was proposed at one point, but I don't remember that we ever actually made that change. As far as I know the implicit Equatable and Hashable conformances on payload-less enums are still public.
Oh, I understand now: Equatable and Hashable are only inferred when building without library evolution enabled or when the enum is marked as @frozen. `DiagnosticSeverity` should conform to `Equatable` · Issue #2964 · swiftlang/swift-syntax · GitHub was reported from developers of swift-testing, which use a version of swift-syntax that has library evolution enabled because they are in the toolchain, so they don’t receive the automatic Equatable and Hashable conformance. So, this change only really affects them.
Hmm… are there any kind of edge-casey situations where code that did depend on the implicit conformance would now behave unexpected with the explicit conformance? I can't think of anything bad happening… were there any unit tests from before this diff that took advantage of the implicit Hashable?
No, swift-testing just hasn’t been able to use DiagnosticSeverity as an Equatable or Hashable type.
(Which we'd like to do!)
I was thinking more about Equatable or Hashable assumptions outside of swift-testing.
For example… how is this check for value equality performed?
That code lives within the same module that defines DiagnosticsSeverity. It’s just that the implicit Hashable conformance is not exported in the public interface when enabling library evolution.
Right. That makes sense. Expanding on that idea could then be:
Equatable or Hashable is assumed outside of swift-syntax…For example… is the Xcode source code putting these DiagnosticsSeverity values in a Set or Dictionary before drawing UI to display to the user?
I can't imagine any reason why this should break… and I would assume this came up when the decision to implement implicit conformance was first discussed (the "forward compatibility" with explicit conformance).
But this might all be super edge-casey… I don't have a strong opinion that this should block landing.
Yes, the only libraries that benefit from this change are ones that rely on a version of swift-syntax that has been built with library evolution enabled. All usages within swift-syntax or from packages that rely on swift-syntax without library evolution enabled (such as a SwiftPM package dependency), already assume that DiagnosticSeverity is Hashable.
So, this change is a no-op for most clients and aligns the library-evolution version closer with the non-library-evolution version.
SGTM. Thanks!
Just curious, but is this expected? I don't see anything in the library-evolution proposal: swift-evolution/proposals/0260-library-evolution.md at main · swiftlang/swift-evolution · GitHub
Yes, my understanding is that this is expected behavior.