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

Swap BinaryHeap for Vec to avoid Ord constraint issue #1523

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented Apr 10, 2024

Substitute BinaryHeap for Vec in the same way that we do for BTreeMap/Set to avoid issues with the Ord constraint on the generic type (because this may be a generated type, and we don't automatically apply Ord to generated types).

Now that we have recursive derives, we might be able to be smarter about this sort of thing in the future by autoamtically recursively deriving Ord on the relevant types (which should be OK because it would very likely have existed on the ndoe side anyway to be using such types)

Closes #1515

@jsdw jsdw marked this pull request as ready for review April 10, 2024 15:32
@jsdw jsdw requested a review from a team as a code owner April 10, 2024 15:32
@niklasad1 niklasad1 changed the title Swap BianryHeap for KeyedVec to avoid Ord constraint issue Swap BinaryHeap for KeyedVec to avoid Ord constraint issue Apr 10, 2024
@jsdw jsdw changed the title Swap BinaryHeap for KeyedVec to avoid Ord constraint issue Swap BinaryHeap for Vec to avoid Ord constraint issue Apr 10, 2024
@jsdw jsdw merged commit 51d9ce0 into master Apr 10, 2024
13 checks passed
@jsdw jsdw deleted the jsdw-binaryheap-keyedvec branch April 10, 2024 16:49
niklasad1 pushed a commit that referenced this pull request Apr 11, 2024
* Swap BianryHeap for KeyedVec to avoid Ord constraint issue

* Vec, not KeyedVec
niklasad1 pushed a commit that referenced this pull request Apr 11, 2024
* Swap BianryHeap for KeyedVec to avoid Ord constraint issue

* Vec, not KeyedVec
@jsdw jsdw mentioned this pull request May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

codegen broken by BinaryHeap in metadata
2 participants