Skip to content

Commit 8819721

Browse files
committed
Auto merge of #75573 - Aaron1011:feature/const-mutation-lint, r=oli-obk
Add CONST_ITEM_MUTATION lint Fixes #74053 Fixes #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.
2 parents a1894e4 + 4434e8c commit 8819721

File tree

22 files changed

+430
-119
lines changed

22 files changed

+430
-119
lines changed

compiler/rustc_middle/src/mir/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,8 @@ pub enum LocalInfo<'tcx> {
922922
User(ClearCrossCrate<BindingForm<'tcx>>),
923923
/// A temporary created that references the static with the given `DefId`.
924924
StaticRef { def_id: DefId, is_thread_local: bool },
925+
/// A temporary created that references the const with the given `DefId`
926+
ConstRef { def_id: DefId },
925927
}
926928

927929
impl<'tcx> LocalDecl<'tcx> {

compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs

+40-57
Original file line numberDiff line numberDiff line change
@@ -804,68 +804,51 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
804804
debug!("move_spans: target_temp = {:?}", target_temp);
805805

806806
if let Some(Terminator {
807-
kind: TerminatorKind::Call { func, args, fn_span, from_hir_call, .. },
808-
..
807+
kind: TerminatorKind::Call { fn_span, from_hir_call, .. }, ..
809808
}) = &self.body[location.block].terminator
810809
{
811-
let mut method_did = None;
812-
if let Operand::Constant(box Constant { literal: ty::Const { ty, .. }, .. }) = func {
813-
if let ty::FnDef(def_id, _) = *ty.kind() {
814-
debug!("move_spans: fn = {:?}", def_id);
815-
if let Some(ty::AssocItem { fn_has_self_parameter, .. }) =
816-
self.infcx.tcx.opt_associated_item(def_id)
817-
{
818-
if *fn_has_self_parameter {
819-
method_did = Some(def_id);
820-
}
821-
}
822-
}
823-
}
810+
let method_did = if let Some(method_did) =
811+
crate::util::find_self_call(self.infcx.tcx, &self.body, target_temp, location.block)
812+
{
813+
method_did
814+
} else {
815+
return normal_ret;
816+
};
824817

825818
let tcx = self.infcx.tcx;
826-
let method_did = if let Some(did) = method_did { did } else { return normal_ret };
827-
828-
if let [Operand::Move(self_place), ..] = **args {
829-
if self_place.as_local() == Some(target_temp) {
830-
let parent = tcx.parent(method_did);
831-
let is_fn_once = parent == tcx.lang_items().fn_once_trait();
832-
let is_operator = !from_hir_call
833-
&& parent.map_or(false, |p| {
834-
tcx.lang_items().group(LangItemGroup::Op).contains(&p)
835-
});
836-
let fn_call_span = *fn_span;
837-
838-
let self_arg = tcx.fn_arg_names(method_did)[0];
839-
840-
let kind = if is_fn_once {
841-
FnSelfUseKind::FnOnceCall
842-
} else if is_operator {
843-
FnSelfUseKind::Operator { self_arg }
844-
} else {
845-
debug!(
846-
"move_spans: method_did={:?}, fn_call_span={:?}",
847-
method_did, fn_call_span
848-
);
849-
let implicit_into_iter = matches!(
850-
fn_call_span.desugaring_kind(),
851-
Some(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
852-
);
853-
FnSelfUseKind::Normal { self_arg, implicit_into_iter }
854-
};
855819

856-
return FnSelfUse {
857-
var_span: stmt.source_info.span,
858-
fn_call_span,
859-
fn_span: self
860-
.infcx
861-
.tcx
862-
.sess
863-
.source_map()
864-
.guess_head_span(self.infcx.tcx.def_span(method_did)),
865-
kind,
866-
};
867-
}
868-
}
820+
let parent = tcx.parent(method_did);
821+
let is_fn_once = parent == tcx.lang_items().fn_once_trait();
822+
let is_operator = !from_hir_call
823+
&& parent.map_or(false, |p| tcx.lang_items().group(LangItemGroup::Op).contains(&p));
824+
let fn_call_span = *fn_span;
825+
826+
let self_arg = tcx.fn_arg_names(method_did)[0];
827+
828+
let kind = if is_fn_once {
829+
FnSelfUseKind::FnOnceCall
830+
} else if is_operator {
831+
FnSelfUseKind::Operator { self_arg }
832+
} else {
833+
debug!("move_spans: method_did={:?}, fn_call_span={:?}", method_did, fn_call_span);
834+
let implicit_into_iter = matches!(
835+
fn_call_span.desugaring_kind(),
836+
Some(DesugaringKind::ForLoop(ForLoopLoc::IntoIter))
837+
);
838+
FnSelfUseKind::Normal { self_arg, implicit_into_iter }
839+
};
840+
841+
return FnSelfUse {
842+
var_span: stmt.source_info.span,
843+
fn_call_span,
844+
fn_span: self
845+
.infcx
846+
.tcx
847+
.sess
848+
.source_map()
849+
.guess_head_span(self.infcx.tcx.def_span(method_did)),
850+
kind,
851+
};
869852
}
870853
normal_ret
871854
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
use rustc_errors::DiagnosticBuilder;
2+
use rustc_middle::lint::LintDiagnosticBuilder;
3+
use rustc_middle::mir::visit::Visitor;
4+
use rustc_middle::mir::*;
5+
use rustc_middle::ty::TyCtxt;
6+
use rustc_session::lint::builtin::CONST_ITEM_MUTATION;
7+
use rustc_span::def_id::DefId;
8+
9+
use crate::transform::{MirPass, MirSource};
10+
11+
pub struct CheckConstItemMutation;
12+
13+
impl<'tcx> MirPass<'tcx> for CheckConstItemMutation {
14+
fn run_pass(&self, tcx: TyCtxt<'tcx>, _src: MirSource<'tcx>, body: &mut Body<'tcx>) {
15+
let mut checker = ConstMutationChecker { body, tcx, target_local: None };
16+
checker.visit_body(&body);
17+
}
18+
}
19+
20+
struct ConstMutationChecker<'a, 'tcx> {
21+
body: &'a Body<'tcx>,
22+
tcx: TyCtxt<'tcx>,
23+
target_local: Option<Local>,
24+
}
25+
26+
impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> {
27+
fn is_const_item(&self, local: Local) -> Option<DefId> {
28+
if let Some(box LocalInfo::ConstRef { def_id }) = self.body.local_decls[local].local_info {
29+
Some(def_id)
30+
} else {
31+
None
32+
}
33+
}
34+
fn lint_const_item_usage(
35+
&self,
36+
const_item: DefId,
37+
location: Location,
38+
decorate: impl for<'b> FnOnce(LintDiagnosticBuilder<'b>) -> DiagnosticBuilder<'b>,
39+
) {
40+
let source_info = self.body.source_info(location);
41+
let lint_root = self.body.source_scopes[source_info.scope]
42+
.local_data
43+
.as_ref()
44+
.assert_crate_local()
45+
.lint_root;
46+
47+
self.tcx.struct_span_lint_hir(CONST_ITEM_MUTATION, lint_root, source_info.span, |lint| {
48+
decorate(lint)
49+
.span_note(self.tcx.def_span(const_item), "`const` item defined here")
50+
.emit()
51+
});
52+
}
53+
}
54+
55+
impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> {
56+
fn visit_statement(&mut self, stmt: &Statement<'tcx>, loc: Location) {
57+
if let StatementKind::Assign(box (lhs, _)) = &stmt.kind {
58+
// Check for assignment to fields of a constant
59+
// Assigning directly to a constant (e.g. `FOO = true;`) is a hard error,
60+
// so emitting a lint would be redundant.
61+
if !lhs.projection.is_empty() {
62+
if let Some(def_id) = self.is_const_item(lhs.local) {
63+
self.lint_const_item_usage(def_id, loc, |lint| {
64+
let mut lint = lint.build("attempting to modify a `const` item");
65+
lint.note("each usage of a `const` item creates a new temporary - the original `const` item will not be modified");
66+
lint
67+
})
68+
}
69+
}
70+
// We are looking for MIR of the form:
71+
//
72+
// ```
73+
// _1 = const FOO;
74+
// _2 = &mut _1;
75+
// method_call(_2, ..)
76+
// ```
77+
//
78+
// Record our current LHS, so that we can detect this
79+
// pattern in `visit_rvalue`
80+
self.target_local = lhs.as_local();
81+
}
82+
self.super_statement(stmt, loc);
83+
self.target_local = None;
84+
}
85+
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, loc: Location) {
86+
if let Rvalue::Ref(_, BorrowKind::Mut { .. }, place) = rvalue {
87+
let local = place.local;
88+
if let Some(def_id) = self.is_const_item(local) {
89+
// If this Rvalue is being used as the right-hand side of a
90+
// `StatementKind::Assign`, see if it ends up getting used as
91+
// the `self` parameter of a method call (as the terminator of our current
92+
// BasicBlock). If so, we emit a more specific lint.
93+
let method_did = self.target_local.and_then(|target_local| {
94+
crate::util::find_self_call(self.tcx, &self.body, target_local, loc.block)
95+
});
96+
let lint_loc =
97+
if method_did.is_some() { self.body.terminator_loc(loc.block) } else { loc };
98+
self.lint_const_item_usage(def_id, lint_loc, |lint| {
99+
let mut lint = lint.build("taking a mutable reference to a `const` item");
100+
lint
101+
.note("each usage of a `const` item creates a new temporary")
102+
.note("the mutable reference will refer to this temporary, not the original `const` item");
103+
104+
if let Some(method_did) = method_did {
105+
lint.span_note(self.tcx.def_span(method_did), "mutable reference created due to call to this method");
106+
}
107+
108+
lint
109+
});
110+
}
111+
}
112+
self.super_rvalue(rvalue, loc);
113+
}
114+
}

