-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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.
- Loading branch information
Showing
22 changed files
with
430 additions
and
119 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
114 changes: 114 additions & 0 deletions
114
compiler/rustc_mir/src/transform/check_const_item_mutation.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| use rustc_errors::DiagnosticBuilder; | ||
| use rustc_middle::lint::LintDiagnosticBuilder; | ||
| use rustc_middle::mir::visit::Visitor; | ||
| use rustc_middle::mir::*; | ||
| use rustc_middle::ty::TyCtxt; | ||
| use rustc_session::lint::builtin::CONST_ITEM_MUTATION; | ||
| use rustc_span::def_id::DefId; | ||
|
|
||
| use crate::transform::{MirPass, MirSource}; | ||
|
|
||
| pub struct CheckConstItemMutation; | ||
|
|
||
| impl<'tcx> MirPass<'tcx> for CheckConstItemMutation { | ||
| fn run_pass(&self, tcx: TyCtxt<'tcx>, _src: MirSource<'tcx>, body: &mut Body<'tcx>) { | ||
| let mut checker = ConstMutationChecker { body, tcx, target_local: None }; | ||
| checker.visit_body(&body); | ||
| } | ||
| } | ||
|
|
||
| struct ConstMutationChecker<'a, 'tcx> { | ||
| body: &'a Body<'tcx>, | ||
| tcx: TyCtxt<'tcx>, | ||
| target_local: Option<Local>, | ||
| } | ||
|
|
||
| impl<'a, 'tcx> ConstMutationChecker<'a, 'tcx> { | ||
| fn is_const_item(&self, local: Local) -> Option<DefId> { | ||
| if let Some(box LocalInfo::ConstRef { def_id }) = self.body.local_decls[local].local_info { | ||
| Some(def_id) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| fn lint_const_item_usage( | ||
| &self, | ||
| const_item: DefId, | ||
| location: Location, | ||
| decorate: impl for<'b> FnOnce(LintDiagnosticBuilder<'b>) -> DiagnosticBuilder<'b>, | ||
| ) { | ||
| let source_info = self.body.source_info(location); | ||
| let lint_root = self.body.source_scopes[source_info.scope] | ||
| .local_data | ||
| .as_ref() | ||
| .assert_crate_local() | ||
| .lint_root; | ||
|
|
||
| self.tcx.struct_span_lint_hir(CONST_ITEM_MUTATION, lint_root, source_info.span, |lint| { | ||
| decorate(lint) | ||
| .span_note(self.tcx.def_span(const_item), "`const` item defined here") | ||
| .emit() | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| impl<'a, 'tcx> Visitor<'tcx> for ConstMutationChecker<'a, 'tcx> { | ||
| fn visit_statement(&mut self, stmt: &Statement<'tcx>, loc: Location) { | ||
| if let StatementKind::Assign(box (lhs, _)) = &stmt.kind { | ||
| // Check for assignment to fields of a constant | ||
| // Assigning directly to a constant (e.g. `FOO = true;`) is a hard error, | ||
| // so emitting a lint would be redundant. | ||
| if !lhs.projection.is_empty() { | ||
| if let Some(def_id) = self.is_const_item(lhs.local) { | ||
| self.lint_const_item_usage(def_id, loc, |lint| { | ||
| let mut lint = lint.build("attempting to modify a `const` item"); | ||
| lint.note("each usage of a `const` item creates a new temporary - the original `const` item will not be modified"); | ||
| lint | ||
| }) | ||
| } | ||
| } | ||
| // We are looking for MIR of the form: | ||
| // | ||
| // ``` | ||
| // _1 = const FOO; | ||
| // _2 = &mut _1; | ||
| // method_call(_2, ..) | ||
| // ``` | ||
| // | ||
| // Record our current LHS, so that we can detect this | ||
| // pattern in `visit_rvalue` | ||
| self.target_local = lhs.as_local(); | ||
| } | ||
| self.super_statement(stmt, loc); | ||
| self.target_local = None; | ||
| } | ||
| fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, loc: Location) { | ||
| if let Rvalue::Ref(_, BorrowKind::Mut { .. }, place) = rvalue { | ||
| let local = place.local; | ||
| if let Some(def_id) = self.is_const_item(local) { | ||
| // If this Rvalue is being used as the right-hand side of a | ||
| // `StatementKind::Assign`, see if it ends up getting used as | ||
| // the `self` parameter of a method call (as the terminator of our current | ||
| // BasicBlock). If so, we emit a more specific lint. | ||
| let method_did = self.target_local.and_then(|target_local| { | ||
| crate::util::find_self_call(self.tcx, &self.body, target_local, loc.block) | ||
| }); | ||
| let lint_loc = | ||
| if method_did.is_some() { self.body.terminator_loc(loc.block) } else { loc }; | ||
| self.lint_const_item_usage(def_id, lint_loc, |lint| { | ||
| let mut lint = lint.build("taking a mutable reference to a `const` item"); | ||
| lint | ||
| .note("each usage of a `const` item creates a new temporary") | ||
| .note("the mutable reference will refer to this temporary, not the original `const` item"); | ||
|
|
||
| if let Some(method_did) = method_did { | ||
| lint.span_note(self.tcx.def_span(method_did), "mutable reference created due to call to this method"); | ||
| } | ||
|
|
||
| lint | ||
| }); | ||
| } | ||
| } | ||
| self.super_rvalue(rvalue, loc); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| use rustc_middle::mir::*; | ||
| use rustc_middle::ty::{self, TyCtxt}; | ||
| use rustc_span::def_id::DefId; | ||
|
|
||
| /// Checks if the specified `local` is used as the `self` prameter of a method call | ||
| /// in the provided `BasicBlock`. If it is, then the `DefId` of the called method is | ||
| /// returned. | ||
| pub fn find_self_call( | ||
| tcx: TyCtxt<'_>, | ||
| body: &Body<'_>, | ||
| local: Local, | ||
| block: BasicBlock, | ||
| ) -> Option<DefId> { | ||
| debug!("find_self_call(local={:?}): terminator={:?}", local, &body[block].terminator); | ||
| if let Some(Terminator { kind: TerminatorKind::Call { func, args, .. }, .. }) = | ||
| &body[block].terminator | ||
| { | ||
| debug!("find_self_call: func={:?}", func); | ||
| if let Operand::Constant(box Constant { literal: ty::Const { ty, .. }, .. }) = func { | ||
| if let ty::FnDef(def_id, _) = *ty.kind() { | ||
| if let Some(ty::AssocItem { fn_has_self_parameter: true, .. }) = | ||
| tcx.opt_associated_item(def_id) | ||
| { | ||
| debug!("find_self_call: args={:?}", args); | ||
| if let [Operand::Move(self_place) | Operand::Copy(self_place), ..] = **args { | ||
| if self_place.as_local() == Some(local) { | ||
| return Some(def_id); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| None | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.