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

Rustc doesn't error when trying to write to a const array #55721

Closed
morbatex opened this issue Nov 6, 2018 · 5 comments · Fixed by #75573
Closed

Rustc doesn't error when trying to write to a const array #55721

morbatex opened this issue Nov 6, 2018 · 5 comments · Fixed by #75573
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@morbatex
Copy link

morbatex commented Nov 6, 2018

When trying to write to a const array rustc does compile the code without throwing an error but does not assign the value to the array.

As an example

const FOO : [i32;1] = [0];

fn main() {
    FOO[0] = 1;
    println!("{:?}",FOO);
}

compiles and prints

[0]

Is this behavior intended or should the compiler stop with an error as it does when compiling the following code?

const BAR : i32 = 0;

fn main() {
   BAR = 1;
    println!("{:?}",BAR);
}
BAR = 1;
^^^^^^^ left-hand of expression not valid
@petrochenkov
Copy link
Contributor

@oli-obk
Copy link
Contributor

oli-obk commented Nov 6, 2018

but does not assign the value to the array.

"of course" not, you can't change constants, as they are (by definition) constant. If we changed that, we'd have a lot of problems (think about what something like 1 = 42; means)

What's going on here is totally not intuitive. Your code essentially expands to

const FOO : [i32;1] = [0];

fn main() {
    let mut tmp = FOO;
    tmp[0] = 1;
    println!("{:?}",FOO);
}

which also doesn't produce any diagnostics.

Maybe we could have a lint which tells you about "unused" variables like this, even if these variables are temporaries.

@estebank estebank added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-diagnostics Area: Messages for errors, warnings, and lints labels Nov 6, 2018
@MSxDOS
Copy link

MSxDOS commented Jul 7, 2019

This is ridiculous that such code even compiles:

struct Thing(usize);

impl Thing {
    fn set(&mut self, val: usize) {
        self.0 = val;
    }
}

const CONST_THING: Thing = Thing(0);

fn main() {
    println!("Before: {}", CONST_THING.0);
    CONST_THING.set(1);
    println!("After: {}", CONST_THING.0);
}

I just spent more than an hour figuring out what was wrong with my code because I accidentally defined one of my values as const while trying to mutate it and Rust didn't even give a warning when it would throw an error if it was a static or a let binding. That just makes no sense whatsoever.

@estebank estebank added D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 10, 2019
@crlf0710 crlf0710 added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jun 11, 2020
@Aaron1011
Copy link
Member

It turns out that there is a legitimate usage of this pattern with thread_local:

When you use the thread_local! macro, a new const gets defined:

($(#[$attr:meta])* $vis:vis $name:ident, $t:ty, $init:expr) => {
$(#[$attr])* $vis const $name: $crate::thread::LocalKey<$t> =
$crate::__thread_local_inner!(@key $t, $init);
}

A LocalKey stores a function pointer, so it's fine for it to get copied at each call site:

pub struct LocalKey<T: 'static> {
// This outer `LocalKey<T>` type is what's going to be stored in statics,
// but actual data inside will sometimes be tagged with #[thread_local].
// It's not valid for a true static to reference a #[thread_local] static,
// so we get around that by exposing an accessor through a layer of function
// indirection (this thunk).
//
// Note that the thunk is itself unsafe because the returned lifetime of the
// slot where data lives, `'static`, is not actually valid. The lifetime
// here is actually slightly shorter than the currently running thread!
//
// Although this is an extra layer of indirection, it should in theory be
// trivially devirtualizable by LLVM because the value of `inner` never
// changes and the constant should be readonly within a crate. This mainly
// only runs into problems when TLS statics are exported across crates.
inner: unsafe fn() -> Option<&'static T>,
}

If this kind of code is very uncommon (which I suspect it is), we could probably get away with whitelisting LocalKey in any lint that we write. However, if there are any crates defining this kind of const, then downstream crates will get a false-positive warning of every single use of the const (even if the fact that it's a const is an implementation detail, like with thread_local!).

@Aaron1011
Copy link
Member

Another challenge is interior mutability: this code is probably fine:

const EMPTY_VEC: Vec<u8> = Vec::new();

fn main() {
    EMPTY_VEC.len();
}

but this code isn't:

use std::cell::Cell;

struct Foo {
    inner: Cell<u8>
}

impl Foo {
    fn sneaky_len(&self) -> u8 {
        self.inner.replace(0)
    }
}

const FOO: Foo = Foo { inner: Cell::new(25) };

fn main() {
    assert_eq!(FOO.sneaky_len(), 25);
    assert_eq!(FOO.sneaky_len(), 25);
    
    let foo = FOO;
    assert_eq!(foo.sneaky_len(), 25);
    assert_eq!(foo.sneaky_len(), 0);
}

bors added a commit to rust-lang-ci/rust that referenced this issue Sep 10, 2020
…, r=oli-obk

Add CONST_ITEM_MUTATION lint

Fixes rust-lang#74053
Fixes rust-lang#55721

This PR adds a new lint `CONST_ITEM_MUTATION`.
Given an item `const FOO: SomeType = ..`, this lint fires on:

* Attempting to write directly to a field (`FOO.field = some_val`) or
  array entry (`FOO.array_field[0] = val`)
* Taking a mutable reference to the `const` item (`&mut FOO`), including
  through an autoderef `FOO.some_mut_self_method()`

The lint message explains that since each use of a constant creates a
new temporary, the original `const` item will not be modified.
@bors bors closed this as completed in f422ef1 Sep 10, 2020
flip1995 pushed a commit to flip1995/rust that referenced this issue Sep 10, 2020
…, r=oli-obk

Add CONST_ITEM_MUTATION lint

Fixes rust-lang#74053
Fixes rust-lang#55721

This PR adds a new lint `CONST_ITEM_MUTATION`.
Given an item `const FOO: SomeType = ..`, this lint fires on:

* Attempting to write directly to a field (`FOO.field = some_val`) or
  array entry (`FOO.array_field[0] = val`)
* Taking a mutable reference to the `const` item (`&mut FOO`), including
  through an autoderef `FOO.some_mut_self_method()`

The lint message explains that since each use of a constant creates a
new temporary, the original `const` item will not be modified.
@fmease fmease added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants