From 058202e5235d334f8b3017b057c684b01a6bde27 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 30 Aug 2017 15:18:46 -0700 Subject: [PATCH] rustc: Remove the `used_unsafe` field on TyCtxt Now that lint levels are available for the entire compilation, this can be an entirely local lint in `effect.rs` --- src/librustc/lint/builtin.rs | 9 +++- src/librustc/middle/effect.rs | 55 ++++++++++++++++++++-- src/librustc/ty/context.rs | 5 -- src/librustc_lint/lib.rs | 1 - src/librustc_lint/unused.rs | 54 --------------------- src/test/ui/span/lint-unused-unsafe.stderr | 16 +++---- 6 files changed, 66 insertions(+), 74 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 811bf977610..21852468146 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -216,6 +216,12 @@ "detects use of deprecated items" } +declare_lint! { + pub UNUSED_UNSAFE, + Warn, + "unnecessary use of an `unsafe` block" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// which are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -256,7 +262,8 @@ fn get_lints(&self) -> LintArray { MISSING_FRAGMENT_SPECIFIER, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES, LATE_BOUND_LIFETIME_ARGUMENTS, - DEPRECATED + DEPRECATED, + UNUSED_UNSAFE ) } } diff --git a/src/librustc/middle/effect.rs b/src/librustc/middle/effect.rs index 98934d60703..7290353e48b 100644 --- a/src/librustc/middle/effect.rs +++ b/src/librustc/middle/effect.rs @@ -14,12 +14,14 @@ use ty::{self, TyCtxt}; use lint; +use lint::builtin::UNUSED_UNSAFE; -use syntax::ast; -use syntax_pos::Span; -use hir::{self, PatKind}; 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 { @@ -47,6 +49,7 @@ struct EffectCheckVisitor<'a, 'tcx: 'a> { /// Whether we're in an unsafe context. unsafe_context: UnsafeContext, + used_unsafe: FxHashSet, } impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> { @@ -73,7 +76,7 @@ fn require_unsafe_ext(&mut self, node_id: ast::NodeId, span: Span, UnsafeBlock(block_id) => { // OK, but record this. debug!("effect: recording unsafe block as used: {}", block_id); - self.tcx.used_unsafe.borrow_mut().insert(block_id); + self.used_unsafe.insert(block_id); } UnsafeFn => {} } @@ -159,7 +162,48 @@ fn visit_block(&mut self, block: &'tcx hir::Block) { intravisit::walk_block(self, block); - self.unsafe_context = old_unsafe_context + 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) { @@ -265,6 +309,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, '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/ty/context.rs b/src/librustc/ty/context.rs index 1fe53882c70..a3d25df10be 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -855,10 +855,6 @@ pub struct GlobalCtxt<'tcx> { pub lang_items: middle::lang_items::LanguageItems, - /// Set of used unsafe nodes (functions or blocks). Unsafe nodes not - /// present in this set can be warned about. - pub used_unsafe: RefCell, - /// Set of nodes which mark locals as mutable which end up getting used at /// some point. Local variable definitions not in this set can be warned /// about. @@ -1091,7 +1087,6 @@ pub fn create_and_enter(s: &'tcx Session, normalized_cache: RefCell::new(FxHashMap()), inhabitedness_cache: RefCell::new(FxHashMap()), lang_items, - used_unsafe: RefCell::new(NodeSet()), used_mut_nodes: RefCell::new(NodeSet()), stability: RefCell::new(stability), selection_cache: traits::SelectionCache::new(), diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 5ef277f02ac..fbf993f4557 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -128,7 +128,6 @@ macro_rules! add_lint_group { NonSnakeCase, NonUpperCaseGlobals, NonShorthandFieldPatterns, - UnusedUnsafe, UnsafeCode, UnusedMut, UnusedAllocation, diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index cbc4ebe90fd..15efc14b061 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -204,60 +204,6 @@ fn check_must_use(cx: &LateContext, def_id: DefId, sp: Span, describe_path: &str } } -declare_lint! { - pub UNUSED_UNSAFE, - Warn, - "unnecessary use of an `unsafe` block" -} - -#[derive(Copy, Clone)] -pub struct UnusedUnsafe; - -impl LintPass for UnusedUnsafe { - fn get_lints(&self) -> LintArray { - lint_array!(UNUSED_UNSAFE) - } -} - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedUnsafe { - fn check_expr(&mut self, cx: &LateContext, e: &hir::Expr) { - /// Return the NodeId for an enclosing scope that is also `unsafe` - fn is_enclosed(cx: &LateContext, id: ast::NodeId) -> Option<(String, ast::NodeId)> { - let parent_id = cx.tcx.hir.get_parent_node(id); - if parent_id != id { - if cx.tcx.used_unsafe.borrow().contains(&parent_id) { - Some(("block".to_string(), parent_id)) - } else if let Some(hir::map::NodeItem(&hir::Item { - node: hir::ItemFn(_, hir::Unsafety::Unsafe, _, _, _, _), - .. - })) = cx.tcx.hir.find(parent_id) { - Some(("fn".to_string(), parent_id)) - } else { - is_enclosed(cx, parent_id) - } - } else { - None - } - } - if let hir::ExprBlock(ref blk) = e.node { - // Don't warn about generated blocks, that'll just pollute the output. - if blk.rules == hir::UnsafeBlock(hir::UserProvided) && - !cx.tcx.used_unsafe.borrow().contains(&blk.id) { - - let mut db = cx.struct_span_lint(UNUSED_UNSAFE, blk.span, - "unnecessary `unsafe` block"); - - db.span_label(blk.span, "unnecessary `unsafe` block"); - if let Some((kind, id)) = is_enclosed(cx, blk.id) { - db.span_note(cx.tcx.hir.span(id), - &format!("because it's nested under this `unsafe` {}", kind)); - } - db.emit(); - } - } - } -} - declare_lint! { pub PATH_STATEMENTS, Warn, diff --git a/src/test/ui/span/lint-unused-unsafe.stderr b/src/test/ui/span/lint-unused-unsafe.stderr index f4998e08907..1fa5f94aa4c 100644 --- a/src/test/ui/span/lint-unused-unsafe.stderr +++ b/src/test/ui/span/lint-unused-unsafe.stderr @@ -65,14 +65,12 @@ note: because it's nested under this `unsafe` block | |_____^ error: unnecessary `unsafe` block - --> $DIR/lint-unused-unsafe.rs:39:5 + --> $DIR/lint-unused-unsafe.rs:40:9 | -39 | / unsafe { //~ ERROR: unnecessary `unsafe` block -40 | | unsafe { //~ ERROR: unnecessary `unsafe` block +40 | / unsafe { //~ ERROR: unnecessary `unsafe` block 41 | | unsf() 42 | | } -43 | | } - | |_____^ unnecessary `unsafe` block + | |_________^ unnecessary `unsafe` block | note: because it's nested under this `unsafe` fn --> $DIR/lint-unused-unsafe.rs:38:1 @@ -87,12 +85,14 @@ note: because it's nested under this `unsafe` fn | |_^ error: unnecessary `unsafe` block - --> $DIR/lint-unused-unsafe.rs:40:9 + --> $DIR/lint-unused-unsafe.rs:39:5 | -40 | / unsafe { //~ ERROR: unnecessary `unsafe` block +39 | / unsafe { //~ ERROR: unnecessary `unsafe` block +40 | | unsafe { //~ ERROR: unnecessary `unsafe` block 41 | | unsf() 42 | | } - | |_________^ unnecessary `unsafe` block +43 | | } + | |_____^ unnecessary `unsafe` block | note: because it's nested under this `unsafe` fn --> $DIR/lint-unused-unsafe.rs:38:1 -- 2.44.0