Modifying a *const* RefCell - why does this even work, but result in "unexpected" behavior?

help
1

Considering this code:

const TEST: RefCell<Option<i32>> = RefCell::new(None);

fn main() {
    let _ = TEST.borrow_mut().insert(42);
    match TEST.borrow().as_ref() {
        Some(x) => println!("Value is: {:?}", x),
        None => unreachable!(),
    }
}

Why is it even possible/allowed to create a const RefCell?

Attempting to modify the "inner" value of the const RefCell neither gives a compiler error, nor does it give an error (panic) at runtime. Still, it obviously does not work! It just fails silently :fearful:

I mean, I understand that modifying a const value is not possible. But, on the other hand, how does a const RefCell ever make sense then? And why is this not prevented at compile-time?

I seems weird that insert() can be called without error, but simply does not have the intended effect.

Isn't this sort of "unexpected" behavior supposed to be impossible in Rust?

Thank you!

2 Likes
2

Take a look at the documentation for const:

const items looks remarkably similar to static items, which introduces some confusion as to which one should be used at which times. To put it simply, constants are inlined wherever they’re used, making using them identical to simply replacing the name of the const with its value.

(bold emphasis mine)

Your code is thus equivalent to this (Rust Playground):

use std::cell::RefCell;

fn main() {
    let _ = RefCell::<Option<i32>>::new(None).borrow_mut().insert(42);
    match RefCell::<Option<i32>>::new(None).borrow().as_ref() {
        Some(x) => println!("Value is: {:?}", x),
        None => unreachable!(),
    }
}

and it becomes obvious that it panics at runtime, as expected.

10 Likes
3

and it becomes obvious that it panics at runtime, as expected.

But it does not panic.

Well, it only panics because I have put an additional unreachable!() in the branch that should never be reached (but unexpectedly is). It does not panic by itself, because we tried to modify a const value.

To put it simply, constants are inlined wherever they’re used, making using them identical to simply replacing the name of the const with its value.

Ah, I see! This explains the observed behavior.

Still, how can a const RefCell ever make sense then? And why was this not prevented?

I mean, as long as a const value is "truly" constant, it does not make any difference whether we have just a single global instance, of whether it actually gets inlined at every place where it is used.

But, as soon as a const value can be modified (like RefCell), the "inlining" creates lots of confusion!

After all, the observed runtime behavior is very unexpected and misleading...

4

Compare with the analogous code for a primitive. At least that one warns, which is nice. Not sure how possible it is to extend the warning to the more indirect version.

Neither are modifying a const value. They're modifying temporaries, like the warning says.

3 Likes
5

As can be seen in the expansion of the code, everytime you reference the const value a "new" RefCell is created. Because the second RefCell has nothing to do with the first (apart from them having the same const initializer), there is no way mutation of one could influence the other. Global mutation is not possible (nor should it be) with const, for that static mut exists which is unsafe for several reasons.

6

Okay, that makes sense. But trying to modify the const RefCell did not give a warning at all :thinking:

7

I agree that a warning or another hint would be a good option, as this could seem confusing to a newcomer. I think the relevant issue is #40543, although there doesn't seem to be a lot of recent development.

8

There's a Clippy lint though.

warning: a `const` item with interior mutability should not be borrowed
 --> src/main.rs:6:13
  |
6 |     let _ = TEST.borrow_mut().insert(42);
  |             ^^^^
  |
  = help: assign this const to a local or static variable, and use the variable here
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
  = note: `#[warn(clippy::borrow_interior_mutable_const)]` on by default

warning: a `const` item with interior mutability should not be borrowed
 --> src/main.rs:7:11
  |
