Skip to content

Make SourceLocation and related types Hashable #254

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 15, 2021
Merged

Make SourceLocation and related types Hashable #254

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9aaae85
Make SourceLocation and related types Equatable
MaxDesiatov Jan 15, 2021
ee428a2
Make SourceLocation and related types Hashable
MaxDesiatov Jan 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Sources/SwiftSyntax/SourceLocation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

/// Represent the user-facing part of SourceLocation that can be calculated
/// on demand.
struct ComputedLocation: Codable, CustomDebugStringConvertible {
struct ComputedLocation: Hashable, Codable, CustomDebugStringConvertible {
/// The line in the file where this location resides. 1-based.
let line: Int

Expand Down Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

@ahoppen ahoppen Jan 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s a good point. I didn’t consider file. With that in mind, I’m fine with keeping the implementation as-is.


/// Line and column that can be computed on demand.
private var compLoc: ComputedLocation?
Expand Down Expand Up @@ -91,7 +91,7 @@ public struct SourceLocation: Codable, CustomDebugStringConvertible {
}

/// Represents a start and end location in a Swift file.
public struct SourceRange: Codable, CustomDebugStringConvertible {
public struct SourceRange: Hashable, Codable, CustomDebugStringConvertible {

/// The beginning location in the source range.
public let start: SourceLocation
Expand Down