-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Implement dataflow-based const validation #64470
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
Changes from all commits
717c64e
457c3aa
e81297d
83a3e04
eec93ca
3a5442a
c2e121d
48d3843
3758e38
908dcb8
3698d04
fc92d3b
c990243
670c84d
27bd849
f2ff425
e296436
b3e59bb
93ee779
bc7928a
406ac2e
2f5ea63
dcecefc
1a14d17
713ec15
ff6faab
a302055
8bfe82b
f2e7faf
ff4158a
0bf1a80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
use rustc::mir::visit::Visitor; | ||
use rustc::mir::{self, Local, Location}; | ||
use rustc::ty::{self, TyCtxt}; | ||
use rustc_data_structures::bit_set::BitSet; | ||
use syntax_pos::DUMMY_SP; | ||
|
||
use crate::dataflow::{self, GenKillSet}; | ||
|
||
/// Whether a borrow to a `Local` has been created that could allow that `Local` to be mutated | ||
/// indirectly. This could either be a mutable reference (`&mut`) or a shared borrow if the type of | ||
/// that `Local` allows interior mutability. Operations that can mutate local's indirectly include: | ||
/// assignments through a pointer (`*p = 42`), function calls, drop terminators and inline assembly. | ||
/// | ||
/// If this returns false for a `Local` at a given statement (or terminator), that `Local` could | ||
/// not possibly have been mutated indirectly prior to that statement. | ||
#[derive(Copy, Clone)] | ||
pub struct IndirectlyMutableLocals<'mir, 'tcx> { | ||
body: &'mir mir::Body<'tcx>, | ||
tcx: TyCtxt<'tcx>, | ||
param_env: ty::ParamEnv<'tcx>, | ||
} | ||
|
||
impl<'mir, 'tcx> IndirectlyMutableLocals<'mir, 'tcx> { | ||
pub fn new( | ||
tcx: TyCtxt<'tcx>, | ||
body: &'mir mir::Body<'tcx>, | ||
param_env: ty::ParamEnv<'tcx>, | ||
) -> Self { | ||
IndirectlyMutableLocals { body, tcx, param_env } | ||
} | ||
|
||
fn transfer_function<'a>( | ||
&self, | ||
trans: &'a mut GenKillSet<Local>, | ||
) -> TransferFunction<'a, 'mir, 'tcx> { | ||
TransferFunction { | ||
body: self.body, | ||
tcx: self.tcx, | ||
param_env: self.param_env, | ||
trans | ||
} | ||
} | ||
} | ||
|
||
impl<'mir, 'tcx> dataflow::BitDenotation<'tcx> for IndirectlyMutableLocals<'mir, 'tcx> { | ||
type Idx = Local; | ||
|
||
fn name() -> &'static str { "mut_borrowed_locals" } | ||
|
||
fn bits_per_block(&self) -> usize { | ||
self.body.local_decls.len() | ||
} | ||
|
||
fn start_block_effect(&self, _entry_set: &mut BitSet<Local>) { | ||
// Nothing is borrowed on function entry | ||
} | ||
|
||
fn statement_effect( | ||
&self, | ||
trans: &mut GenKillSet<Local>, | ||
loc: Location, | ||
) { | ||
let stmt = &self.body[loc.block].statements[loc.statement_index]; | ||
self.transfer_function(trans).visit_statement(stmt, loc); | ||
} | ||
|
||
fn terminator_effect( | ||
&self, | ||
trans: &mut GenKillSet<Local>, | ||
loc: Location, | ||
) { | ||
let terminator = self.body[loc.block].terminator(); | ||
self.transfer_function(trans).visit_terminator(terminator, loc); | ||
} | ||
|
||
fn propagate_call_return( | ||
&self, | ||
_in_out: &mut BitSet<Local>, | ||
_call_bb: mir::BasicBlock, | ||
_dest_bb: mir::BasicBlock, | ||
_dest_place: &mir::Place<'tcx>, | ||
) { | ||
// Nothing to do when a call returns successfully | ||
} | ||
} | ||
|
||
impl<'mir, 'tcx> dataflow::BottomValue for IndirectlyMutableLocals<'mir, 'tcx> { | ||
// bottom = unborrowed | ||
const BOTTOM_VALUE: bool = false; | ||
} | ||
|
||
/// A `Visitor` that defines the transfer function for `IndirectlyMutableLocals`. | ||
struct TransferFunction<'a, 'mir, 'tcx> { | ||
trans: &'a mut GenKillSet<Local>, | ||
body: &'mir mir::Body<'tcx>, | ||
tcx: TyCtxt<'tcx>, | ||
param_env: ty::ParamEnv<'tcx>, | ||
} | ||
|
||
impl<'tcx> Visitor<'tcx> for TransferFunction<'_, '_, 'tcx> { | ||
fn visit_rvalue( | ||
&mut self, | ||
rvalue: &mir::Rvalue<'tcx>, | ||
location: Location, | ||
) { | ||
if let mir::Rvalue::Ref(_, kind, ref borrowed_place) = *rvalue { | ||
let is_mut = match kind { | ||
mir::BorrowKind::Mut { .. } => true, | ||
|
||
| mir::BorrowKind::Shared | ||
| mir::BorrowKind::Shallow | ||
| mir::BorrowKind::Unique | ||
=> { | ||
!borrowed_place | ||
.ty(self.body, self.tcx) | ||
.ty | ||
.is_freeze(self.tcx, self.param_env, DUMMY_SP) | ||
} | ||
}; | ||
|
||
if is_mut { | ||
match borrowed_place.base { | ||
mir::PlaceBase::Local(borrowed_local) if !borrowed_place.is_indirect() | ||
=> self.trans.gen(borrowed_local), | ||
|
||
_ => (), | ||
} | ||
} | ||
} | ||
|
||
self.super_rvalue(rvalue, location); | ||
} | ||
|
||
|
||
fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { | ||
// This method purposely does nothing except call `super_terminator`. It exists solely to | ||
// document the subtleties around drop terminators. | ||
|
||
self.super_terminator(terminator, location); | ||
|
||
if let mir::TerminatorKind::Drop { location: _, .. } | ||
| mir::TerminatorKind::DropAndReplace { location: _, .. } = &terminator.kind | ||
{ | ||
// Although drop terminators mutably borrow the location being dropped, that borrow | ||
// cannot live beyond the drop terminator because the dropped location is invalidated. | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
//! Check the bodies of `const`s, `static`s and `const fn`s for illegal operations. | ||
//! | ||
//! This module will eventually replace the parts of `qualify_consts.rs` that check whether a local | ||
//! has interior mutability or needs to be dropped, as well as the visitor that emits errors when | ||
//! it finds operations that are invalid in a certain context. | ||
|
||
use rustc::hir::def_id::DefId; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Giving the module (and submodules) some docs (explaining the overall structure of it, the purpose, etc.) would be good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True. I'll work on this when we get closer to the end and things are more stable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a short description in 60ea470aefdd9a3234c83803d3a06cd49559920a. |
||
use rustc::mir; | ||
use rustc::ty::{self, TyCtxt}; | ||
|
||
pub use self::qualifs::Qualif; | ||
|
||
pub mod ops; | ||
mod qualifs; | ||
mod resolver; | ||
pub mod validation; | ||
|
||
/// Information about the item currently being validated, as well as a reference to the global | ||
/// context. | ||
pub struct Item<'mir, 'tcx> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this isn't the best name? But I don't have any suggestions right now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name of the equivalent data structure in the old code was There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This type appears in a lot of function signatures (even more once this alias disappears), so brevity is desirable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit shorter There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I defer to others, but I'm not enthusiastic about any of the alternatives. Writing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that spirit, maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but I don't have an opinion either way, just wanted to say why it would be consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
👍 -- you are right, I think I was considering |
||
body: &'mir mir::Body<'tcx>, | ||
tcx: TyCtxt<'tcx>, | ||
def_id: DefId, | ||
param_env: ty::ParamEnv<'tcx>, | ||
mode: validation::Mode, | ||
} | ||
|
||
impl Item<'mir, 'tcx> { | ||
pub fn new( | ||
tcx: TyCtxt<'tcx>, | ||
def_id: DefId, | ||
body: &'mir mir::Body<'tcx>, | ||
) -> Self { | ||
let param_env = tcx.param_env(def_id); | ||
let mode = validation::Mode::for_item(tcx, def_id) | ||
.expect("const validation must only be run inside a const context"); | ||
|
||
Item { | ||
body, | ||
tcx, | ||
def_id, | ||
param_env, | ||
mode, | ||
} | ||
} | ||
} | ||
|
||
|
||
fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { | ||
Some(def_id) == tcx.lang_items().panic_fn() || | ||
Some(def_id) == tcx.lang_items().begin_panic_fn() | ||
} |
Uh oh!
There was an error while loading. Please reload this page.