Make SourceLocation and related types Hashable #254
Conversation
It seems natural to be able to compare ranges for equality. Currently this requires declaring manual `Equatable` conformance in libraries that use SwiftSyntax, which is error-prone.
This comment has been minimized.
This comment has been minimized.
MaxDesiatov
changed the title
This comment has been minimized.
This comment has been minimized.
|
Build failed |
|
@swift-ci please test |
|
Please feel free to merge when you see fit, I don't have commit access here. |
| @@ -43,7 +43,7 @@ struct ComputedLocation: Codable, CustomDebugStringConvertible { | |||
| } | |||
|
|
|||
| /// Represents a source location in a Swift file. | |||
| public struct SourceLocation: Codable, CustomDebugStringConvertible { | |||
| public struct SourceLocation: Hashable, Codable, CustomDebugStringConvertible { | |||
There was a problem hiding this comment.
Just to be clear: Should two SourceLocations be different if one has a ComputedLocation and the other does not? After all, they still refer to the same location in the source code.
I’m not 100% sure on this, but I’m leaning towards making the identity of SourceLocation be purely based on the value of offset. What do you think, @MaxDesiatov?
CC: @akyrtzi
There was a problem hiding this comment.
In my opinion they shouldn't be equal since line, column, and file will return different results for them. The one without a ComputedLocation could refer to a different file altogether, as file is inferred from compLoc.
There was a problem hiding this comment.
That’s a good point. I didn’t consider file. With that in mind, I’m fine with keeping the implementation as-is.
It seems natural to be able to compare source locations for equality, or to add locations to a
Setor use them as keys in dictionaries. Currently this requires declaring manualHashableconformance in libraries that use SwiftSyntax, which is error-prone.