compiler/rustc_mir/src/transform/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::borrow::Cow;
1616
pub mod add_call_guards;
1717
pub mod add_moves_for_packed_drops;
1818
pub mod add_retag;
19+
pub mod check_const_item_mutation;
1920
pub mod check_consts;
2021
pub mod check_packed_ref;
2122
pub mod check_unsafety;
@@ -307,6 +308,7 @@ fn mir_const<'tcx>(
307308
&[&[
308309
// MIR-level lints.
309310
&check_packed_ref::CheckPackedRef,
311+
&check_const_item_mutation::CheckConstItemMutation,
310312
// What we need to do constant evaluation.
311313
&simplify::SimplifyCfg::new("initial"),
312314
&rustc_peek::SanityCheck,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
use rustc_middle::mir::*;
2+
use rustc_middle::ty::{self, TyCtxt};
3+
use rustc_span::def_id::DefId;
4+
5+
/// Checks if the specified `local` is used as the `self` prameter of a method call
6+
/// in the provided `BasicBlock`. If it is, then the `DefId` of the called method is
7+
/// returned.
8+
pub fn find_self_call(
9+
tcx: TyCtxt<'_>,
10+
body: &Body<'_>,
11+
local: Local,
12+
block: BasicBlock,
13+
) -> Option<DefId> {
14+
debug!("find_self_call(local={:?}): terminator={:?}", local, &body[block].terminator);
15+
if let Some(Terminator { kind: TerminatorKind::Call { func, args, .. }, .. }) =
16+
&body[block].terminator
17+
{
18+
debug!("find_self_call: func={:?}", func);
19+
if let Operand::Constant(box Constant { literal: ty::Const { ty, .. }, .. }) = func {
20+
if let ty::FnDef(def_id, _) = *ty.kind() {
21+
if let Some(ty::AssocItem { fn_has_self_parameter: true, .. }) =
22+
tcx.opt_associated_item(def_id)
23+
{
24+
debug!("find_self_call: args={:?}", args);
25+
if let [Operand::Move(self_place) | Operand::Copy(self_place), ..] = **args {
26+
if self_place.as_local() == Some(local) {
27+
return Some(def_id);
28+
}
29+
}
30+
}
31+
}
32+
}
33+
}
34+
None
35+
}

compiler/rustc_mir/src/util/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ pub mod storage;
77

88
mod alignment;
99
pub mod collect_writes;
10+
mod find_self_call;
1011
mod graphviz;
1112
pub(crate) mod pretty;
1213
pub(crate) mod spanview;
1314

1415
pub use self::aggregate::expand_aggregate;
1516
pub use self::alignment::is_disaligned;
17+
pub use self::find_self_call::find_self_call;
1618
pub use self::graphviz::write_node_label as write_graphviz_node_label;
1719
pub use self::graphviz::{graphviz_safe_def_name, write_mir_graphviz};
1820
pub use self::pretty::{dump_enabled, dump_mir, write_mir_pretty, PassWhere};

compiler/rustc_mir_build/src/build/expr/as_constant.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
2121
let Expr { ty, temp_lifetime: _, span, kind } = expr;
2222
match kind {
2323
ExprKind::Scope { region_scope: _, lint_level: _, value } => this.as_constant(value),
24-
ExprKind::Literal { literal, user_ty } => {
24+
ExprKind::Literal { literal, user_ty, const_id: _ } => {
2525
let user_ty = user_ty.map(|user_ty| {
2626
this.canonical_user_type_annotations.push(CanonicalUserTypeAnnotation {
2727
span,

compiler/rustc_mir_build/src/build/expr/as_temp.rs

+3
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
7676
local_decl.local_info =
7777
Some(box LocalInfo::StaticRef { def_id, is_thread_local: true });
7878
}
79+
ExprKind::Literal { const_id: Some(def_id), .. } => {
80+
local_decl.local_info = Some(box LocalInfo::ConstRef { def_id });
81+
}
7982
_ => {}
8083
}
8184
this.local_decls.push(local_decl)

compiler/rustc_mir_build/src/thir/cx/expr.rs

+13-2
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(
247247
hir::ExprKind::Lit(ref lit) => ExprKind::Literal {
248248
literal: cx.const_eval_literal(&lit.node, expr_ty, lit.span, false),
249249
user_ty: None,
250+
const_id: None,
250251
},
251252

252253
hir::ExprKind::Binary(op, ref lhs, ref rhs) => {
@@ -306,6 +307,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(
306307
ExprKind::Literal {
307308
literal: cx.const_eval_literal(&lit.node, expr_ty, lit.span, true),
308309
user_ty: None,
310+
const_id: None,
309311
}
310312
} else {
311313
ExprKind::Unary { op: UnOp::Neg, arg: arg.to_ref() }
@@ -447,6 +449,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(
447449
kind: ExprKind::Literal {
448450
literal: ty::Const::zero_sized(cx.tcx, ty),
449451
user_ty,
452+
const_id: None,
450453
},
451454
}
452455
.to_ref(),
@@ -473,6 +476,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(
473476
kind: ExprKind::Literal {
474477
literal: ty::Const::zero_sized(cx.tcx, ty),
475478
user_ty: None,
479+
const_id: None,
476480
},
477481
}
478482
.to_ref(),
@@ -585,7 +589,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(
585589
temp_lifetime,
586590
ty: var_ty,
587591
span: expr.span,
588-
kind: ExprKind::Literal { literal, user_ty: None },
592+
kind: ExprKind::Literal { literal, user_ty: None, const_id: None },
589593
}
590594
.to_ref()
591595
};
@@ -714,7 +718,11 @@ fn method_callee<'a, 'tcx>(
714718
temp_lifetime,
715719
ty,
716720
span,
717-
kind: ExprKind::Literal { literal: ty::Const::zero_sized(cx.tcx(), ty), user_ty },
721+
kind: ExprKind::Literal {
722+
literal: ty::Const::zero_sized(cx.tcx(), ty),
723+
user_ty,
724+
const_id: None,
725+
},
718726
}
719727
}
720728

@@ -777,6 +785,7 @@ fn convert_path_expr<'a, 'tcx>(
777785
ExprKind::Literal {
778786
literal: ty::Const::zero_sized(cx.tcx, cx.typeck_results().node_type(expr.hir_id)),
779787
user_ty,
788+
const_id: None,
780789
}
781790
}
782791

@@ -794,6 +803,7 @@ fn convert_path_expr<'a, 'tcx>(
794803
.tcx
795804
.mk_const(ty::Const { val, ty: cx.typeck_results().node_type(expr.hir_id) }),
796805
user_ty: None,
806+
const_id: Some(def_id),
797807
}
798808
}
799809

@@ -810,6 +820,7 @@ fn convert_path_expr<'a, 'tcx>(
810820
ty: cx.typeck_results().node_type(expr.hir_id),
811821
}),
812822
user_ty,
823+
const_id: Some(def_id),
813824
}
814825
}
815826

0 commit comments

Comments
 (0)