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

Explicitly state the contracts of Stack and GuardedStack #41

Closed
wants to merge 2 commits into from
Closed

Explicitly state the contracts of Stack and GuardedStack #41

wants to merge 2 commits into from

Conversation

whitequark
Copy link
Contributor

Fixes #39. Fixes #40. cc @Amanieu

@Amanieu
Copy link
Collaborator

Amanieu commented Sep 2, 2016

As I mentioned in #39, Stack should be changed to an unsafe trait since an implementation that fails to uphold the contract could lead to memory unsafety.

The alternative is to have only GuardedStack uphold a contract. Callers of Generator::unsafe_new will then have to ensure themselves that the Stack they pass in correctly upholds the stack requirements.

@whitequark
Copy link
Contributor Author

As I mentioned in #39, Stack should be changed to an unsafe trait since an implementation that fails to uphold the contract could lead to memory unsafety.

I don't see how this is true. You cannot use a Stack implementation without unsafe code.

@Amanieu
Copy link
Collaborator

Amanieu commented Sep 2, 2016

I don't see how this is true. You cannot use a Stack implementation without unsafe code.

Fair enough, my point is that you should add a note in the documentation of unsafe_new saying that the caller is responsible for ensuring that the Stack he is providing properly upholds its contract.

@whitequark
Copy link
Contributor Author

@Amanieu addressed

@edef1c
Copy link
Owner

edef1c commented Sep 2, 2016

Landed ❤️

@edef1c edef1c closed this Sep 2, 2016
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

3 participants