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

Add a static analysis that prevents references to inner mutability only in the trailing expression of a constant #80373

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
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
6 changes: 4 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0492.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ A borrow of a constant containing interior mutability was attempted.
Erroneous code example:

```compile_fail,E0492
#![feature(const_refs_to_cell)]
use std::sync::atomic::AtomicUsize;

const A: AtomicUsize = AtomicUsize::new(0);
static B: &'static AtomicUsize = &A;
const B: &'static AtomicUsize = &A;
// error: cannot borrow a constant which may contain interior mutability,
// create a static instead
```
Expand All @@ -18,7 +19,7 @@ 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.

So, in order to solve this error, either use statics which are `Sync`:
So, in order to solve this error, use statics which are `Sync`:

```
use std::sync::atomic::AtomicUsize;
Expand All @@ -30,6 +31,7 @@ static B: &'static AtomicUsize = &A; // ok!
You can also have this error while using a cell type:

```compile_fail,E0492
#![feature(const_refs_to_cell)]
use std::cell::Cell;

const A: Cell<usize> = Cell::new(1);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,9 @@ declare_features! (
/// Allows arbitrary expressions in key-value attributes at parse time.
(active, extended_key_value_attributes, "1.50.0", Some(78835), None),

/// Allows references to types with interior mutability within constants
(active, const_refs_to_cell, "1.51.0", Some(80384), None),

// -------------------------------------------------------------------------
// feature-group-end: actual feature gates
// -------------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,7 @@ impl<'hir> Map<'hir> {
| Node::ForeignItem(ForeignItem { kind: ForeignItemKind::Fn(..), .. })
| Node::TraitItem(TraitItem { kind: TraitItemKind::Fn(..), .. })
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(..), .. })
| Node::AnonConst(..)
| Node::Block(_) = node
{
return Some(hir_id);
Expand Down
58 changes: 57 additions & 1 deletion compiler/rustc_mir/src/dataflow/framework/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub trait DebugWithContext<C>: Eq + fmt::Debug {
}

write!(f, "\u{001f}-")?;
self.fmt_with(ctxt, f)
old.fmt_with(ctxt, f)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a drive-by bugfix

}
}

Expand Down Expand Up @@ -133,6 +133,28 @@ where
}
}

impl<T, C, V> DebugWithContext<C> for rustc_index::vec::IndexVec<T, V>
where
T: Idx + DebugWithContext<C>,
V: DebugWithContext<C> + Eq,
{
fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_set()
.entries(self.iter_enumerated().map(|this| DebugWithAdapter { this, ctxt }))
.finish()
}

fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result {
assert_eq!(self.len(), old.len());

for (new, old) in self.iter_enumerated().zip(old.iter_enumerated()) {
write!(f, "{:?}", DebugDiffWithAdapter { new, old, ctxt })?;
}

Ok(())
}
}

impl<T, C> DebugWithContext<C> for &'_ T
where
T: DebugWithContext<C>,
Expand All @@ -146,6 +168,40 @@ where
}
}

impl<T, C> DebugWithContext<C> for Option<T>
where
T: DebugWithContext<C>,
{
fn fmt_with(&self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
None => Ok(()),
Some(v) => v.fmt_with(ctxt, f),
}
}

fn fmt_diff_with(&self, old: &Self, ctxt: &C, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match (self, old) {
(None, None) => Ok(()),
(Some(a), Some(b)) => a.fmt_diff_with(b, ctxt, f),
(Some(a), None) => {
write!(f, "\u{001f}+")?;
a.fmt_with(ctxt, f)
}
(None, Some(b)) => {
write!(f, "\u{001f}-")?;
b.fmt_with(ctxt, f)
}
}
}
}

impl<T, U, C> DebugWithContext<C> for (T, U)
where
T: DebugWithContext<C>,
U: DebugWithContext<C>,
{
}

impl<C> DebugWithContext<C> for rustc_middle::mir::Local {}
impl<C> DebugWithContext<C> for crate::dataflow::move_paths::InitIndex {}

Expand Down
28 changes: 28 additions & 0 deletions compiler/rustc_mir/src/dataflow/framework/lattice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,34 @@ impl JoinSemiLattice for bool {
}
}

/// Just for completion, we implement `JoinSemiLattice` for `()`, where top and bottom are
/// the same value.
impl JoinSemiLattice for () {
fn join(&mut self, _: &Self) -> bool {
false
}
}

/// An `Option` is a "two-point" lattice with `Some as the top element and `None` as the bottom:
///
/// ```text
/// Some(T)
/// |
/// None
/// ```
impl<T: JoinSemiLattice + Clone> JoinSemiLattice for Option<T> {
fn join(&mut self, other: &Self) -> bool {
match (self, other) {
(None, None) | (Some(_), None) => false,
(this @ None, Some(val)) => {
*this = Some(val.clone());
true
}
(Some(a), Some(b)) => a.join(b),
}
}
}

impl MeetSemiLattice for bool {
fn meet(&mut self, other: &Self) -> bool {
if let (true, false) = (*self, *other) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::mir;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::Symbol;

pub use self::qualifs::Qualif;
pub(super) use self::qualifs::Qualif;

mod ops;
pub mod post_drop_elaboration;
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_mir/src/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,9 +208,29 @@ impl NonConstOp for LiveDrop {
}
}

#[derive(Debug)]
pub struct CellBorrowBehindRef;
impl NonConstOp for CellBorrowBehindRef {
fn status_in_item(&self, _: &ConstCx<'_, '_>) -> Status {
Status::Unstable(sym::const_refs_to_cell)
}
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
feature_err(
&ccx.tcx.sess.parse_sess,
sym::const_refs_to_cell,
span,
"cannot borrow here, since the borrowed element may contain interior mutability",
)
}
}

#[derive(Debug)]
pub struct CellBorrow;
impl NonConstOp for CellBorrow {
fn importance(&self) -> DiagnosticImportance {
// The problematic cases will already emit a `CellBorrowBehindRef`
DiagnosticImportance::Secondary
}
fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
struct_span_err!(
ccx.tcx.sess,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
match &terminator.kind {
mir::TerminatorKind::Drop { place: dropped_place, .. } => {
let dropped_ty = dropped_place.ty(self.body, self.tcx).ty;
if !NeedsDrop::in_any_value_of_ty(self.ccx, dropped_ty) {
if NeedsDrop::in_any_value_of_ty(self.ccx, dropped_ty).is_none() {
return;
}

Expand All @@ -87,7 +87,7 @@ impl Visitor<'tcx> for CheckLiveDrops<'mir, 'tcx> {
return;
}

if self.qualifs.needs_drop(self.ccx, dropped_place.local, location) {
if self.qualifs.needs_drop(self.ccx, dropped_place.local, location).is_some() {
// Use the span where the dropped local was declared for the error.
let span = self.body.local_decls[dropped_place.local].source_info.span;
self.check_live_drop(span);
Expand Down
Loading