Put 2 and 2 together and make unique_ptr Sync.#646
Open
copybara-service[bot] wants to merge 1 commit intomainfrom
Open
Put 2 and 2 together and make unique_ptr Sync.#646copybara-service[bot] wants to merge 1 commit intomainfrom
Sync.#646copybara-service[bot] wants to merge 1 commit intomainfrom
Conversation
As long as we don't expose access to the `T` for a `&unique_ptr`, it doesn't matter that it's internally mutable! So making it `!Sync` is equivocating and delaying a decision, here. The model used in this CL is to make it `Sync`, and not allow any safe access to the `T` when behind a `&unique_ptr`. C++ can mutably access `T`, but this is unsafe in C++ just as it is in Rust, and there's no _new_ UB from this. When we have safe aliasing references, we should likely be able to project a `&unique_ptr<T>` to a `CRef<T>`, equally safely to C++. To enable this operation, uh, we might need to tighten when `unique_ptr` is `Sync`... I haven't thought about it. Anyway, dealing with unique_ptr won't be nice and tidy. It's much better to use a `Box` if possible. (We need to merge the allocators, first, though!) PiperOrigin-RevId: 874698118
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Put 2 and 2 together and make unique_ptr
Sync.As long as we don't expose access to the
Tfor a&unique_ptr, it doesn't matter that it's internally mutable! So making it!Syncis equivocating and delaying a decision, here.The model used in this CL is to make it
Sync, and not allow any safe access to theTwhen behind a&unique_ptr. C++ can mutably accessT, but this is unsafe in C++ just as it is in Rust, and there's no new UB from this.When we have safe aliasing references, we should likely be able to project a
&unique_ptr<T>to aCRef<T>, equally safely to C++. To enable this operation, uh, we might need to tighten whenunique_ptrisSync... I haven't thought about it.Anyway, dealing with unique_ptr won't be nice and tidy. It's much better to use a
Boxif possible. (We need to merge the allocators, first, though!)