7 |     match TEST.borrow().as_ref() {
  |           ^^^^
  |
  = help: assign this const to a local or static variable, and use the variable here
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
6 Likes
9

Don't use static mut for mutable globals, use a regular static with a proper synchronization primitive like an Rwlock or a Mutex. Much safer and less error-prone.

3 Likes
10

Yeah, I think I understand "why" it happens now.

But what the code effectively expands to is clearly very different form the intent expressed in the original code. So wouldn't it be better/reasonable to simply disallow this and make it a compile-time error?

I mean, the whole point of having a Cell or RefCell is that we can modify its "inner" content. But, if every access to the const RefCell creates a totally new RefCell in-place, then the whole purpose of using a RefCell is contradicted – as any modification we might do will be inevitably be "lost" :slight_smile:

11

Yeah, I agree the warning should be extended if possible.

Here's where that lint came from.

And I think the idea behind this one is what you wish already existed.

1 Like
12

This is not possible, because it would break backwards compatibility (i.e. code that was written for a previous version of Rust that relied on this would fail to compile).

A warning might be good, but as you can see in the discussion of the linked issue #40543 one problem is that there is no way to statically determine whether a given type uses interior mutability - another crate could export a RefCell-ish or Mutex-like type and the lint would not work. The clippy lint @steffahn mentioned might also be sufficient (for the standard library and selected crates) for now.

1 Like
13

It's not public, but there's a trait for no-interior-mutability (lamentably called Freeze at the moment). More generally, the compiler has to understand where it exists one way or another. That's how the clippy lints are implemented, by asking the compiler if there's any interior mutability types involved.

3 Likes
14

There's multiple types of analysis w. r. t. interior mutability. Next to the type-based analysis there's also a value based one. E. g. the first of these two consts compiles fine

use core::cell::Cell;
const C: &Option<Cell<()>> = &None;
const D: &Option<Cell<()>> = &Some(Cell::new(()));

error[E0492]: constants cannot refer to interior mutable data
 --> src/lib.rs:3:30
  |
3 | const D: &Option<Cell<()>> = &Some(Cell::new(()));
  |                              ^^^^^^^^^^^^^^^^^^^^ this borrow of an interior mutable value may end up in the final value

I haven't tested which one the Clippy lint uses. I'm also not sure off the top of my head in what situstions the Freeze trait is used. (Feel free to point out an example of an error that's clearly based on such a type based analysis, if you know one.) Well, some relevance at least: Of course, the type based analysis is the one necessary for code generation, to inform LLVM about the possibility of aliased mutation.

1 Like
15

Because RefCell::new is a const fn.

So the question becomes "why is it a const fn?". There are two reasons. First, const functions can be used not only to directly initialize a const item, but also in const expressions and other const functions. It is desirable to have the const functions be as close as possible to ordinary Rust code in their capabilities, so that code and developer skills can be reused. But there is nothing wrong in creating a RefCell inside of a const block/const fn and using is just for temporary storage during evaluation! The case of assigning RefCell::new to a global const item becomes just a consequence of general rules. A bit of a degenerate case and likely undesirable, but that is a lint-level consideration, not an implementation issue warranting a compiler error.

The second reason is that it is very useful to create safe "immutable" static items with interior mutability:

static COUNTER: RefCell<u64> = RefCell::new(0);

fn count() {
    *COUNTER.borrow_mut() += 1;
}

This pattern is even more useful in multithreaded code, with a global state guarded by a Mutex. But the rules for expressions which can initialize statics are mostly the same as for initializers of consts, because a static must be similarly fully computable at compile time.

16

Caveat: Freeze does not indicate that a type exposes no shared mutability. It only indicates that a type does not have any UnsafeCell before indirection. Box<Mutex<_>> is Freeze.

6 Likes
17

thread_local! with const initializer tends to generate much more efficient code since it doesn't have to do lazy initialization. With proper tls-model it can even be as efficient as unsynchronized mutable statics.

2 Likes
18

What is the use case for having a const variable whose type is not Copy? Seems inherently fraught.

19

Having Mutex::new and Vec::new and HashMap::new being const is extremely helpful for initializing static variables.
And I have to stress that, It's not const variable, it's const item. You are not declaring a variable with const statement.

20

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.