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.

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

Reconsider implementation of Equatable and Hashable for HPACKHeaders #342

Open
Lukasa opened this issue May 4, 2022 · 2 comments
Open
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
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
@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
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 <dnadoba@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Feature doesn't work as expected.
Projects
None yet
Development

No branches or pull requests

2 participants