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

Add more constructors and entry-APIs for la-arena #12931

Merged
merged 2 commits into from
Aug 6, 2022

Conversation

oxalica
Copy link
Contributor

@oxalica oxalica commented Aug 3, 2022

la-arena on crates.io is quite helpful when just a thin wrapper for Vec with u32 indices is needed.
But the current API is not ergonomic enough.

This PR

  • Adds ArenaMap::new. Not sure why only Arena has it now.
  • Adds Arena{,Map}::with_capacity for known-size storage.
  • Adds entry-API for ArenaMap for easier .entry(idx).or_default().push(value) or .entry(idx).or_insert(...) operations.

We enforce integral and `Copy` key, so some key-related functions are
not necessary since user can just reuse the index for the `entry` call.
self.slot.as_mut().expect("Occupied")
}

/// Sets the value of the entry with the `OccupiedEntry`’s key, and returns the entry’s old value.
Copy link
Member

Choose a reason for hiding this comment

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

Wow, we already have a couple of smart quotes in the codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's copied from std. Should I convert them to '?

Copy link
Member

Choose a reason for hiding this comment

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

No need to, I guess, since we already have others.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, we already have a couple of smart quotes in the codebase.

Hehe that was probably me, since I wrote the la-arena docs and I always use smart quotes out of habit :)

@lnicola
Copy link
Member

lnicola commented Aug 6, 2022

@bors r+

This also needs a version bump, but it seems we don't have auto-publishing set up for it yet.

@bors
Copy link
Collaborator

bors commented Aug 6, 2022

📌 Commit 10f870e has been approved by lnicola

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 6, 2022

⌛ Testing commit 10f870e with merge 5170569...

@bors
Copy link
Collaborator

bors commented Aug 6, 2022

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing 5170569 to master...

@bors bors merged commit 5170569 into rust-lang:master Aug 6, 2022
bors added a commit that referenced this pull request Aug 7, 2022
More methods and traits for `la_arena::ArenaMap`

Continue of #12931. Seems that I forgot some methods in the previous PR :(

I also changed `ArenaMap::insert` to return the old value, to match the map-like collection API of std. **So this is a breaking change.**

r? `@lnicola`
@oxalica
Copy link
Contributor Author

oxalica commented Aug 12, 2022

@lnicola

This also needs a version bump, but it seems we don't have auto-publishing set up for it yet.

Is there any schedule of publishing? The version on crates.io is still 0.2.1, which is more than one year old e20a1a4

@lnicola
Copy link
Member

lnicola commented Aug 12, 2022

There's no schedule, and we don't have the infra for it. In other crates there's a workflow running xtask ci, which checks the version number and does the publishing.

The main RA crates are published on each release, without checking for changes.

Note that the xtask ci trick only works if it can find the appropriate tags in the repository. We can't tag la-arena like that in this repo. Either way, lsp-server is in the same situation, having been moved recently.

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.

None yet

4 participants