Add E0492 and E0498 errors explanation #29976

GuillaumeGomez · 2015-11-21T21:24:36Z

nagisa · 2015-11-21T21:31:09Z

src/librustc/diagnostics.rs

+#![plugin="foo")] // error: malformed plugin attribute
+```
+
+The plugin name must be written without quotes and within parenthesis. Example:


plural form of “parenthesis” is “parentheses”.

Also probably “between” instead of “within”, but either works for me.

Well, is there any preference for it ?

How does #![plugin="foo")] even parse?
EDIT: it doesn't: "error: incorrect close delimiter: )".

Well saw, my bad. I forgot the ')'.

GuillaumeGomez · 2015-11-21T22:12:23Z

Updated.

Manishearth · 2015-11-22T02:01:02Z

src/librustc/diagnostics.rs

@@ -1941,6 +1941,53 @@ you want. Example:
 ```
 "##,

+E0492: r##"
+A constant borrow which contains interior mutability was attempted. Erroneous


A borrow of a constant containing interior mutability

bors · 2015-11-22T10:31:36Z

☔ The latest upstream changes (presumably #29716) made this pull request unmergeable. Please resolve the merge conflicts.

GuillaumeGomez · 2015-11-22T17:34:06Z

Updated.

Manishearth · 2015-11-23T03:20:32Z

src/librustc/diagnostics.rs

+```
+
+To solve this error, either use statics or avoid using `cell` type. If you want
+to use statics with a `cell` type, you will have to wrap it:


I think we should encourage people to use a mutex here instead.

There should always be a safe suggestion.

Manishearth · 2015-11-23T12:48:00Z

src/librustc/diagnostics.rs

+```
+
+To solve this error, either use statics or avoid using `cell` type. If you want
+to use statics with a `cell` type, you will have to wrap it, but this is


you will have to wrap it with something which manually implements Sync, but this is unsafe and you will have to ensure that accesses to the cell are synchronized.

huonw · 2015-11-23T13:25:35Z

src/librustc/diagnostics.rs

+fn main() {}
+```
+
+To solve this error, either use statics or avoid using `cell` type. If you want


It'd be great to have an example of these solutions, e.g. using one of the atomic types.

It'd also be nice to have an explanation of why this is an error: a const represents a constant value that should never change. If one takes a & reference to the constant, then one is taking a pointer to some memory location containing the value, normally this is perfectly fine: most values can't be changed via a shared & pointer, but interior mutability would allow it. That is, a constant value could be mutated. On the other hand, a static is explicitly a single memory location, which can be mutated at will.

I added an example, but an unsafe one so @Manishearth asked me to remove it. But I can totally add one, however it wouldn't really correspond to the previous example.

I agree with having the extra explanation about const.

The issue with the statics solution is that there's no safe way to do it (statics can't have destructors), and getting the unsafe way correct depends on the exact situation.

The issue with the statics solution is that there's no safe way to do it (statics can't have destructors), and getting the unsafe way correct depends on the exact situation.

I don't understand your point, because destructors are entirely orthogonal: types with or without destructors can have (or not have) interior mutability.

Here's a before/after example that I was thinking of:

use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT}; // before: // const A: AtomicUsize = ATOMIC_USIZE_INIT; // static B: &'static AtomicUsize = &A; // after: static A: AtomicUsize = ATOMIC_USIZE_INIT; static B: &'static AtomicUsize = &A; fn main() {}

But I can totally add one, however it wouldn't really correspond to the previous example.

Adjust the previous example to include enough that examples of possible solutions follow from it.

That only works for atomics, not for a cell holding a type of your choice. RefCell and Cell won't work because of UnsafeCell, and Mutex doesn't work because of destructors. So there's no safe way to place something like that in a static.

So... I think we should have an example of a thing that does work, if we're going to suggest that thing as a solution. There can be text and/or more examples discussing why this solution doesn't work all the time, but that shouldn't be a reason to not go into more detail about it.

IMO, the goals for these sort of explanations are:

making it clear exactly what the problem is

explaining why something is an error (when it makes sense, like here)

helping people with a solution, as much as possible (code examples are important)

RefCell and Cell won't work because of UnsafeCell

NB. these don't work because of Sync, not UnsafeCell: all interior mutability (including Mutex and the atomics) go through UnsafeCell.

Well, yeah, UnsafeCell isn't Sync. This error diagnostic is about unsafecell.

Some types that contain UnsafeCell do work fine, so UnsafeCell can't be the underlying problem. You are correct that those two types do get their non-Sync-ness by just letting UnsafeCell's opt-out imply it, but the fundamental reason is the semantics of Cell and RefCell are not compatible with Sync i.e. they can never be Sync, no matter what their implementation details are.

(It's somewhat a bug that the error message focuses on UnsafeCell rather than Cell.)

bors · 2015-11-26T20:10:36Z

☔ The latest upstream changes (presumably #30043) made this pull request unmergeable. Please resolve the merge conflicts.

GuillaumeGomez · 2015-11-28T16:33:51Z

Updated. Not sure it is what you had in mind @huonw.

Manishearth · 2015-11-29T02:43:24Z

src/librustc/diagnostics.rs

+it. That is, a constant value could be mutated. On the other hand, a `static` is
+explicitly a single memory location, which can be mutated at will.
+
+So, in order to solve this error, either use statics with a `Sync` type:


Use statics which are Sync.

GuillaumeGomez · 2015-11-29T18:00:15Z

Updated.

Manishearth · 2015-11-29T18:09:47Z

src/librustc/diagnostics.rs

+static B: &'static AtomicUsize = &A; // ok!
+```
+
+You can also have this error while using a `cell type`:


no code formatting around "cell type"

Manishearth · 2015-11-29T18:10:02Z

src/librustc/diagnostics.rs

+const F: &'static C = &D; // error
+```
+
+This is because `cell types` internally use `UnsafeCell`, which isn't `Sync`.


same regarding code formatting

GuillaumeGomez · 2015-11-29T18:51:50Z

Re-updated.

Manishearth · 2015-11-29T18:55:24Z

src/librustc/diagnostics.rs

+static B: &'static NotThreadSafe<usize> = &A; // ok!
+```
+
+Remember this solution is unsafe! You will have to ensute that accesses to the


Manishearth · 2015-11-29T18:56:07Z

r=me after one spelling fix

Manishearth · 2015-11-29T18:56:12Z

@bors delegate+

Manishearth · 2015-11-29T18:56:21Z

Oh, that won't work on this bors.

GuillaumeGomez · 2015-11-29T18:59:43Z

No it doesn't. :p (fixed)

Manishearth · 2015-11-29T19:00:36Z

@bors r+ rollup

bors · 2015-11-29T19:00:40Z

📌 Commit 483656b has been approved by Manishearth

bors · 2015-11-29T19:58:44Z

⌛ Testing commit 483656b with merge dc9fb96...

bors · 2015-11-29T21:10:36Z

💔 Test failed - auto-mac-32-opt

steveklabnik · 2015-11-29T21:58:26Z

@bors: retry

bors · 2015-11-29T22:54:44Z

⌛ Testing commit 483656b with merge b22d7a5...

@Manishearth

r? @Manishearth

huonw · 2015-11-29T23:56:15Z

src/librustc/diagnostics.rs

+const F: &'static C = &D; // error
+```
+
+This is because cell types internally use `UnsafeCell`, which isn't `Sync`.


Hm, as I said before, it isn't because the types use UnsafeCell internally: there are Sync types that use UnsafeCell, like the atomics and Mutex<T>. It would be better to just say:

This is because these types do operations that are not thread-safe, and hence do not implement Sync.

bors · 2015-11-30T00:41:11Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

rust-highfive assigned Manishearth Nov 21, 2015

nagisa reviewed Nov 21, 2015
View reviewed changes

GuillaumeGomez changed the title ~~Add E0498 error explanation~~ Add E0492 and E0498 errors explanation Nov 21, 2015

Manishearth reviewed Nov 22, 2015
View reviewed changes

GuillaumeGomez force-pushed the patch-5 branch 3 times, most recently from 61a9188 to 2c9a389 Compare November 22, 2015 14:41

Manishearth reviewed Nov 23, 2015
View reviewed changes

GuillaumeGomez force-pushed the patch-5 branch from 2c9a389 to 718c1ee Compare November 23, 2015 12:45

Manishearth reviewed Nov 23, 2015
View reviewed changes

GuillaumeGomez force-pushed the patch-5 branch from 718c1ee to 479f6bc Compare November 23, 2015 13:10

huonw reviewed Nov 23, 2015
View reviewed changes

Add E0498 error explanation

1904b33

GuillaumeGomez force-pushed the patch-5 branch 2 times, most recently from 35c594d to bbf56d0 Compare November 28, 2015 15:41

Add E0492 error explanation

483656b

Manishearth reviewed Nov 29, 2015
View reviewed changes

GuillaumeGomez force-pushed the patch-5 branch from bbf56d0 to cd9811e Compare November 29, 2015 17:59

Manishearth reviewed Nov 29, 2015
View reviewed changes

GuillaumeGomez force-pushed the patch-5 branch from cd9811e to 79f1259 Compare November 29, 2015 18:51

Manishearth reviewed Nov 29, 2015
View reviewed changes

GuillaumeGomez force-pushed the patch-5 branch from 79f1259 to 483656b Compare November 29, 2015 18:59

bors added a commit that referenced this pull request Nov 29, 2015

Auto merge of #29976 - GuillaumeGomez:patch-5, r=Manishearth

b22d7a5

r? @Manishearth

huonw reviewed Nov 29, 2015
View reviewed changes

bors merged commit 483656b into rust-lang:master Nov 30, 2015

GuillaumeGomez deleted the patch-5 branch November 30, 2015 09:27

Add E0492 and E0498 errors explanation #29976

Add E0492 and E0498 errors explanation #29976

Conversation

GuillaumeGomez commented Nov 21, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Nov 21, 2015

Choose a reason for hiding this comment

bors commented Nov 22, 2015

GuillaumeGomez commented Nov 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Nov 26, 2015

GuillaumeGomez commented Nov 28, 2015

Choose a reason for hiding this comment

GuillaumeGomez commented Nov 29, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Nov 29, 2015

Choose a reason for hiding this comment

Manishearth commented Nov 29, 2015

Manishearth commented Nov 29, 2015

Manishearth commented Nov 29, 2015

GuillaumeGomez commented Nov 29, 2015

Manishearth commented Nov 29, 2015

bors commented Nov 29, 2015

bors commented Nov 29, 2015

bors commented Nov 29, 2015

steveklabnik commented Nov 29, 2015

bors commented Nov 29, 2015

Choose a reason for hiding this comment

bors commented Nov 30, 2015