Skip to content

expand on the Send/Sync situation for consts #33

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

Merged
merged 2 commits into from
Dec 27, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions const.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,28 @@ fine: inlining that reference everywhere has the same behavior as computing a
new reference each time. In both cases, there exists exactly one instance of
the mutable memory that everything references.

### 3. `Sync`
### 3. `Send`

Finally, the same constant reference is actually shared across threads. This is
very similar to multiple threads having a shared reference to the same `static`,
which is why `static` must be `Sync`. So it seems like we should reject
non-`Sync` types, conforming with the desugaring described above.
Finally, the same constant value is actually shared across threads. This is
very similar to sending the same value across threads, so it seems like we
should reject non-`Send` types. For shared references, this means the pointee
type ought to be `Sync`. That is already required for `static` items, so this
is consistent with the desugaring described above.

However, this does not currently happen, and there are several crates across the
ecosystem that would break if we just started enforcing this now. See
[this issue](https://github.com/rust-lang/rust/issues/49206) and the
[PR attempting to fix this](https://github.com/rust-lang/rust/pull/54424/).

One could make the argument that the value does not have to be `Send` because it
is not actually sent to other threads; instead, conceptually, each thread
re-does the same computation. But we know they will all come to the same
result. This works, except when we consider address identity: with references
in the `const`, all threads will get the same address, unlike in case of a
per-thread recomputation which would lead to different addresses. As a
consequence, non-`Send` `const` without references are fine, but once references
and thus address identity comes into play, we have a problem.

*Dynamic check.* It is unclear how the Miri engine could dynamically check this.
Copy link
Contributor

Choose a reason for hiding this comment

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

I started implementing this before we had validation because I thought we could scrap some of const qualif for a value analysis. What I was basically doing is what validation is doing, but looked for explicit unsafe impl Send or unsafe impl Sync on the type of the value currently being validated. Aggregates that got their trait impls due to the auto trait nature and not explicit impls were simply walked. This permitted Option<*const u8>::None but forbade Option<*const u8>::Some(&1)

Copy link
Member Author

Choose a reason for hiding this comment

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

I doubt we can actually get away with forbidding Option<*const u8>::Some(&1) though...

What I was basically doing is what validation is doing, but looked for explicit unsafe impl Send or unsafe impl Sync on the type of the value currently being validated.

Yeah, I had thoughts along those lines, too. Unfortunately though, negative impls aren't stable yet, so it seems plausible people might do things like add a PhantomData<*const ()> field as a "no-Sync" marker... :/


### 4. Drop
Expand Down