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.

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

Jump to bottom

Reconsider implementation of Equatable and Hashable for HPACKHeaders #342

Open
Lukasa opened this issue May 4, 2022 · 2 comments
Open

Reconsider implementation of Equatable and Hashable for HPACKHeaders #342

Lukasa opened this issue May 4, 2022 · 2 comments
Labels
kind/bug Feature doesn't work as expected.

Comments

@Lukasa
Copy link
Contributor

Lukasa commented May 4, 2022

For historical reasons, HPACKHeaders does not pay attention to order or indexability when considering equality and hashability. We should consider whether this is appropriate for this type, or whether we should make the definition more strict.
@Lukasa Lukasa added the kind/bug Feature doesn't work as expected. label May 4, 2022
Lukasa added a commit to Lukasa/swift-nio-http2 that referenced this issue May 4, 2022
@Lukasa
Make HPACKHeaders hashable.
f284102 
Motivation:

HPACKHeaders is equatable, and it rarely makes sense to have a type that
is equatable but not hashable. The definition of equatability is pretty
limited here (see apple#342) but we can define a definition of hashability
consistent with it.

Modifications:

- Make HPACKHeaders hashable.

Result:

People can store HPACKHeaders in Sets
@Lukasa Lukasa mentioned this issue May 4, 2022
Merged
@dnadoba
Copy link
Member

dnadoba commented May 4, 2022

We could also deprecate the conformance and add two views to it, ordered and unordered, and only conform the views to Equatable/Hashable.

@Lukasa
Copy link
Contributor Author

Lukasa commented May 4, 2022

I don't think that's really the right thing to do. We should commit the core type to one of those two positions, and where necessary we can always expose the other.

Lukasa added a commit that referenced this issue May 4, 2022
@Lukasa @dnadoba
Make HPACKHeaders hashable. (#344)
c28d13d 
Motivation:

HPACKHeaders is equatable, and it rarely makes sense to have a type that
is equatable but not hashable. The definition of equatability is pretty
limited here (see #342) but we can define a definition of hashability
consistent with it.

Modifications:

- Make HPACKHeaders hashable.

Result:

People can store HPACKHeaders in Sets

Co-authored-by: David Nadoba <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Feature doesn't work as expected.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Lukasa @dnadoba