From 8c7500f9b68d56b8313a8d3c103fd31b638ec8d2 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 13 Sep 2017 22:33:07 +0300 Subject: [PATCH 1/5] add lint levels to VisibilityScope --- src/librustc/ich/impls_mir.rs | 18 ++++++++ src/librustc/lint/levels.rs | 5 ++ src/librustc/mir/mod.rs | 45 +++++++++++++++++- src/librustc/mir/visit.rs | 2 + src/librustc_driver/driver.rs | 7 +-- src/librustc_mir/build/block.rs | 18 ++++++-- src/librustc_mir/build/expr/as_constant.rs | 2 +- src/librustc_mir/build/expr/as_lvalue.rs | 4 +- src/librustc_mir/build/expr/as_operand.rs | 4 +- src/librustc_mir/build/expr/as_rvalue.rs | 5 +- src/librustc_mir/build/expr/as_temp.rs | 4 +- src/librustc_mir/build/expr/into.rs | 5 +- src/librustc_mir/build/expr/stmt.rs | 4 +- src/librustc_mir/build/matches/mod.rs | 21 ++++++++- src/librustc_mir/build/mod.rs | 26 ++++++++--- src/librustc_mir/build/scope.rs | 38 ++++++++++++++-- src/librustc_mir/hair/cx/block.rs | 1 + src/librustc_mir/hair/cx/expr.rs | 26 ++++++----- src/librustc_mir/hair/cx/mod.rs | 48 +++++++++++++++++++- src/librustc_mir/hair/mod.rs | 22 ++++++++- src/librustc_mir/shim.rs | 5 ++ src/librustc_mir/transform/generator.rs | 3 ++ src/librustc_mir/transform/promote_consts.rs | 8 ++-- src/librustc_typeck/check/mod.rs | 1 - 24 files changed, 269 insertions(+), 53 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index 9b6613e4cae96..068830d688c3e 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -28,6 +28,7 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> { name, source_info, internal, + lexical_scope, is_user_variable }); impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, by_ref }); @@ -75,6 +76,22 @@ for mir::Terminator<'gcx> { } } +impl<'gcx, T> HashStable> for mir::ClearOnDecode + where T: HashStable> +{ + #[inline] + fn hash_stable(&self, + hcx: &mut StableHashingContext<'gcx>, + hasher: &mut StableHasher) { + mem::discriminant(self).hash_stable(hcx, hasher); + match *self { + mir::ClearOnDecode::Clear => {} + mir::ClearOnDecode::Set(ref value) => { + value.hash_stable(hcx, hasher); + } + } + } +} impl<'gcx> HashStable> for mir::Local { #[inline] @@ -347,6 +364,7 @@ for mir::ProjectionElem<'gcx, V, T> } impl_stable_hash_for!(struct mir::VisibilityScopeData { span, parent_scope }); +impl_stable_hash_for!(struct mir::VisibilityScopeInfo { lint_root }); impl<'gcx> HashStable> for mir::Operand<'gcx> { fn hash_stable(&self, diff --git a/src/librustc/lint/levels.rs b/src/librustc/lint/levels.rs index 21dfd3267df51..4bc37747f2a76 100644 --- a/src/librustc/lint/levels.rs +++ b/src/librustc/lint/levels.rs @@ -384,6 +384,11 @@ impl LintLevelMap { self.sets.get_lint_level(lint, *idx, None) }) } + + /// Returns if this `id` has lint level information. + pub fn lint_level_set(&self, id: HirId) -> Option { + self.id_to_set.get(&id).cloned() + } } impl<'gcx> HashStable> for LintLevelMap { diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index d43504b77ba0c..09da1d1f820bc 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -18,6 +18,7 @@ use rustc_data_structures::indexed_vec::{IndexVec, Idx}; use rustc_data_structures::control_flow_graph::dominators::{Dominators, dominators}; use rustc_data_structures::control_flow_graph::{GraphPredecessors, GraphSuccessors}; use rustc_data_structures::control_flow_graph::ControlFlowGraph; +use rustc_serialize as serialize; use hir::def::CtorKind; use hir::def_id::DefId; use ty::subst::{Subst, Substs}; @@ -33,7 +34,7 @@ use std::fmt::{self, Debug, Formatter, Write}; use std::{iter, u32}; use std::ops::{Index, IndexMut}; use std::vec::IntoIter; -use syntax::ast::Name; +use syntax::ast::{self, Name}; use syntax_pos::Span; mod cache; @@ -96,6 +97,10 @@ pub struct Mir<'tcx> { /// and used (eventually) for debuginfo. Indexed by a `VisibilityScope`. pub visibility_scopes: IndexVec, + /// Crate-local information for each visibility scope, that can't (and + /// needn't) be tracked across crates. + pub visibility_scope_info: ClearOnDecode>, + /// Rvalues promoted from this function, such as borrows of constants. /// Each of them is the Mir of a constant with the fn's type parameters /// in scope, but a separate set of locals. @@ -151,6 +156,8 @@ pub const START_BLOCK: BasicBlock = BasicBlock(0); impl<'tcx> Mir<'tcx> { pub fn new(basic_blocks: IndexVec>, visibility_scopes: IndexVec, + visibility_scope_info: ClearOnDecode>, promoted: IndexVec>, return_ty: Ty<'tcx>, yield_ty: Option>, @@ -167,6 +174,7 @@ impl<'tcx> Mir<'tcx> { Mir { basic_blocks, visibility_scopes, + visibility_scope_info, promoted, return_ty, yield_ty, @@ -278,9 +286,16 @@ impl<'tcx> Mir<'tcx> { } } +#[derive(Clone, Debug)] +pub struct VisibilityScopeInfo { + /// A NodeId with lint levels equivalent to this scope's lint levels. + pub lint_root: ast::NodeId, +} + impl_stable_hash_for!(struct Mir<'tcx> { basic_blocks, visibility_scopes, + visibility_scope_info, promoted, return_ty, yield_ty, @@ -310,6 +325,24 @@ impl<'tcx> IndexMut for Mir<'tcx> { } } +#[derive(Clone, Debug)] +pub enum ClearOnDecode { + Clear, + Set(T) +} + +impl serialize::Encodable for ClearOnDecode { + fn encode(&self, s: &mut S) -> Result<(), S::Error> { + serialize::Encodable::encode(&(), s) + } +} + +impl serialize::Decodable for ClearOnDecode { + fn decode(d: &mut D) -> Result { + serialize::Decodable::decode(d).map(|()| ClearOnDecode::Clear) + } +} + /// Grouped information about the source code origin of a MIR entity. /// Intended to be inspected by diagnostics and debuginfo. /// Most passes can work with it as a whole, within a single function. @@ -438,6 +471,12 @@ pub struct LocalDecl<'tcx> { /// Source info of the local. pub source_info: SourceInfo, + + /// The *lexical* visibility scope the local is defined + /// in. If the local was defined in a let-statement, this + /// is *within* the let-statement, rather than outside + /// of iit. + pub lexical_scope: VisibilityScope, } impl<'tcx> LocalDecl<'tcx> { @@ -452,6 +491,7 @@ impl<'tcx> LocalDecl<'tcx> { span, scope: ARGUMENT_VISIBILITY_SCOPE }, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: false, is_user_variable: false } @@ -468,6 +508,7 @@ impl<'tcx> LocalDecl<'tcx> { span, scope: ARGUMENT_VISIBILITY_SCOPE }, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: true, is_user_variable: false } @@ -485,6 +526,7 @@ impl<'tcx> LocalDecl<'tcx> { span, scope: ARGUMENT_VISIBILITY_SCOPE }, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: false, name: None, // FIXME maybe we do want some name here? is_user_variable: false @@ -1607,6 +1649,7 @@ impl<'tcx> TypeFoldable<'tcx> for Mir<'tcx> { Mir { basic_blocks: self.basic_blocks.fold_with(folder), visibility_scopes: self.visibility_scopes.clone(), + visibility_scope_info: self.visibility_scope_info.clone(), promoted: self.promoted.fold_with(folder), return_ty: self.return_ty.fold_with(folder), yield_ty: self.yield_ty.fold_with(folder), diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 37c97ad3dad90..63652980f9b4b 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -690,11 +690,13 @@ macro_rules! make_mir_visitor { name: _, ref $($mutability)* source_info, internal: _, + ref $($mutability)* lexical_scope, is_user_variable: _, } = *local_decl; self.visit_ty(ty, Lookup::Src(*source_info)); self.visit_source_info(source_info); + self.visit_visibility_scope(lexical_scope); } fn super_visibility_scope(&mut self, diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index bcfbc1980cf6a..5116a3087d281 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1009,15 +1009,16 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, // FIXME: ariel points SimplifyBranches should run after // mir-borrowck; otherwise code within `if false { ... }` would // not be checked. - passes.push_pass(MIR_VALIDATED, - mir::transform::simplify_branches::SimplifyBranches::new("initial")); passes.push_pass(MIR_VALIDATED, mir::transform::simplify::SimplifyCfg::new("qualify-consts")); passes.push_pass(MIR_VALIDATED, mir::transform::nll::NLL); // borrowck runs between MIR_VALIDATED and MIR_OPTIMIZED. - // These next passes must be executed together passes.push_pass(MIR_OPTIMIZED, mir::transform::no_landing_pads::NoLandingPads); + passes.push_pass(MIR_OPTIMIZED, + mir::transform::simplify_branches::SimplifyBranches::new("initial")); + + // These next passes must be executed together passes.push_pass(MIR_OPTIMIZED, mir::transform::add_call_guards::CriticalCallEdges); passes.push_pass(MIR_OPTIMIZED, mir::transform::elaborate_drops::ElaborateDrops); passes.push_pass(MIR_OPTIMIZED, mir::transform::no_landing_pads::NoLandingPads); diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 0e412fb27ca68..db04596dd589f 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -24,7 +24,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let Block { region_scope, opt_destruction_scope, span, stmts, expr, targeted_by_break } = self.hir.mirror(ast_block); self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), block, move |this| { - this.in_scope((region_scope, source_info), block, move |this| { + this.in_scope((region_scope, source_info), LintLevel::Inherited, block, move |this| { if targeted_by_break { // This is a `break`-able block (currently only `catch { ... }`) let exit_block = this.cfg.start_new_block(); @@ -76,13 +76,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { StmtKind::Expr { scope, expr } => { unpack!(block = this.in_opt_scope( opt_destruction_scope.map(|de|(de, source_info)), block, |this| { - this.in_scope((scope, source_info), block, |this| { + let si = (scope, source_info); + this.in_scope(si, LintLevel::Inherited, block, |this| { let expr = this.hir.mirror(expr); this.stmt_expr(block, expr) }) })); } - StmtKind::Let { remainder_scope, init_scope, pattern, initializer } => { + StmtKind::Let { + remainder_scope, + init_scope, + pattern, + initializer, + lint_level + } => { // Enter the remainder scope, i.e. the bindings' destruction scope. this.push_scope((remainder_scope, source_info)); let_scope_stack.push(remainder_scope); @@ -90,13 +97,14 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Declare the bindings, which may create a visibility scope. let remainder_span = remainder_scope.span(this.hir.tcx(), &this.hir.region_scope_tree); - let scope = this.declare_bindings(None, remainder_span, &pattern); + let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern); // Evaluate the initializer, if present. if let Some(init) = initializer { unpack!(block = this.in_opt_scope( opt_destruction_scope.map(|de|(de, source_info)), block, move |this| { - this.in_scope((init_scope, source_info), block, move |this| { + let scope = (init_scope, source_info); + this.in_scope(scope, lint_level, block, move |this| { // FIXME #30046 ^~~~ this.expr_into_pattern(block, pattern, init) }) diff --git a/src/librustc_mir/build/expr/as_constant.rs b/src/librustc_mir/build/expr/as_constant.rs index a86b7f4d239a3..a57f1b9549485 100644 --- a/src/librustc_mir/build/expr/as_constant.rs +++ b/src/librustc_mir/build/expr/as_constant.rs @@ -29,7 +29,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let Expr { ty, temp_lifetime: _, span, kind } = expr; match kind { - ExprKind::Scope { region_scope: _, value } => + ExprKind::Scope { region_scope: _, lint_level: _, value } => this.as_constant(value), ExprKind::Literal { literal } => Constant { span: span, ty: ty, literal: literal }, diff --git a/src/librustc_mir/build/expr/as_lvalue.rs b/src/librustc_mir/build/expr/as_lvalue.rs index 9cbaff2c113b6..69d0dd992281e 100644 --- a/src/librustc_mir/build/expr/as_lvalue.rs +++ b/src/librustc_mir/build/expr/as_lvalue.rs @@ -39,8 +39,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let expr_span = expr.span; let source_info = this.source_info(expr_span); match expr.kind { - ExprKind::Scope { region_scope, value } => { - this.in_scope((region_scope, source_info), block, |this| { + ExprKind::Scope { region_scope, lint_level, value } => { + this.in_scope((region_scope, source_info), lint_level, block, |this| { this.as_lvalue(block, value) }) } diff --git a/src/librustc_mir/build/expr/as_operand.rs b/src/librustc_mir/build/expr/as_operand.rs index 0a72ce8d05e14..ea6e4342098bc 100644 --- a/src/librustc_mir/build/expr/as_operand.rs +++ b/src/librustc_mir/build/expr/as_operand.rs @@ -55,10 +55,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { debug!("expr_as_operand(block={:?}, expr={:?})", block, expr); let this = self; - if let ExprKind::Scope { region_scope, value } = expr.kind { + if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind { let source_info = this.source_info(expr.span); let region_scope = (region_scope, source_info); - return this.in_scope(region_scope, block, |this| { + return this.in_scope(region_scope, lint_level, block, |this| { this.as_operand(block, scope, value) }); } diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index f2607b164de26..d17f00b489c31 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -58,9 +58,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = this.source_info(expr_span); match expr.kind { - ExprKind::Scope { region_scope, value } => { + ExprKind::Scope { region_scope, lint_level, value } => { let region_scope = (region_scope, source_info); - this.in_scope(region_scope, block, |this| this.as_rvalue(block, scope, value)) + this.in_scope(region_scope, lint_level, block, + |this| this.as_rvalue(block, scope, value)) } ExprKind::Repeat { value, count } => { let value_operand = unpack!(block = this.as_operand(block, scope, value)); diff --git a/src/librustc_mir/build/expr/as_temp.rs b/src/librustc_mir/build/expr/as_temp.rs index 7826769600bfa..ba422a8183160 100644 --- a/src/librustc_mir/build/expr/as_temp.rs +++ b/src/librustc_mir/build/expr/as_temp.rs @@ -41,8 +41,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let expr_span = expr.span; let source_info = this.source_info(expr_span); - if let ExprKind::Scope { region_scope, value } = expr.kind { - return this.in_scope((region_scope, source_info), block, |this| { + if let ExprKind::Scope { region_scope, lint_level, value } = expr.kind { + return this.in_scope((region_scope, source_info), lint_level, block, |this| { this.as_temp(block, temp_lifetime, value) }); } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index ec06e474980a6..65e16657a031a 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -38,9 +38,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let source_info = this.source_info(expr_span); match expr.kind { - ExprKind::Scope { region_scope, value } => { + ExprKind::Scope { region_scope, lint_level, value } => { let region_scope = (region_scope, source_info); - this.in_scope(region_scope, block, |this| this.into(destination, block, value)) + this.in_scope(region_scope, lint_level, block, + |this| this.into(destination, block, value)) } ExprKind::Block { body: ast_block } => { this.ast_block(destination, block, ast_block, source_info) diff --git a/src/librustc_mir/build/expr/stmt.rs b/src/librustc_mir/build/expr/stmt.rs index 84468d5d6dc18..3cfb0ff4010da 100644 --- a/src/librustc_mir/build/expr/stmt.rs +++ b/src/librustc_mir/build/expr/stmt.rs @@ -22,9 +22,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Handle a number of expressions that don't need a destination at all. This // avoids needing a mountain of temporary `()` variables. match expr.kind { - ExprKind::Scope { region_scope, value } => { + ExprKind::Scope { region_scope, lint_level, value } => { let value = this.hir.mirror(value); - this.in_scope((region_scope, source_info), block, |this| { + this.in_scope((region_scope, source_info), lint_level, block, |this| { this.stmt_expr(block, value) }) } diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index f560fa426e22e..b856e6345dc6b 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -46,8 +46,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Get the arm bodies and their scopes, while declaring bindings. let arm_bodies: Vec<_> = arms.iter().map(|arm| { + // BUG: use arm lint level let body = self.hir.mirror(arm.body.clone()); - let scope = self.declare_bindings(None, body.span, &arm.patterns[0]); + let scope = self.declare_bindings(None, body.span, + LintLevel::Inherited, + &arm.patterns[0]); (body, scope.unwrap_or(self.visibility_scope)) }).collect(); @@ -171,11 +174,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn declare_bindings(&mut self, mut var_scope: Option, scope_span: Span, + lint_level: LintLevel, pattern: &Pattern<'tcx>) -> Option { + assert!(!(var_scope.is_some() && lint_level.is_explicit()), + "can't have both a var and a lint scope at the same time"); self.visit_bindings(pattern, &mut |this, mutability, name, var, span, ty| { if var_scope.is_none() { - var_scope = Some(this.new_visibility_scope(scope_span)); + var_scope = Some(this.new_visibility_scope(scope_span, + LintLevel::Inherited)); + // If we have lints, create a new visibility scope + // that marks the lints for the locals. + if lint_level.is_explicit() { + this.new_visibility_scope(scope_span, lint_level); + } } let source_info = SourceInfo { span, @@ -183,6 +195,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }; this.declare_binding(source_info, mutability, name, var, ty); }); + // Pop any scope we created for the locals. + if let Some(var_scope) = var_scope { + self.visibility_scope = var_scope; + } var_scope } @@ -712,6 +728,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ty: var_ty.clone(), name: Some(name), source_info, + lexical_scope: self.visibility_scope, internal: false, is_user_variable: true, }); diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index be6f8c9e56c40..9c2c5bbcdb8ba 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -11,6 +11,7 @@ use build; use hair::cx::Cx; +use hair::LintLevel; use rustc::hir; use rustc::hir::def_id::DefId; use rustc::middle::region; @@ -277,6 +278,7 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { /// the vector of all scopes that we have created thus far; /// we track this for debuginfo later visibility_scopes: IndexVec, + visibility_scope_info: IndexVec, visibility_scope: VisibilityScope, /// Maps node ids of variable bindings to the `Local`s created for them. @@ -378,8 +380,10 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>, let arg_scope = region::Scope::Arguments(body.value.hir_id.local_id); let mut block = START_BLOCK; let source_info = builder.source_info(span); - unpack!(block = builder.in_scope((call_site_scope, source_info), block, |builder| { - unpack!(block = builder.in_scope((arg_scope, source_info), block, |builder| { + let call_site_s = (call_site_scope, source_info); + unpack!(block = builder.in_scope(call_site_s, LintLevel::Inherited, block, |builder| { + let arg_scope_s = (arg_scope, source_info); + unpack!(block = builder.in_scope(arg_scope_s, LintLevel::Inherited, block, |builder| { builder.args_and_body(block, &arguments, arg_scope, &body.value) })); // Attribute epilogue to function's closing brace @@ -456,9 +460,10 @@ fn construct_const<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>, } fn construct_error<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>, - body_id: hir::BodyId) - -> Mir<'tcx> { - let span = hir.tcx().hir.span(hir.tcx().hir.body_owner(body_id)); + body_id: hir::BodyId) + -> Mir<'tcx> { + let owner_id = hir.tcx().hir.body_owner(body_id); + let span = hir.tcx().hir.span(owner_id); let ty = hir.tcx().types.err; let mut builder = Builder::new(hir, span, 0, ty); let source_info = builder.source_info(span); @@ -472,6 +477,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { arg_count: usize, return_ty: Ty<'tcx>) -> Builder<'a, 'gcx, 'tcx> { + let lint_level = LintLevel::Explicit(hir.root_lint_level); let mut builder = Builder { hir, cfg: CFG { basic_blocks: IndexVec::new() }, @@ -480,6 +486,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { scopes: vec![], visibility_scopes: IndexVec::new(), visibility_scope: ARGUMENT_VISIBILITY_SCOPE, + visibility_scope_info: IndexVec::new(), breakable_scopes: vec![], local_decls: IndexVec::from_elem_n(LocalDecl::new_return_pointer(return_ty, span), 1), @@ -490,7 +497,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }; assert_eq!(builder.cfg.start_new_block(), START_BLOCK); - assert_eq!(builder.new_visibility_scope(span), ARGUMENT_VISIBILITY_SCOPE); + assert_eq!( + builder.new_visibility_scope(span, lint_level), + ARGUMENT_VISIBILITY_SCOPE); builder.visibility_scopes[ARGUMENT_VISIBILITY_SCOPE].parent_scope = None; builder @@ -509,6 +518,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { Mir::new(self.cfg.basic_blocks, self.visibility_scopes, + ClearOnDecode::Set(self.visibility_scope_info), IndexVec::new(), return_ty, yield_ty, @@ -543,6 +553,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { scope: ARGUMENT_VISIBILITY_SCOPE, span: pattern.map_or(self.fn_span, |pat| pat.span) }, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, name, internal: false, is_user_variable: false, @@ -557,7 +568,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if let Some(pattern) = pattern { let pattern = self.hir.pattern_from_hir(pattern); - scope = self.declare_bindings(scope, ast_body.span, &pattern); + scope = self.declare_bindings(scope, ast_body.span, + LintLevel::Inherited, &pattern); unpack!(block = self.lvalue_into_pattern(block, pattern, &lvalue)); } diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index bac884b4d01e9..62613c7168a30 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -88,8 +88,10 @@ should go to. */ use build::{BlockAnd, BlockAndExtension, Builder, CFG}; +use hair::LintLevel; use rustc::middle::region; use rustc::ty::{Ty, TyCtxt}; +use rustc::hir::def_id::LOCAL_CRATE; use rustc::mir::*; use rustc::mir::transform::MirSource; use syntax_pos::{Span}; @@ -304,15 +306,37 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// to build its contents, popping the scope afterwards. pub fn in_scope(&mut self, region_scope: (region::Scope, SourceInfo), + lint_level: LintLevel, mut block: BasicBlock, f: F) -> BlockAnd where F: FnOnce(&mut Builder<'a, 'gcx, 'tcx>) -> BlockAnd { debug!("in_scope(region_scope={:?}, block={:?})", region_scope, block); + let visibility_scope = self.visibility_scope; + let tcx = self.hir.tcx(); + if let LintLevel::Explicit(node_id) = lint_level { + let same_lint_scopes = tcx.dep_graph.with_ignore(|| { + let sets = tcx.lint_levels(LOCAL_CRATE); + let parent_hir_id = + tcx.hir.definitions().node_to_hir_id( + self.visibility_scope_info[visibility_scope].lint_root + ); + let current_hir_id = + tcx.hir.definitions().node_to_hir_id(node_id); + sets.lint_level_set(parent_hir_id) == + sets.lint_level_set(current_hir_id) + }); + + if !same_lint_scopes { + self.visibility_scope = + self.new_visibility_scope(region_scope.1.span, lint_level); + } + } self.push_scope(region_scope); let rv = unpack!(block = f(self)); unpack!(block = self.pop_scope(region_scope, block)); + self.visibility_scope = visibility_scope; debug!("in_scope: exiting region_scope={:?} block={:?}", region_scope, block); block.and(rv) } @@ -474,13 +498,21 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } /// Creates a new visibility scope, nested in the current one. - pub fn new_visibility_scope(&mut self, span: Span) -> VisibilityScope { + pub fn new_visibility_scope(&mut self, + span: Span, + lint_level: LintLevel) -> VisibilityScope { + debug!("new_visibility_scope({:?}, {:?})", span, lint_level); let parent = self.visibility_scope; - let scope = VisibilityScope::new(self.visibility_scopes.len()); - self.visibility_scopes.push(VisibilityScopeData { + let info = if let LintLevel::Explicit(lint_level) = lint_level { + VisibilityScopeInfo { lint_root: lint_level } + } else { + self.visibility_scope_info[parent].clone() + }; + let scope = self.visibility_scopes.push(VisibilityScopeData { span, parent_scope: Some(parent), }); + self.visibility_scope_info.push(info); scope } diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index f6b847d6d6de5..1c1fc2dcafa52 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -71,6 +71,7 @@ fn mirror_stmts<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, init_scope: region::Scope::Node(hir_id.local_id), pattern, initializer: local.init.to_ref(), + lint_level: cx.lint_level_of(local.id), }, opt_destruction_scope: opt_dxn_ext, }))); diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 45449103c8083..f5a53e2aa8eed 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -48,22 +48,24 @@ impl<'tcx> Mirror<'tcx> for &'tcx hir::Expr { kind: ExprKind::Scope { region_scope: expr_scope, value: expr.to_ref(), + lint_level: cx.lint_level_of(self.id), }, }; // Finally, create a destruction scope, if any. if let Some(region_scope) = - cx.region_scope_tree.opt_destruction_scope(self.hir_id.local_id) { - expr = Expr { - temp_lifetime, - ty: expr.ty, - span: self.span, - kind: ExprKind::Scope { - region_scope, - value: expr.to_ref(), - }, - }; - } + cx.region_scope_tree.opt_destruction_scope(self.hir_id.local_id) { + expr = Expr { + temp_lifetime, + ty: expr.ty, + span: self.span, + kind: ExprKind::Scope { + region_scope, + value: expr.to_ref(), + lint_level: LintLevel::Inherited, + }, + }; + } // OK, all done! expr @@ -619,6 +621,8 @@ fn convert_arm<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, arm: &'tcx hir::Arm) patterns: arm.pats.iter().map(|p| cx.pattern_from_hir(p)).collect(), guard: arm.guard.to_ref(), body: arm.body.to_ref(), + // BUG: fix this + lint_level: LintLevel::Inherited, } } diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index f5e15979006af..4434df0ac3e9b 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -20,13 +20,14 @@ use rustc::mir::transform::MirSource; use rustc::middle::const_val::{ConstEvalErr, ConstVal}; use rustc_const_eval::ConstContext; use rustc_data_structures::indexed_vec::Idx; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::hir::map::blocks::FnLikeNode; use rustc::middle::region; use rustc::infer::InferCtxt; use rustc::ty::subst::Subst; use rustc::ty::{self, Ty, TyCtxt}; use rustc::ty::subst::Substs; +use syntax::ast; use syntax::symbol::Symbol; use rustc::hir; use rustc_const_math::{ConstInt, ConstUsize}; @@ -37,6 +38,7 @@ pub struct Cx<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { tcx: TyCtxt<'a, 'gcx, 'tcx>, infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, + pub root_lint_level: ast::NodeId, pub param_env: ty::ParamEnv<'gcx>, /// Identity `Substs` for use with const-evaluation. @@ -57,7 +59,8 @@ pub struct Cx<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { } impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { - pub fn new(infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, src: MirSource) -> Cx<'a, 'gcx, 'tcx> { + pub fn new(infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, + src: MirSource) -> Cx<'a, 'gcx, 'tcx> { let constness = match src { MirSource::Const(_) | MirSource::Static(..) => hir::Constness::Const, @@ -87,9 +90,11 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { // Constants and const fn's always need overflow checks. check_overflow |= constness == hir::Constness::Const; + let lint_level = lint_level_for_hir_id(tcx, src_id); Cx { tcx, infcx, + root_lint_level: lint_level, param_env: tcx.param_env(src_def_id), identity_substs: Substs::identity_for_item(tcx.global_tcx(), src_def_id), region_scope_tree: tcx.region_scope_tree(src_def_id), @@ -99,6 +104,7 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { check_overflow, } } + } impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { @@ -229,6 +235,19 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { ty.needs_drop(self.tcx.global_tcx(), param_env) } + fn lint_level_of(&self, node_id: ast::NodeId) -> LintLevel { + let hir_id = self.tcx.hir.definitions().node_to_hir_id(node_id); + let has_lint_level = self.tcx.dep_graph.with_ignore(|| { + self.tcx.lint_levels(LOCAL_CRATE).lint_level_set(hir_id).is_some() + }); + + if has_lint_level { + LintLevel::Explicit(node_id) + } else { + LintLevel::Inherited + } + } + pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> { self.tcx } @@ -242,6 +261,31 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { } } +fn lint_level_for_hir_id(tcx: TyCtxt, mut id: ast::NodeId) -> ast::NodeId { + // Right now we insert a `with_ignore` node in the dep graph here to + // ignore the fact that `lint_levels` below depends on the entire crate. + // For now this'll prevent false positives of recompiling too much when + // anything changes. + // + // Once red/green incremental compilation lands we should be able to + // remove this because while the crate changes often the lint level map + // will change rarely. + tcx.dep_graph.with_ignore(|| { + let sets = tcx.lint_levels(LOCAL_CRATE); + loop { + let hir_id = tcx.hir.definitions().node_to_hir_id(id); + if sets.lint_level_set(hir_id).is_some() { + return id + } + let next = tcx.hir.get_parent_node(id); + if next == id { + bug!("lint traversal reached the root of the crate"); + } + id = next; + } + }) +} + mod block; mod expr; mod to_ref; diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 3162242de66c5..0c8dba5159f4b 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -29,6 +29,21 @@ pub mod cx; pub use rustc_const_eval::pattern::{BindingMode, Pattern, PatternKind, FieldPattern}; +#[derive(Copy, Clone, Debug)] +pub enum LintLevel { + Inherited, + Explicit(ast::NodeId) +} + +impl LintLevel { + pub fn is_explicit(self) -> bool { + match self { + LintLevel::Inherited => false, + LintLevel::Explicit(_) => true + } + } +} + #[derive(Clone, Debug)] pub struct Block<'tcx> { pub targeted_by_break: bool, @@ -73,7 +88,10 @@ pub enum StmtKind<'tcx> { pattern: Pattern<'tcx>, /// let pat = ... - initializer: Option> + initializer: Option>, + + /// the lint level for this let-statement + lint_level: LintLevel, }, } @@ -111,6 +129,7 @@ pub struct Expr<'tcx> { pub enum ExprKind<'tcx> { Scope { region_scope: region::Scope, + lint_level: LintLevel, value: ExprRef<'tcx>, }, Box { @@ -275,6 +294,7 @@ pub struct Arm<'tcx> { pub patterns: Vec>, pub guard: Option>, pub body: ExprRef<'tcx>, + pub lint_level: LintLevel, } #[derive(Copy, Clone, Debug)] diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 31480457723fd..a3a986918a4fd 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -140,6 +140,7 @@ fn temp_decl(mutability: Mutability, ty: Ty, span: Span) -> LocalDecl { LocalDecl { mutability, ty, name: None, source_info: SourceInfo { scope: ARGUMENT_VISIBILITY_SCOPE, span }, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: false, is_user_variable: false } @@ -195,6 +196,7 @@ fn build_drop_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, IndexVec::from_elem_n( VisibilityScopeData { span: span, parent_scope: None }, 1 ), + ClearOnDecode::Clear, IndexVec::new(), sig.output(), None, @@ -342,6 +344,7 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> { IndexVec::from_elem_n( VisibilityScopeData { span: self.span, parent_scope: None }, 1 ), + ClearOnDecode::Clear, IndexVec::new(), self.sig.output(), None, @@ -804,6 +807,7 @@ fn build_call_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, IndexVec::from_elem_n( VisibilityScopeData { span: span, parent_scope: None }, 1 ), + ClearOnDecode::Clear, IndexVec::new(), sig.output(), None, @@ -876,6 +880,7 @@ pub fn build_adt_ctor<'a, 'gcx, 'tcx>(infcx: &infer::InferCtxt<'a, 'gcx, 'tcx>, IndexVec::from_elem_n( VisibilityScopeData { span: span, parent_scope: None }, 1 ), + ClearOnDecode::Clear, IndexVec::new(), sig.output(), None, diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 507a42970c10a..729fe46ef37ec 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -301,6 +301,7 @@ fn replace_result_variable<'tcx>(ret_ty: Ty<'tcx>, ty: ret_ty, name: None, source_info: source_info(mir), + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: false, is_user_variable: false, }; @@ -559,6 +560,7 @@ fn create_generator_drop_shim<'a, 'tcx>( ty: tcx.mk_nil(), name: None, source_info, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: false, is_user_variable: false, }; @@ -574,6 +576,7 @@ fn create_generator_drop_shim<'a, 'tcx>( }), name: None, source_info, + lexical_scope: ARGUMENT_VISIBILITY_SCOPE, internal: false, is_user_variable: false, }; diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index ca6eda5c2d716..339ea8a414b1e 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -380,10 +380,10 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>, let mut promoter = Promoter { promoted: Mir::new( IndexVec::new(), - Some(VisibilityScopeData { - span, - parent_scope: None - }).into_iter().collect(), + // FIXME: maybe try to filter this to avoid blowing up + // memory usage? + mir.visibility_scopes.clone(), + mir.visibility_scope_info.clone(), IndexVec::new(), ty, None, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index edbdfc1a7d4bc..c6461217ac321 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2236,7 +2236,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { adjusted_ty, index_ty); - // First, try built-in indexing. match (adjusted_ty.builtin_index(), &index_ty.sty) { (Some(ty), &ty::TyUint(ast::UintTy::Us)) | (Some(ty), &ty::TyInfer(ty::IntVar(_))) => { From c72a979979fef0cc00c3023cd4a4ce550a22935b Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 19 Sep 2017 16:20:02 +0300 Subject: [PATCH 2/5] move unsafety checking to MIR No functional changes intended. --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/diagnostics.rs | 34 -- src/librustc/ich/impls_mir.rs | 22 +- src/librustc/lib.rs | 1 - src/librustc/middle/effect.rs | 316 -------------- src/librustc/mir/mod.rs | 27 +- src/librustc/ty/maps/mod.rs | 4 + src/librustc_driver/driver.rs | 9 +- src/librustc_mir/build/block.rs | 65 ++- src/librustc_mir/build/expr/into.rs | 19 +- src/librustc_mir/build/matches/mod.rs | 6 +- src/librustc_mir/build/mod.rs | 31 +- src/librustc_mir/build/scope.rs | 27 +- src/librustc_mir/diagnostics.rs | 34 ++ src/librustc_mir/hair/cx/block.rs | 10 + src/librustc_mir/hair/mod.rs | 9 + src/librustc_mir/transform/check_unsafety.rs | 387 ++++++++++++++++++ src/librustc_mir/transform/mod.rs | 3 + src/librustc_mir/transform/type_check.rs | 10 + .../borrowck/borrowck-move-from-unsafe-ptr.rs | 4 +- src/test/compile-fail/issue-43733.rs | 4 +- src/test/compile-fail/union/union-unsafe.rs | 4 +- .../unsafe-fn-assign-deref-ptr.rs | 3 +- src/test/compile-fail/unsafe-move-val-init.rs | 20 + 24 files changed, 662 insertions(+), 388 deletions(-) delete mode 100644 src/librustc/middle/effect.rs create mode 100644 src/librustc_mir/transform/check_unsafety.rs create mode 100644 src/test/compile-fail/unsafe-move-val-init.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index b492caf10bb0d..7a78765365db0 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -445,6 +445,7 @@ define_dep_nodes!( <'tcx> [] BorrowCheckKrate, [] BorrowCheck(DefId), [] MirBorrowCheck(DefId), + [] UnsafetyViolations(DefId), [] RvalueCheck(DefId), [] Reachability, diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 6e0f49bba90ff..6b79f0cde1a8f 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -479,40 +479,6 @@ fn main() { ``` "##, -E0133: r##" -Unsafe code was used outside of an unsafe function or block. - -Erroneous code example: - -```compile_fail,E0133 -unsafe fn f() { return; } // This is the unsafe code - -fn main() { - f(); // error: call to unsafe function requires unsafe function or block -} -``` - -Using unsafe functionality is potentially dangerous and disallowed by safety -checks. Examples: - -* Dereferencing raw pointers -* Calling functions via FFI -* Calling functions marked unsafe - -These safety checks can be relaxed for a section of the code by wrapping the -unsafe instructions with an `unsafe` block. For instance: - -``` -unsafe fn f() { return; } - -fn main() { - unsafe { f(); } // ok! -} -``` - -See also https://doc.rust-lang.org/book/first-edition/unsafe.html -"##, - // This shouldn't really ever trigger since the repeated value error comes first E0136: r##" A binary can only have one entry point, and by default that entry point is the diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index 068830d688c3e..4bda89690b7a9 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -33,6 +33,7 @@ impl_stable_hash_for!(struct mir::LocalDecl<'tcx> { }); impl_stable_hash_for!(struct mir::UpvarDecl { debug_name, by_ref }); impl_stable_hash_for!(struct mir::BasicBlockData<'tcx> { statements, terminator, is_cleanup }); +impl_stable_hash_for!(struct mir::UnsafetyViolation { source_info, description, lint_node_id }); impl<'gcx> HashStable> for mir::Terminator<'gcx> { @@ -364,7 +365,26 @@ for mir::ProjectionElem<'gcx, V, T> } impl_stable_hash_for!(struct mir::VisibilityScopeData { span, parent_scope }); -impl_stable_hash_for!(struct mir::VisibilityScopeInfo { lint_root }); +impl_stable_hash_for!(struct mir::VisibilityScopeInfo { + lint_root, safety +}); + +impl<'gcx> HashStable> for mir::Safety { + fn hash_stable(&self, + hcx: &mut StableHashingContext<'gcx>, + hasher: &mut StableHasher) { + mem::discriminant(self).hash_stable(hcx, hasher); + + match *self { + mir::Safety::Safe | + mir::Safety::BuiltinUnsafe | + mir::Safety::FnUnsafe => {} + mir::Safety::ExplicitUnsafe(node_id) => { + node_id.hash_stable(hcx, hasher); + } + } + } +} impl<'gcx> HashStable> for mir::Operand<'gcx> { fn hash_stable(&self, diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index cd39ef7094632..1e90aa47267ff 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -111,7 +111,6 @@ pub mod middle { pub mod dataflow; pub mod dead; pub mod dependency_format; - pub mod effect; pub mod entry; pub mod exported_symbols; pub mod free_region; diff --git a/src/librustc/middle/effect.rs b/src/librustc/middle/effect.rs deleted file mode 100644 index 7290353e48b0c..0000000000000 --- a/src/librustc/middle/effect.rs +++ /dev/null @@ -1,316 +0,0 @@ -// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution and at -// http://rust-lang.org/COPYRIGHT. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -//! Enforces the Rust effect system. Currently there is just one effect, -//! `unsafe`. -use self::RootUnsafeContext::*; - -use ty::{self, TyCtxt}; -use lint; -use lint::builtin::UNUSED_UNSAFE; - -use hir::def::Def; -use hir::intravisit::{self, FnKind, Visitor, NestedVisitorMap}; -use hir::{self, PatKind}; -use syntax::ast; -use syntax_pos::Span; -use util::nodemap::FxHashSet; - -#[derive(Copy, Clone)] -struct UnsafeContext { - push_unsafe_count: usize, - root: RootUnsafeContext, -} - -impl UnsafeContext { - fn new(root: RootUnsafeContext) -> UnsafeContext { - UnsafeContext { root: root, push_unsafe_count: 0 } - } -} - -#[derive(Copy, Clone, PartialEq)] -enum RootUnsafeContext { - SafeContext, - UnsafeFn, - UnsafeBlock(ast::NodeId), -} - -struct EffectCheckVisitor<'a, 'tcx: 'a> { - tcx: TyCtxt<'a, 'tcx, 'tcx>, - tables: &'a ty::TypeckTables<'tcx>, - body_id: hir::BodyId, - - /// Whether we're in an unsafe context. - unsafe_context: UnsafeContext, - used_unsafe: FxHashSet, -} - -impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> { - fn require_unsafe_ext(&mut self, node_id: ast::NodeId, span: Span, - description: &str, is_lint: bool) { - if self.unsafe_context.push_unsafe_count > 0 { return; } - match self.unsafe_context.root { - SafeContext => { - if is_lint { - self.tcx.lint_node(lint::builtin::SAFE_EXTERN_STATICS, - node_id, - span, - &format!("{} requires unsafe function or \ - block (error E0133)", description)); - } else { - // Report an error. - struct_span_err!( - self.tcx.sess, span, E0133, - "{} requires unsafe function or block", description) - .span_label(span, description) - .emit(); - } - } - UnsafeBlock(block_id) => { - // OK, but record this. - debug!("effect: recording unsafe block as used: {}", block_id); - self.used_unsafe.insert(block_id); - } - UnsafeFn => {} - } - } - - fn require_unsafe(&mut self, span: Span, description: &str) { - self.require_unsafe_ext(ast::DUMMY_NODE_ID, span, description, false) - } -} - -impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> { - fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { - NestedVisitorMap::None - } - - fn visit_nested_body(&mut self, body: hir::BodyId) { - let old_tables = self.tables; - let old_body_id = self.body_id; - self.tables = self.tcx.body_tables(body); - self.body_id = body; - let body = self.tcx.hir.body(body); - self.visit_body(body); - self.tables = old_tables; - self.body_id = old_body_id; - } - - fn visit_fn(&mut self, fn_kind: FnKind<'tcx>, fn_decl: &'tcx hir::FnDecl, - body_id: hir::BodyId, span: Span, id: ast::NodeId) { - - let (is_item_fn, is_unsafe_fn) = match fn_kind { - FnKind::ItemFn(_, _, unsafety, ..) => - (true, unsafety == hir::Unsafety::Unsafe), - FnKind::Method(_, sig, ..) => - (true, sig.unsafety == hir::Unsafety::Unsafe), - _ => (false, false), - }; - - let old_unsafe_context = self.unsafe_context; - if is_unsafe_fn { - self.unsafe_context = UnsafeContext::new(UnsafeFn) - } else if is_item_fn { - self.unsafe_context = UnsafeContext::new(SafeContext) - } - - intravisit::walk_fn(self, fn_kind, fn_decl, body_id, span, id); - - self.unsafe_context = old_unsafe_context - } - - fn visit_block(&mut self, block: &'tcx hir::Block) { - let old_unsafe_context = self.unsafe_context; - match block.rules { - hir::UnsafeBlock(source) => { - // By default only the outermost `unsafe` block is - // "used" and so nested unsafe blocks are pointless - // (the inner ones are unnecessary and we actually - // warn about them). As such, there are two cases when - // we need to create a new context, when we're - // - outside `unsafe` and found a `unsafe` block - // (normal case) - // - inside `unsafe`, found an `unsafe` block - // created internally to the compiler - // - // The second case is necessary to ensure that the - // compiler `unsafe` blocks don't accidentally "use" - // external blocks (e.g. `unsafe { println("") }`, - // expands to `unsafe { ... unsafe { ... } }` where - // the inner one is compiler generated). - if self.unsafe_context.root == SafeContext || source == hir::CompilerGenerated { - self.unsafe_context.root = UnsafeBlock(block.id) - } - } - hir::PushUnsafeBlock(..) => { - self.unsafe_context.push_unsafe_count = - self.unsafe_context.push_unsafe_count.checked_add(1).unwrap(); - } - hir::PopUnsafeBlock(..) => { - self.unsafe_context.push_unsafe_count = - self.unsafe_context.push_unsafe_count.checked_sub(1).unwrap(); - } - hir::DefaultBlock => {} - } - - intravisit::walk_block(self, block); - - self.unsafe_context = old_unsafe_context; - - // Don't warn about generated blocks, that'll just pollute the output. - match block.rules { - hir::UnsafeBlock(hir::UserProvided) => {} - _ => return, - } - if self.used_unsafe.contains(&block.id) { - return - } - - /// Return the NodeId for an enclosing scope that is also `unsafe` - fn is_enclosed(tcx: TyCtxt, - used_unsafe: &FxHashSet, - id: ast::NodeId) -> Option<(String, ast::NodeId)> { - let parent_id = tcx.hir.get_parent_node(id); - if parent_id != id { - if used_unsafe.contains(&parent_id) { - Some(("block".to_string(), parent_id)) - } else if let Some(hir::map::NodeItem(&hir::Item { - node: hir::ItemFn(_, hir::Unsafety::Unsafe, _, _, _, _), - .. - })) = tcx.hir.find(parent_id) { - Some(("fn".to_string(), parent_id)) - } else { - is_enclosed(tcx, used_unsafe, parent_id) - } - } else { - None - } - } - - let mut db = self.tcx.struct_span_lint_node(UNUSED_UNSAFE, - block.id, - block.span, - "unnecessary `unsafe` block"); - db.span_label(block.span, "unnecessary `unsafe` block"); - if let Some((kind, id)) = is_enclosed(self.tcx, &self.used_unsafe, block.id) { - db.span_note(self.tcx.hir.span(id), - &format!("because it's nested under this `unsafe` {}", kind)); - } - db.emit(); - } - - fn visit_expr(&mut self, expr: &'tcx hir::Expr) { - match expr.node { - hir::ExprMethodCall(..) => { - let def_id = self.tables.type_dependent_defs()[expr.hir_id].def_id(); - let sig = self.tcx.fn_sig(def_id); - debug!("effect: method call case, signature is {:?}", - sig); - - if sig.0.unsafety == hir::Unsafety::Unsafe { - self.require_unsafe(expr.span, - "invocation of unsafe method") - } - } - hir::ExprCall(ref base, _) => { - let base_type = self.tables.expr_ty_adjusted(base); - debug!("effect: call case, base type is {:?}", - base_type); - match base_type.sty { - ty::TyFnDef(..) | ty::TyFnPtr(_) => { - if base_type.fn_sig(self.tcx).unsafety() == hir::Unsafety::Unsafe { - self.require_unsafe(expr.span, "call to unsafe function") - } - } - _ => {} - } - } - hir::ExprUnary(hir::UnDeref, ref base) => { - let base_type = self.tables.expr_ty_adjusted(base); - debug!("effect: unary case, base type is {:?}", - base_type); - if let ty::TyRawPtr(_) = base_type.sty { - self.require_unsafe(expr.span, "dereference of raw pointer") - } - } - hir::ExprInlineAsm(..) => { - self.require_unsafe(expr.span, "use of inline assembly"); - } - hir::ExprPath(hir::QPath::Resolved(_, ref path)) => { - if let Def::Static(def_id, mutbl) = path.def { - if mutbl { - self.require_unsafe(expr.span, "use of mutable static"); - } else if match self.tcx.hir.get_if_local(def_id) { - Some(hir::map::NodeForeignItem(..)) => true, - Some(..) => false, - None => self.tcx.is_foreign_item(def_id), - } { - self.require_unsafe_ext(expr.id, expr.span, "use of extern static", true); - } - } - } - hir::ExprField(ref base_expr, field) => { - if let ty::TyAdt(adt, ..) = self.tables.expr_ty_adjusted(base_expr).sty { - if adt.is_union() { - self.require_unsafe(field.span, "access to union field"); - } - } - } - hir::ExprAssign(ref lhs, ref rhs) => { - if let hir::ExprField(ref base_expr, field) = lhs.node { - if let ty::TyAdt(adt, ..) = self.tables.expr_ty_adjusted(base_expr).sty { - if adt.is_union() { - let field_ty = self.tables.expr_ty_adjusted(lhs); - let owner_def_id = self.tcx.hir.body_owner_def_id(self.body_id); - let param_env = self.tcx.param_env(owner_def_id); - if field_ty.moves_by_default(self.tcx, param_env, field.span) { - self.require_unsafe(field.span, - "assignment to non-`Copy` union field"); - } - // Do not walk the field expr again. - intravisit::walk_expr(self, base_expr); - intravisit::walk_expr(self, rhs); - return - } - } - } - } - _ => {} - } - - intravisit::walk_expr(self, expr); - } - - fn visit_pat(&mut self, pat: &'tcx hir::Pat) { - if let PatKind::Struct(_, ref fields, _) = pat.node { - if let ty::TyAdt(adt, ..) = self.tables.pat_ty(pat).sty { - if adt.is_union() { - for field in fields { - self.require_unsafe(field.span, "matching on union field"); - } - } - } - } - - intravisit::walk_pat(self, pat); - } -} - -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - let mut visitor = EffectCheckVisitor { - tcx, - tables: &ty::TypeckTables::empty(None), - body_id: hir::BodyId { node_id: ast::CRATE_NODE_ID }, - unsafe_context: UnsafeContext::new(SafeContext), - used_unsafe: FxHashSet(), - }; - - tcx.hir.krate().visit_all_item_likes(&mut visitor.as_deep_visitor()); -} diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 09da1d1f820bc..a5fe482845367 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -290,6 +290,19 @@ impl<'tcx> Mir<'tcx> { pub struct VisibilityScopeInfo { /// A NodeId with lint levels equivalent to this scope's lint levels. pub lint_root: ast::NodeId, + /// The unsafe block that contains this node. + pub safety: Safety, +} + +#[derive(Copy, Clone, Debug)] +pub enum Safety { + Safe, + /// Unsafe because of a PushUnsafeBlock + BuiltinUnsafe, + /// Unsafe because of an unsafe fn + FnUnsafe, + /// Unsafe because of an `unsafe` block + ExplicitUnsafe(ast::NodeId) } impl_stable_hash_for!(struct Mir<'tcx> { @@ -346,7 +359,7 @@ impl serialize::Decodable for ClearOnDecode { /// Grouped information about the source code origin of a MIR entity. /// Intended to be inspected by diagnostics and debuginfo. /// Most passes can work with it as a whole, within a single function. -#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable)] +#[derive(Copy, Clone, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash)] pub struct SourceInfo { /// Source span for the AST pertaining to this MIR entity. pub span: Span, @@ -447,7 +460,7 @@ pub struct LocalDecl<'tcx> { /// True if this is an internal local /// /// These locals are not based on types in the source code and are only used - /// for drop flags at the moment. + /// for a few desugarings at the moment. /// /// The generator transformation will sanity check the locals which are live /// across a suspension point against the type components of the generator @@ -455,6 +468,9 @@ pub struct LocalDecl<'tcx> { /// flag drop flags to avoid triggering this check as they are introduced /// after typeck. /// + /// Unsafety checking will also ignore dereferences of these locals, + /// so they can be used for raw pointers only used in a desugaring. + /// /// This should be sound because the drop flags are fully algebraic, and /// therefore don't affect the OIBIT or outlives properties of the /// generator. @@ -1634,6 +1650,13 @@ impl Location { } } +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub struct UnsafetyViolation { + pub source_info: SourceInfo, + pub description: &'static str, + pub lint_node_id: Option, +} + /// The layout of generator state #[derive(Clone, Debug, RustcEncodable, RustcDecodable)] pub struct GeneratorLayout<'tcx> { diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs index d264fecb52158..4bd2d5be6d716 100644 --- a/src/librustc/ty/maps/mod.rs +++ b/src/librustc/ty/maps/mod.rs @@ -159,6 +159,10 @@ define_maps! { <'tcx> /// expression defining the closure. [] fn closure_kind: ClosureKind(DefId) -> ty::ClosureKind, + /// Unsafety violations for this def ID. + [] fn unsafety_violations: UnsafetyViolations(DefId) + -> Rc<[mir::UnsafetyViolation]>, + /// The signature of functions and closures. [] fn fn_sig: FnSignature(DefId) -> ty::PolyFnSig<'tcx>, diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 5116a3087d281..4725f6e2c7839 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1081,10 +1081,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, "intrinsic checking", || middle::intrinsicck::check_crate(tcx)); - time(time_passes, - "effect checking", - || middle::effect::check_crate(tcx)); - time(time_passes, "match checking", || check_match::check_crate(tcx)); @@ -1105,6 +1101,11 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, "MIR borrow checking", || for def_id in tcx.body_owners() { tcx.mir_borrowck(def_id) }); + time(time_passes, + "MIR effect checking", + || for def_id in tcx.body_owners() { + mir::transform::check_unsafety::check_unsafety(tcx, def_id) + }); // Avoid overwhelming user with errors if type checking failed. // I'm not sure how helpful this is, to be honest, but it avoids // a diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index db04596dd589f..1fc96dbf45197 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -21,7 +21,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ast_block: &'tcx hir::Block, source_info: SourceInfo) -> BlockAnd<()> { - let Block { region_scope, opt_destruction_scope, span, stmts, expr, targeted_by_break } = + let Block { + region_scope, + opt_destruction_scope, + span, + stmts, + expr, + targeted_by_break, + safety_mode + } = self.hir.mirror(ast_block); self.in_opt_scope(opt_destruction_scope.map(|de|(de, source_info)), block, move |this| { this.in_scope((region_scope, source_info), LintLevel::Inherited, block, move |this| { @@ -30,13 +38,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let exit_block = this.cfg.start_new_block(); let block_exit = this.in_breakable_scope( None, exit_block, destination.clone(), |this| { - this.ast_block_stmts(destination, block, span, stmts, expr) + this.ast_block_stmts(destination, block, span, stmts, expr, + safety_mode) }); this.cfg.terminate(unpack!(block_exit), source_info, TerminatorKind::Goto { target: exit_block }); exit_block.unit() } else { - this.ast_block_stmts(destination, block, span, stmts, expr) + this.ast_block_stmts(destination, block, span, stmts, expr, + safety_mode) } }) }) @@ -47,7 +57,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { mut block: BasicBlock, span: Span, stmts: Vec>, - expr: Option>) + expr: Option>, + safety_mode: BlockSafety) -> BlockAnd<()> { let this = self; @@ -69,6 +80,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // First we build all the statements in the block. let mut let_scope_stack = Vec::with_capacity(8); let outer_visibility_scope = this.visibility_scope; + let outer_push_unsafe_count = this.push_unsafe_count; + let outer_unpushed_unsafe = this.unpushed_unsafe; + this.update_visibility_scope_for_safety_mode(span, safety_mode); + let source_info = this.source_info(span); for stmt in stmts { let Stmt { kind, opt_destruction_scope } = this.hir.mirror(stmt); @@ -137,6 +152,48 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } // Restore the original visibility scope. this.visibility_scope = outer_visibility_scope; + this.push_unsafe_count = outer_push_unsafe_count; + this.unpushed_unsafe = outer_unpushed_unsafe; block.unit() } + + /// If we are changing the safety mode, create a new visibility scope + fn update_visibility_scope_for_safety_mode(&mut self, + span: Span, + safety_mode: BlockSafety) + { + debug!("update_visibility_scope_for({:?}, {:?})", span, safety_mode); + let new_unsafety = match safety_mode { + BlockSafety::Safe => None, + BlockSafety::ExplicitUnsafe(node_id) => { + assert_eq!(self.push_unsafe_count, 0); + match self.unpushed_unsafe { + Safety::Safe => {} + _ => return + } + self.unpushed_unsafe = Safety::ExplicitUnsafe(node_id); + Some(Safety::ExplicitUnsafe(node_id)) + } + BlockSafety::PushUnsafe => { + self.push_unsafe_count += 1; + Some(Safety::BuiltinUnsafe) + } + BlockSafety::PopUnsafe => { + self.push_unsafe_count = + self.push_unsafe_count.checked_sub(1).unwrap_or_else(|| { + span_bug!(span, "unsafe count underflow") + }); + if self.push_unsafe_count == 0 { + Some(self.unpushed_unsafe) + } else { + None + } + } + }; + + if let Some(unsafety) = new_unsafety { + self.visibility_scope = self.new_visibility_scope( + span, LintLevel::Inherited, Some(unsafety)); + } + } } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 65e16657a031a..cdbcb43370fe0 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -228,9 +228,22 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let val = args.next().expect("1 argument to `move_val_init`"); assert!(args.next().is_none(), ">2 arguments to `move_val_init`"); - let topmost_scope = this.topmost_scope(); - let ptr = unpack!(block = this.as_temp(block, Some(topmost_scope), ptr)); - this.into(&Lvalue::Local(ptr).deref(), block, val) + let ptr = this.hir.mirror(ptr); + let ptr_ty = ptr.ty; + // Create an *internal* temp for the pointer, so that unsafety + // checking won't complain about the raw pointer assignment. + let ptr_temp = this.local_decls.push(LocalDecl { + mutability: Mutability::Mut, + ty: ptr_ty, + name: None, + source_info, + lexical_scope: source_info.scope, + internal: true, + is_user_variable: false + }); + let ptr_temp = Lvalue::Local(ptr_temp); + let block = unpack!(this.into(&ptr_temp, block, ptr)); + this.into(&ptr_temp.deref(), block, val) } else { let args: Vec<_> = args.into_iter() diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index b856e6345dc6b..f04dede6e4005 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -182,11 +182,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { self.visit_bindings(pattern, &mut |this, mutability, name, var, span, ty| { if var_scope.is_none() { var_scope = Some(this.new_visibility_scope(scope_span, - LintLevel::Inherited)); + LintLevel::Inherited, + None)); // If we have lints, create a new visibility scope // that marks the lints for the locals. if lint_level.is_explicit() { - this.new_visibility_scope(scope_span, lint_level); + this.visibility_scope = + this.new_visibility_scope(scope_span, lint_level, None); } } let source_info = SourceInfo { diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 9c2c5bbcdb8ba..68ef646184c2c 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -72,14 +72,14 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'t // is a constant "initializer" expression. match expr.node { hir::ExprClosure(_, _, body, _, _) => body, - _ => hir::BodyId { node_id: expr.id } + _ => hir::BodyId { node_id: expr.id }, } } hir::map::NodeVariant(variant) => return create_constructor_shim(tcx, id, &variant.node.data), hir::map::NodeStructCtor(ctor) => return create_constructor_shim(tcx, id, ctor), - _ => unsupported() + _ => unsupported(), }; let src = MirSource::from_node(tcx, id); @@ -109,6 +109,12 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'t _ => None, }; + // FIXME: safety in closures + let safety = match fn_sig.unsafety { + hir::Unsafety::Normal => Safety::Safe, + hir::Unsafety::Unsafe => Safety::FnUnsafe, + }; + let body = tcx.hir.body(body_id); let explicit_arguments = body.arguments @@ -127,7 +133,8 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'t (None, fn_sig.output()) }; - build::construct_fn(cx, id, arguments, abi, return_ty, yield_ty, body) + build::construct_fn(cx, id, arguments, safety, abi, + return_ty, yield_ty, body) } else { build::construct_const(cx, body_id) }; @@ -271,6 +278,13 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { /// see the `scope` module for more details scopes: Vec>, + /// The current unsafe block in scope, even if it is hidden by + /// a PushUnsafeBlock + unpushed_unsafe: Safety, + + /// The number of `push_unsafe_block` levels in scope + push_unsafe_count: usize, + /// the current set of breakables; see the `scope` module for more /// details breakable_scopes: Vec>, @@ -360,6 +374,7 @@ macro_rules! unpack { fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>, fn_id: ast::NodeId, arguments: A, + safety: Safety, abi: Abi, return_ty: Ty<'gcx>, yield_ty: Option>, @@ -374,6 +389,7 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>, let mut builder = Builder::new(hir.clone(), span, arguments.len(), + safety, return_ty); let call_site_scope = region::Scope::CallSite(body.value.hir_id.local_id); @@ -444,7 +460,7 @@ fn construct_const<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>, let ty = hir.tables().expr_ty_adjusted(ast_expr); let owner_id = tcx.hir.body_owner(body_id); let span = tcx.hir.span(owner_id); - let mut builder = Builder::new(hir.clone(), span, 0, ty); + let mut builder = Builder::new(hir.clone(), span, 0, Safety::Safe, ty); let mut block = START_BLOCK; let expr = builder.hir.mirror(ast_expr); @@ -465,7 +481,7 @@ fn construct_error<'a, 'gcx, 'tcx>(hir: Cx<'a, 'gcx, 'tcx>, let owner_id = hir.tcx().hir.body_owner(body_id); let span = hir.tcx().hir.span(owner_id); let ty = hir.tcx().types.err; - let mut builder = Builder::new(hir, span, 0, ty); + let mut builder = Builder::new(hir, span, 0, Safety::Safe, ty); let source_info = builder.source_info(span); builder.cfg.terminate(START_BLOCK, source_info, TerminatorKind::Unreachable); builder.finish(vec![], ty, None) @@ -475,6 +491,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { fn new(hir: Cx<'a, 'gcx, 'tcx>, span: Span, arg_count: usize, + safety: Safety, return_ty: Ty<'tcx>) -> Builder<'a, 'gcx, 'tcx> { let lint_level = LintLevel::Explicit(hir.root_lint_level); @@ -487,6 +504,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { visibility_scopes: IndexVec::new(), visibility_scope: ARGUMENT_VISIBILITY_SCOPE, visibility_scope_info: IndexVec::new(), + push_unsafe_count: 0, + unpushed_unsafe: safety, breakable_scopes: vec![], local_decls: IndexVec::from_elem_n(LocalDecl::new_return_pointer(return_ty, span), 1), @@ -498,7 +517,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { assert_eq!(builder.cfg.start_new_block(), START_BLOCK); assert_eq!( - builder.new_visibility_scope(span, lint_level), + builder.new_visibility_scope(span, lint_level, Some(safety)), ARGUMENT_VISIBILITY_SCOPE); builder.visibility_scopes[ARGUMENT_VISIBILITY_SCOPE].parent_scope = None; diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 62613c7168a30..20d3efdfffc3d 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -330,7 +330,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if !same_lint_scopes { self.visibility_scope = - self.new_visibility_scope(region_scope.1.span, lint_level); + self.new_visibility_scope(region_scope.1.span, lint_level, + None); } } self.push_scope(region_scope); @@ -500,19 +501,27 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { /// Creates a new visibility scope, nested in the current one. pub fn new_visibility_scope(&mut self, span: Span, - lint_level: LintLevel) -> VisibilityScope { - debug!("new_visibility_scope({:?}, {:?})", span, lint_level); + lint_level: LintLevel, + safety: Option) -> VisibilityScope { let parent = self.visibility_scope; - let info = if let LintLevel::Explicit(lint_level) = lint_level { - VisibilityScopeInfo { lint_root: lint_level } - } else { - self.visibility_scope_info[parent].clone() - }; + debug!("new_visibility_scope({:?}, {:?}, {:?}) - parent({:?})={:?}", + span, lint_level, safety, + parent, self.visibility_scope_info.get(parent)); let scope = self.visibility_scopes.push(VisibilityScopeData { span, parent_scope: Some(parent), }); - self.visibility_scope_info.push(info); + let scope_info = VisibilityScopeInfo { + lint_root: if let LintLevel::Explicit(lint_root) = lint_level { + lint_root + } else { + self.visibility_scope_info[parent].lint_root + }, + safety: safety.unwrap_or_else(|| { + self.visibility_scope_info[parent].safety + }) + }; + self.visibility_scope_info.push(scope_info); scope } diff --git a/src/librustc_mir/diagnostics.rs b/src/librustc_mir/diagnostics.rs index 26436d54ac886..2c4afb0aa0e04 100644 --- a/src/librustc_mir/diagnostics.rs +++ b/src/librustc_mir/diagnostics.rs @@ -195,6 +195,40 @@ instead of using a `const fn`, or refactoring the code to a functional style to avoid mutation if possible. "##, +E0133: r##" +Unsafe code was used outside of an unsafe function or block. + +Erroneous code example: + +```compile_fail,E0133 +unsafe fn f() { return; } // This is the unsafe code + +fn main() { + f(); // error: call to unsafe function requires unsafe function or block +} +``` + +Using unsafe functionality is potentially dangerous and disallowed by safety +checks. Examples: + +* Dereferencing raw pointers +* Calling functions via FFI +* Calling functions marked unsafe + +These safety checks can be relaxed for a section of the code by wrapping the +unsafe instructions with an `unsafe` block. For instance: + +``` +unsafe fn f() { return; } + +fn main() { + unsafe { f(); } // ok! +} +``` + +See also https://doc.rust-lang.org/book/first-edition/unsafe.html +"##, + E0381: r##" It is not allowed to use or capture an uninitialized variable. For example: diff --git a/src/librustc_mir/hair/cx/block.rs b/src/librustc_mir/hair/cx/block.rs index 1c1fc2dcafa52..a8172a60174fa 100644 --- a/src/librustc_mir/hair/cx/block.rs +++ b/src/librustc_mir/hair/cx/block.rs @@ -30,6 +30,16 @@ impl<'tcx> Mirror<'tcx> for &'tcx hir::Block { span: self.span, stmts, expr: self.expr.to_ref(), + safety_mode: match self.rules { + hir::BlockCheckMode::DefaultBlock => + BlockSafety::Safe, + hir::BlockCheckMode::UnsafeBlock(..) => + BlockSafety::ExplicitUnsafe(self.id), + hir::BlockCheckMode::PushUnsafeBlock(..) => + BlockSafety::PushUnsafe, + hir::BlockCheckMode::PopUnsafeBlock(..) => + BlockSafety::PopUnsafe + }, } } } diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 0c8dba5159f4b..09a31f9ab8fa5 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -52,6 +52,15 @@ pub struct Block<'tcx> { pub span: Span, pub stmts: Vec>, pub expr: Option>, + pub safety_mode: BlockSafety, +} + +#[derive(Copy, Clone, Debug)] +pub enum BlockSafety { + Safe, + ExplicitUnsafe(ast::NodeId), + PushUnsafe, + PopUnsafe } #[derive(Clone, Debug)] diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs new file mode 100644 index 0000000000000..49ce36223994b --- /dev/null +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -0,0 +1,387 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::indexed_vec::IndexVec; + +use rustc::ty::maps::Providers; +use rustc::ty::{self, TyCtxt}; +use rustc::hir; +use rustc::hir::def::Def; +use rustc::hir::def_id::DefId; +use rustc::hir::map::{DefPathData, Node}; +use rustc::lint::builtin::{SAFE_EXTERN_STATICS, UNUSED_UNSAFE}; +use rustc::mir::*; +use rustc::mir::visit::{LvalueContext, Visitor}; + +use syntax::ast; + +use std::rc::Rc; + + +pub struct UnsafetyChecker<'a, 'tcx: 'a> { + mir: &'a Mir<'tcx>, + visibility_scope_info: &'a IndexVec, + violations: Vec, + source_info: SourceInfo, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + param_env: ty::ParamEnv<'tcx>, + used_unsafe: FxHashSet, +} + +impl<'a, 'gcx, 'tcx> UnsafetyChecker<'a, 'tcx> { + fn new(mir: &'a Mir<'tcx>, + visibility_scope_info: &'a IndexVec, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + param_env: ty::ParamEnv<'tcx>) -> Self { + Self { + mir, + visibility_scope_info, + violations: vec![], + source_info: SourceInfo { + span: mir.span, + scope: ARGUMENT_VISIBILITY_SCOPE + }, + tcx, + param_env, + used_unsafe: FxHashSet(), + } + } +} + +impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { + fn visit_terminator(&mut self, + block: BasicBlock, + terminator: &Terminator<'tcx>, + location: Location) + { + self.source_info = terminator.source_info; + match terminator.kind { + TerminatorKind::Goto { .. } | + TerminatorKind::SwitchInt { .. } | + TerminatorKind::Drop { .. } | + TerminatorKind::Yield { .. } | + TerminatorKind::Assert { .. } | + TerminatorKind::DropAndReplace { .. } | + TerminatorKind::GeneratorDrop | + TerminatorKind::Resume | + TerminatorKind::Return | + TerminatorKind::Unreachable => { + // safe (at least as emitted during MIR construction) + } + + TerminatorKind::Call { ref func, .. } => { + let func_ty = func.ty(self.mir, self.tcx); + let sig = func_ty.fn_sig(self.tcx); + if let hir::Unsafety::Unsafe = sig.unsafety() { + self.require_unsafe("call to unsafe function") + } + } + } + self.super_terminator(block, terminator, location); + } + + fn visit_statement(&mut self, + block: BasicBlock, + statement: &Statement<'tcx>, + location: Location) + { + self.source_info = statement.source_info; + match statement.kind { + StatementKind::Assign(..) | + StatementKind::SetDiscriminant { .. } | + StatementKind::StorageLive(..) | + StatementKind::StorageDead(..) | + StatementKind::EndRegion(..) | + StatementKind::Validate(..) | + StatementKind::Nop => { + // safe (at least as emitted during MIR construction) + } + + StatementKind::InlineAsm { .. } => { + self.require_unsafe("use of inline assembly") + }, + } + self.super_statement(block, statement, location); + } + + fn visit_rvalue(&mut self, + rvalue: &Rvalue<'tcx>, + location: Location) + { + if let &Rvalue::Aggregate( + box AggregateKind::Closure(def_id, _), + _ + ) = rvalue { + let unsafety_violations = self.tcx.unsafety_violations(def_id); + self.register_violations(&unsafety_violations); + } + self.super_rvalue(rvalue, location); + } + + fn visit_lvalue(&mut self, + lvalue: &Lvalue<'tcx>, + context: LvalueContext<'tcx>, + location: Location) { + match lvalue { + &Lvalue::Projection(box Projection { + ref base, ref elem + }) => { + let old_source_info = self.source_info; + if let &Lvalue::Local(local) = base { + if self.mir.local_decls[local].internal { + // Internal locals are used in the `move_val_init` desugaring. + // We want to check unsafety against the source info of the + // desugaring, rather than the source info of the RHS. + self.source_info = self.mir.local_decls[local].source_info; + } + } + let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx); + match base_ty.sty { + ty::TyRawPtr(..) => { + self.require_unsafe("dereference of raw pointer") + } + ty::TyAdt(adt, _) if adt.is_union() => { + if context == LvalueContext::Store || + context == LvalueContext::Drop + { + let elem_ty = match elem { + &ProjectionElem::Field(_, ty) => ty, + _ => span_bug!( + self.source_info.span, + "non-field projection {:?} from union?", + lvalue) + }; + if elem_ty.moves_by_default(self.tcx, self.param_env, + self.source_info.span) { + self.require_unsafe( + "assignment to non-`Copy` union field") + } else { + // write to non-move union, safe + } + } else { + self.require_unsafe("access to union field") + } + } + _ => {} + } + self.source_info = old_source_info; + } + &Lvalue::Local(..) => { + // locals are safe + } + &Lvalue::Static(box Static { def_id, ty: _ }) => { + if self.is_static_mut(def_id) { + self.require_unsafe("use of mutable static"); + } else if self.tcx.is_foreign_item(def_id) { + let source_info = self.source_info; + let lint_root = + self.visibility_scope_info[source_info.scope].lint_root; + self.register_violations(&[UnsafetyViolation { + source_info, + description: "use of extern static", + lint_node_id: Some(lint_root) + }]); + } + } + } + self.super_lvalue(lvalue, context, location); + } +} + +impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { + fn is_static_mut(&self, def_id: DefId) -> bool { + if let Some(node) = self.tcx.hir.get_if_local(def_id) { + match node { + Node::NodeItem(&hir::Item { + node: hir::ItemStatic(_, hir::MutMutable, _), .. + }) => true, + Node::NodeForeignItem(&hir::ForeignItem { + node: hir::ForeignItemStatic(_, mutbl), .. + }) => mutbl, + _ => false + } + } else { + match self.tcx.describe_def(def_id) { + Some(Def::Static(_, mutbl)) => mutbl, + _ => false + } + } + } + fn require_unsafe(&mut self, + description: &'static str) + { + let source_info = self.source_info; + self.register_violations(&[UnsafetyViolation { + source_info, description, lint_node_id: None + }]); + } + + fn register_violations(&mut self, violations: &[UnsafetyViolation]) { + match self.visibility_scope_info[self.source_info.scope].safety { + Safety::Safe => { + for violation in violations { + if !self.violations.contains(violation) { + self.violations.push(violation.clone()) + } + } + } + Safety::BuiltinUnsafe | Safety::FnUnsafe => {} + Safety::ExplicitUnsafe(node_id) => { + if !violations.is_empty() { + self.used_unsafe.insert(node_id); + } + } + } + } +} + +pub(crate) fn provide(providers: &mut Providers) { + *providers = Providers { + unsafety_violations, + ..*providers + }; +} + +struct UnusedUnsafeVisitor<'a, 'tcx: 'a> { + tcx: TyCtxt<'a, 'tcx, 'tcx>, + used_unsafe: FxHashSet +} + +impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for UnusedUnsafeVisitor<'a, 'tcx> { + fn nested_visit_map<'this>(&'this mut self) -> + hir::intravisit::NestedVisitorMap<'this, 'tcx> + { + hir::intravisit::NestedVisitorMap::None + } + + fn visit_block(&mut self, block: &'tcx hir::Block) { + hir::intravisit::walk_block(self, block); + + if let hir::UnsafeBlock(hir::UserProvided) = block.rules { + if !self.used_unsafe.contains(&block.id) { + self.report_unused_unsafe(block); + } + } + } +} + +impl<'a, 'tcx> UnusedUnsafeVisitor<'a, 'tcx> { + /// Return the NodeId for an enclosing scope that is also `unsafe` + fn is_enclosed(&self, id: ast::NodeId) -> Option<(String, ast::NodeId)> { + let parent_id = self.tcx.hir.get_parent_node(id); + if parent_id != id { + if self.used_unsafe.contains(&parent_id) { + Some(("block".to_string(), parent_id)) + } else if let Some(hir::map::NodeItem(&hir::Item { + node: hir::ItemFn(_, hir::Unsafety::Unsafe, _, _, _, _), + .. + })) = self.tcx.hir.find(parent_id) { + Some(("fn".to_string(), parent_id)) + } else { + self.is_enclosed(parent_id) + } + } else { + None + } + } + + fn report_unused_unsafe(&self, block: &'tcx hir::Block) { + let mut db = self.tcx.struct_span_lint_node(UNUSED_UNSAFE, + block.id, + block.span, + "unnecessary `unsafe` block"); + db.span_label(block.span, "unnecessary `unsafe` block"); + if let Some((kind, id)) = self.is_enclosed(block.id) { + db.span_note(self.tcx.hir.span(id), + &format!("because it's nested under this `unsafe` {}", kind)); + } + db.emit(); + } +} + +fn check_unused_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + def_id: DefId, + used_unsafe: FxHashSet) +{ + let body_id = + tcx.hir.as_local_node_id(def_id).and_then(|node_id| { + tcx.hir.maybe_body_owned_by(node_id) + }); + + let body_id = match body_id { + Some(body) => body, + None => { + debug!("check_unused_unsafe({:?}) - no body found", def_id); + return + } + }; + let body = tcx.hir.body(body_id); + debug!("check_unused_unsafe({:?}, body={:?}, used_unsafe={:?})", + def_id, body, used_unsafe); + + hir::intravisit::Visitor::visit_body( + &mut UnusedUnsafeVisitor { tcx, used_unsafe }, + body); +} + +fn unsafety_violations<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> + Rc<[UnsafetyViolation]> +{ + debug!("unsafety_violations({:?})", def_id); + + // NB: this borrow is valid because all the consumers of + // `mir_const` force this. + let mir = &tcx.mir_const(def_id).borrow(); + + let visibility_scope_info = match mir.visibility_scope_info { + ClearOnDecode::Set(ref data) => data, + ClearOnDecode::Clear => { + debug!("unsafety_violations: {:?} - remote, skipping", def_id); + return Rc::new([]) + } + }; + + let param_env = tcx.param_env(def_id); + let mut checker = UnsafetyChecker::new( + mir, visibility_scope_info, tcx, param_env); + checker.visit_mir(mir); + + check_unused_unsafe(tcx, def_id, checker.used_unsafe); + checker.violations.into() +} + +pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) { + debug!("check_unsafety({:?})", def_id); + match tcx.def_key(def_id).disambiguated_data.data { + // closures are handled by their parent fn. + DefPathData::ClosureExpr => return, + _ => {} + }; + + for &UnsafetyViolation { + source_info, description, lint_node_id + } in &*tcx.unsafety_violations(def_id) { + // Report an error. + if let Some(lint_node_id) = lint_node_id { + tcx.lint_node(SAFE_EXTERN_STATICS, + lint_node_id, + source_info.span, + &format!("{} requires unsafe function or \ + block (error E0133)", description)); + } else { + struct_span_err!( + tcx.sess, source_info.span, E0133, + "{} requires unsafe function or block", description) + .span_label(source_info.span, description) + .emit(); + } + } +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index e0f2a40ab0732..7245b2daa1260 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -26,6 +26,7 @@ use transform; pub mod add_validation; pub mod clean_end_regions; +pub mod check_unsafety; pub mod simplify_branches; pub mod simplify; pub mod erase_regions; @@ -46,6 +47,7 @@ pub mod nll; pub(crate) fn provide(providers: &mut Providers) { self::qualify_consts::provide(providers); + self::check_unsafety::provide(providers); *providers = Providers { mir_keys, mir_const, @@ -116,6 +118,7 @@ fn mir_validated<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx // directly need the result or `mir_const_qualif`, so we can just force it. ty::queries::mir_const_qualif::force(tcx, DUMMY_SP, def_id); } + ty::queries::unsafety_violations::force(tcx, DUMMY_SP, def_id); let mut mir = tcx.mir_const(def_id).steal(); transform::run_suite(tcx, source, MIR_VALIDATED, &mut mir); diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index ab5998a34805b..5311b80d4bdf7 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -487,6 +487,16 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { if self.is_box_free(func) { self.check_box_free_inputs(mir, term, &sig, args); + // THIS IS A TEST. TEST TEST. + if let ClearOnDecode::Set(ref data) = mir.visibility_scope_info { + let lint_node_id = data[term.source_info.scope].lint_root; + tcx.struct_span_lint_node( + ::rustc::lint::builtin::TRIVIAL_NUMERIC_CASTS, + lint_node_id, + term.source_info.span, + "hi I'm a lint") + .emit(); + } } else { self.check_call_inputs(mir, term, &sig, args); } diff --git a/src/test/compile-fail/borrowck/borrowck-move-from-unsafe-ptr.rs b/src/test/compile-fail/borrowck/borrowck-move-from-unsafe-ptr.rs index 7284fa7a850f0..9a39ff6206bfb 100644 --- a/src/test/compile-fail/borrowck/borrowck-move-from-unsafe-ptr.rs +++ b/src/test/compile-fail/borrowck/borrowck-move-from-unsafe-ptr.rs @@ -9,8 +9,8 @@ // except according to those terms. -fn foo(x: *const Box) -> Box { - let y = *x; //~ ERROR dereference of raw pointer requires unsafe function or block +unsafe fn foo(x: *const Box) -> Box { + let y = *x; //~ ERROR cannot move out of dereference of raw pointer return y; } diff --git a/src/test/compile-fail/issue-43733.rs b/src/test/compile-fail/issue-43733.rs index f10531e407d62..0ae4cafa88b66 100644 --- a/src/test/compile-fail/issue-43733.rs +++ b/src/test/compile-fail/issue-43733.rs @@ -16,6 +16,8 @@ type Foo = std::cell::RefCell; #[cfg(target_thread_local)] static __KEY: std::thread::__FastLocalKeyInner = std::thread::__FastLocalKeyInner::new(); +//~^^ ERROR Sync` is not satisfied +//~^^^ ERROR Sync` is not satisfied #[cfg(not(target_thread_local))] static __KEY: std::thread::__OsLocalKeyInner = @@ -25,7 +27,7 @@ fn __getit() -> std::option::Option< &'static std::cell::UnsafeCell< std::option::Option>> { - __KEY.get() //~ ERROR invocation of unsafe method requires unsafe + __KEY.get() //~ ERROR call to unsafe function requires unsafe } static FOO: std::thread::LocalKey = diff --git a/src/test/compile-fail/union/union-unsafe.rs b/src/test/compile-fail/union/union-unsafe.rs index 2e018e696a415..e57d65dcb891f 100644 --- a/src/test/compile-fail/union/union-unsafe.rs +++ b/src/test/compile-fail/union/union-unsafe.rs @@ -42,8 +42,8 @@ fn main() { let mut u1 = U1 { a: 10 }; // OK let a = u1.a; //~ ERROR access to union field requires unsafe u1.a = 11; // OK - let U1 { a } = u1; //~ ERROR matching on union field requires unsafe - if let U1 { a: 12 } = u1 {} //~ ERROR matching on union field requires unsafe + let U1 { a } = u1; //~ ERROR access to union field requires unsafe + if let U1 { a: 12 } = u1 {} //~ ERROR access to union field requires unsafe // let U1 { .. } = u1; // OK let mut u2 = U2 { a: String::from("old") }; // OK diff --git a/src/test/compile-fail/unsafe-fn-assign-deref-ptr.rs b/src/test/compile-fail/unsafe-fn-assign-deref-ptr.rs index cff10329b8589..f30da250f6ac8 100644 --- a/src/test/compile-fail/unsafe-fn-assign-deref-ptr.rs +++ b/src/test/compile-fail/unsafe-fn-assign-deref-ptr.rs @@ -1,3 +1,4 @@ + // Copyright 2012 The Rust Project Developers. See the COPYRIGHT // file at the top-level directory of this distribution and at // http://rust-lang.org/COPYRIGHT. @@ -9,7 +10,7 @@ // except according to those terms. -fn f(p: *const u8) { +fn f(p: *mut u8) { *p = 0; //~ ERROR dereference of raw pointer requires unsafe function or block return; } diff --git a/src/test/compile-fail/unsafe-move-val-init.rs b/src/test/compile-fail/unsafe-move-val-init.rs new file mode 100644 index 0000000000000..84a8c84a0dbd2 --- /dev/null +++ b/src/test/compile-fail/unsafe-move-val-init.rs @@ -0,0 +1,20 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(core_intrinsics)] + +use std::intrinsics; + +// `move_val_init` has an odd desugaring, check that it is still treated +// as unsafe. +fn main() { + intrinsics::move_val_init(1 as *mut u32, 1); + //~^ ERROR dereference of raw pointer requires unsafe function or block +} From 9c4898eaddd7e54ba442899e0a6b2acf4d14f6c4 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 20 Sep 2017 16:34:31 +0300 Subject: [PATCH 3/5] address review comments --- src/librustc/mir/mod.rs | 2 +- src/librustc_driver/driver.rs | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index a5fe482845367..ba221ef6ae10b 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -491,7 +491,7 @@ pub struct LocalDecl<'tcx> { /// The *lexical* visibility scope the local is defined /// in. If the local was defined in a let-statement, this /// is *within* the let-statement, rather than outside - /// of iit. + /// of it. pub lexical_scope: VisibilityScope, } diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 4725f6e2c7839..d9df2e1f9ad50 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -1005,10 +1005,6 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session, // What we need to run borrowck etc. passes.push_pass(MIR_VALIDATED, mir::transform::qualify_consts::QualifyAndPromoteConstants); - - // FIXME: ariel points SimplifyBranches should run after - // mir-borrowck; otherwise code within `if false { ... }` would - // not be checked. passes.push_pass(MIR_VALIDATED, mir::transform::simplify::SimplifyCfg::new("qualify-consts")); passes.push_pass(MIR_VALIDATED, mir::transform::nll::NLL); From b408144dbec61d3257444fbee529aa4617161c28 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 21 Sep 2017 00:29:32 +0300 Subject: [PATCH 4/5] remove test code accidentally checked in --- src/librustc_mir/transform/type_check.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 5311b80d4bdf7..ab5998a34805b 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -487,16 +487,6 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { if self.is_box_free(func) { self.check_box_free_inputs(mir, term, &sig, args); - // THIS IS A TEST. TEST TEST. - if let ClearOnDecode::Set(ref data) = mir.visibility_scope_info { - let lint_node_id = data[term.source_info.scope].lint_root; - tcx.struct_span_lint_node( - ::rustc::lint::builtin::TRIVIAL_NUMERIC_CASTS, - lint_node_id, - term.source_info.span, - "hi I'm a lint") - .emit(); - } } else { self.check_call_inputs(mir, term, &sig, args); } From 516534ffdf2d267d5706443d5bd2bd7987d7498e Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 24 Sep 2017 13:15:18 +0300 Subject: [PATCH 5/5] fix test --- src/test/compile-fail/issue-43733.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/compile-fail/issue-43733.rs b/src/test/compile-fail/issue-43733.rs index 0ae4cafa88b66..90ccc589b4ea7 100644 --- a/src/test/compile-fail/issue-43733.rs +++ b/src/test/compile-fail/issue-43733.rs @@ -9,15 +9,15 @@ // except according to those terms. #![feature(const_fn)] +#![feature(thread_local)] #![feature(cfg_target_thread_local, thread_local_internals)] type Foo = std::cell::RefCell; #[cfg(target_thread_local)] +#[thread_local] static __KEY: std::thread::__FastLocalKeyInner = std::thread::__FastLocalKeyInner::new(); -//~^^ ERROR Sync` is not satisfied -//~^^^ ERROR Sync` is not satisfied #[cfg(not(target_thread_local))] static __KEY: std::thread::__OsLocalKeyInner =