auto merge of #12419 : huonw/rust/compiler-unsafe, r=alexcrichton

Previously an `unsafe` block created by the compiler (like those in the
formatting macros) would be "ignored" if surrounded by `unsafe`, that
is, the internal unsafety would be being legitimised by the external
block:

    unsafe { println!("...") } =(expansion)=> unsafe { ... unsafe { ... } }

And the code in the inner block would be using the outer block, making
it considered used (and the inner one considered unused).

This patch forces the compiler to create a new unsafe context for
compiler generated blocks, so that their internal unsafety doesn't
escape to external blocks.

Fixes #12418.

Author: bors

src/librustc/middle/effect.rs

@@ -106,11 +106,28 @@ impl Visitor<()> for EffectCheckVisitor {
 
     fn visit_block(&mut self, block: &ast::Block, _:()) {
         let old_unsafe_context = self.unsafe_context;
-        let is_unsafe = match block.rules {
-            ast::UnsafeBlock(..) => true, ast::DefaultBlock => false
-        };
-        if is_unsafe && self.unsafe_context == SafeContext {
-            self.unsafe_context = UnsafeBlock(block.id)
+        match block.rules {
+            ast::DefaultBlock => {}
+            ast::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` but 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 == SafeContext || source == ast::CompilerGenerated {
+                    self.unsafe_context = UnsafeBlock(block.id)
+                }
+            }
         }
 
         visit::walk_block(self, block, ());

src/librustc/middle/trans/base.rs

@@ -918,11 +918,9 @@ pub fn invoke<'a>(
     }
 
     if need_invoke(bcx) {
-        unsafe {
-            debug!("invoking {} at {}", llfn, bcx.llbb);
-            for &llarg in llargs.iter() {
-                debug!("arg: {}", llarg);
-            }
+        debug!("invoking {} at {}", llfn, bcx.llbb);
+        for &llarg in llargs.iter() {
+            debug!("arg: {}", llarg);
         }
         let normal_bcx = bcx.fcx.new_temp_block("normal-return");
         let landing_pad = bcx.fcx.get_landing_pad();
@@ -940,11 +938,9 @@ pub fn invoke<'a>(
                               attributes);
         return (llresult, normal_bcx);
     } else {
-        unsafe {
-            debug!("calling {} at {}", llfn, bcx.llbb);
-            for &llarg in llargs.iter() {
-                debug!("arg: {}", llarg);
-            }
+        debug!("calling {} at {}", llfn, bcx.llbb);
+        for &llarg in llargs.iter() {
+            debug!("arg: {}", llarg);
         }
 
         match call_info {

src/librustc/middle/typeck/check/mod.rs

@@ -991,9 +991,7 @@ impl RegionScope for infer::InferCtxt {
 
 impl FnCtxt {
     pub fn tag(&self) -> ~str {
-        unsafe {
-            format!("{}", self as *FnCtxt)
-        }
+        format!("{}", self as *FnCtxt)
     }
 
     pub fn local_ty(&self, span: Span, nid: ast::NodeId) -> ty::t {

src/libstd/comm/shared.rs

@@ -486,14 +486,12 @@ impl<T: Send> Packet<T> {
 #[unsafe_destructor]
 impl<T: Send> Drop for Packet<T> {
     fn drop(&mut self) {
-        unsafe {
-            // Note that this load is not only an assert for correctness about
-            // disconnection, but also a proper fence before the read of
-            // `to_wake`, so this assert cannot be removed with also removing
-            // the `to_wake` assert.
-            assert_eq!(self.cnt.load(atomics::SeqCst), DISCONNECTED);
-            assert_eq!(self.to_wake.load(atomics::SeqCst), 0);
-            assert_eq!(self.channels.load(atomics::SeqCst), 0);
-        }
+        // Note that this load is not only an assert for correctness about
+        // disconnection, but also a proper fence before the read of
+        // `to_wake`, so this assert cannot be removed with also removing
+        // the `to_wake` assert.
+        assert_eq!(self.cnt.load(atomics::SeqCst), DISCONNECTED);
+        assert_eq!(self.to_wake.load(atomics::SeqCst), 0);
+        assert_eq!(self.channels.load(atomics::SeqCst), 0);
     }
 }

src/libstd/comm/stream.rs

@@ -471,13 +471,11 @@ impl<T: Send> Packet<T> {
 #[unsafe_destructor]
 impl<T: Send> Drop for Packet<T> {
     fn drop(&mut self) {
-        unsafe {
-            // Note that this load is not only an assert for correctness about
-            // disconnection, but also a proper fence before the read of
-            // `to_wake`, so this assert cannot be removed with also removing
-            // the `to_wake` assert.
-            assert_eq!(self.cnt.load(atomics::SeqCst), DISCONNECTED);
-            assert_eq!(self.to_wake.load(atomics::SeqCst), 0);
-        }
+        // Note that this load is not only an assert for correctness about
+        // disconnection, but also a proper fence before the read of
+        // `to_wake`, so this assert cannot be removed with also removing
+        // the `to_wake` assert.
+        assert_eq!(self.cnt.load(atomics::SeqCst), DISCONNECTED);
+        assert_eq!(self.to_wake.load(atomics::SeqCst), 0);
     }
 }

src/test/compile-fail/unsafe-around-compiler-generated-unsafe.rs

@@ -0,0 +1,17 @@
+// Copyright 2014 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// issue #12418
+
+#[deny(unused_unsafe)];
+
+fn main() {
+    unsafe { println!("foo"); } //~ ERROR unnecessary `unsafe`
+}