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

Track which reference belongs to which store #292

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexkeizer
Copy link
Contributor

Add a unique id to stores, and keep track of this id in references.
When a reference is used with a different store, this will panic with a message explaining that the user might be trying to reuse object across executions.

This implements the suggestion in #290, although it adds a new id to stores, rather than reusing the execution ids.

One point I would like some more feedback on, is the naming of Ref::from_usize.
I changed the signature to also take a store id, but left the name as is.
Changing it to Ref::new feels natural, but figured I'd ask for the rationale behind naming it from_usize in the first place.

@alexkeizer alexkeizer changed the title Track which reference belongs to with store Track which reference belongs to which store Nov 18, 2022
@alexkeizer
Copy link
Contributor Author

I fixed the formatting, and the compile error for the checkpoint feature, but the minrust job complains that it can't build once_cell, and I'm not really sure how that's caused by my changes (I didn't touch dependencies).
The error is (taken from https://github.com/tokio-rs/loom/actions/runs/3498939232/jobs/5946011488)

Checking once_cell v1.16.0
error: edition 2021 is unstable and only available with -Z unstable-options.

error: could not compile `once_cell`

@alexkeizer
Copy link
Contributor Author

alexkeizer commented Nov 25, 2022

Some digging later, it seems that once_cell bumped its MSRV to 1.56.0 when going from 1.14 to 1.15, and loom transitively depends on once_cell 1.16, but minrust checks with rust 1.51.0.

Is there a specific procedure for changing looms MSRV, or should I just add a PR that changes the minrust job to check with 1.56? Alternatively, do we want to stick with 1.51.0, and pin the once_cell dependency to 1.14 somehow?

EDIT: Fixed in #296

Add a unique id to stores, and keep track of this id
in references.
When a reference is used with a different store, this will
panic with a message explaining that the user might be
trying to reuse object across executions.
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

1 participant