Auto merge of #44195 - alexcrichton:remove-used-unsafe, r=nikomatsakis

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`

cc #44137

src/librustc/lint/builtin.rs

@@ -216,6 +216,12 @@ declare_lint! {
     "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 @@ impl LintPass for HardwiredLints {
             MISSING_FRAGMENT_SPECIFIER,
             PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
             LATE_BOUND_LIFETIME_ARGUMENTS,
-            DEPRECATED
+            DEPRECATED,
+            UNUSED_UNSAFE
         )
     }
 }

src/librustc/middle/effect.rs

@@ -14,12 +14,14 @@ use self::RootUnsafeContext::*;
 
 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<ast::NodeId>,
 }
 
 impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> {
@@ -73,7 +76,7 @@ impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> {
             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 @@ impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {
 
         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<ast::NodeId>,
+                       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());

src/librustc/ty/context.rs

@@ -853,10 +853,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<NodeSet>,
-
     /// 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.
@@ -1092,7 +1088,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             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(),

src/librustc_lint/lib.rs

@@ -128,7 +128,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                  NonSnakeCase,
                  NonUpperCaseGlobals,
                  NonShorthandFieldPatterns,
-                 UnusedUnsafe,
                  UnsafeCode,
                  UnusedMut,
                  UnusedAllocation,

src/librustc_lint/unused.rs

@@ -204,60 +204,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults {
     }
 }
 
-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,

src/test/ui/span/lint-unused-unsafe.stderr

@@ -64,6 +64,26 @@ note: because it's nested under this `unsafe` block
 36 | |     }
    | |_____^
 
+error: unnecessary `unsafe` block
+  --> $DIR/lint-unused-unsafe.rs:40:9
+   |
+40 | /         unsafe {                         //~ ERROR: unnecessary `unsafe` block
+41 | |             unsf()
+42 | |         }
+   | |_________^ unnecessary `unsafe` block
+   |
+note: because it's nested under this `unsafe` fn
+  --> $DIR/lint-unused-unsafe.rs:38:1
+   |
+38 | / unsafe fn bad7() {
+39 | |     unsafe {                             //~ ERROR: unnecessary `unsafe` block
+40 | |         unsafe {                         //~ ERROR: unnecessary `unsafe` block
+41 | |             unsf()
+42 | |         }
+43 | |     }
+44 | | }
+   | |_^
+
 error: unnecessary `unsafe` block
   --> $DIR/lint-unused-unsafe.rs:39:5
    |
@@ -86,25 +106,5 @@ note: because it's nested under this `unsafe` fn
 44 | | }
    | |_^
 
-error: unnecessary `unsafe` block
-  --> $DIR/lint-unused-unsafe.rs:40:9
-   |
-40 | /         unsafe {                         //~ ERROR: unnecessary `unsafe` block
-41 | |             unsf()
-42 | |         }
-   | |_________^ unnecessary `unsafe` block
-   |
-note: because it's nested under this `unsafe` fn
-  --> $DIR/lint-unused-unsafe.rs:38:1
-   |
-38 | / unsafe fn bad7() {
-39 | |     unsafe {                             //~ ERROR: unnecessary `unsafe` block
-40 | |         unsafe {                         //~ ERROR: unnecessary `unsafe` block
-41 | |             unsf()
-42 | |         }
-43 | |     }
-44 | | }
-   | |_^
-
 error: aborting due to 8 previous errors