]> git.lizzy.rs Git - rust.git/commitdiff
Hack for "unsafety hygiene" -- `push_unsafe!` and `pop_unsafe!`.
authorFelix S. Klock II <pnkfelix@pnkfx.org>
Fri, 5 Jun 2015 06:31:27 +0000 (08:31 +0200)
committerFelix S. Klock II <pnkfelix@pnkfx.org>
Wed, 22 Jul 2015 13:33:59 +0000 (15:33 +0200)
Even after expansion, the generated expressions still track depth of
such pushes (i.e. how often you have "pushed" without a corresponding
"pop"), and we add a rule that in a context with a positive
`push_unsafe!` depth, it is effectively an `unsafe` block context.

(This way, we can inject code that uses `unsafe` features, but still
contains within it a sub-expression that should inherit the outer
safety checking setting, outside of the injected code.)

This is a total hack; it not only needs a feature-gate, but probably
should be feature-gated forever (if possible).

ignore-pretty in test/run-pass/pushpop-unsafe-okay.rs

12 files changed:
src/librustc/middle/effect.rs
src/librustc_typeck/check/mod.rs
src/libsyntax/ast.rs
src/libsyntax/ext/base.rs
src/libsyntax/ext/expand.rs
src/libsyntax/ext/pushpop_safe.rs [new file with mode: 0644]
src/libsyntax/feature_gate.rs
src/libsyntax/lib.rs
src/libsyntax/print/pprust.rs
src/test/compile-fail/feature-gate-pushpop-unsafe.rs [new file with mode: 0644]
src/test/compile-fail/pushpop-unsafe-rejects.rs [new file with mode: 0644]
src/test/run-pass/pushpop-unsafe-okay.rs [new file with mode: 0644]

index b2a064bf86c6a08e3bd6b3a18a14db1f935d4559..a2e42245bbef84cf9cc5aa6510078688a1589c19 100644 (file)
@@ -10,7 +10,7 @@
 
 //! Enforces the Rust effect system. Currently there is just one effect,
 //! `unsafe`.
-use self::UnsafeContext::*;
+use self::RootUnsafeContext::*;
 
 use middle::def;
 use middle::ty::{self, Ty};
 use syntax::visit;
 use syntax::visit::Visitor;
 
+#[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 UnsafeContext {
+enum RootUnsafeContext {
     SafeContext,
     UnsafeFn,
     UnsafeBlock(ast::NodeId),
@@ -44,7 +56,8 @@ struct EffectCheckVisitor<'a, 'tcx: 'a> {
 
 impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> {
     fn require_unsafe(&mut self, span: Span, description: &str) {
-        match self.unsafe_context {
+        if self.unsafe_context.push_unsafe_count > 0 { return; }
+        match self.unsafe_context.root {
             SafeContext => {
                 // Report an error.
                 span_err!(self.tcx.sess, span, E0133,
@@ -75,9 +88,9 @@ fn visit_fn(&mut self, fn_kind: visit::FnKind<'v>, fn_decl: &'v ast::FnDecl,
 
         let old_unsafe_context = self.unsafe_context;
         if is_unsafe_fn {
-            self.unsafe_context = UnsafeFn
+            self.unsafe_context = UnsafeContext::new(UnsafeFn)
         } else if is_item_fn {
-            self.unsafe_context = SafeContext
+            self.unsafe_context = UnsafeContext::new(SafeContext)
         }
 
         visit::walk_fn(self, fn_kind, fn_decl, block, span);
@@ -105,10 +118,18 @@ fn visit_block(&mut self, block: &ast::Block) {
                 // 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)
+                if self.unsafe_context.root == SafeContext || source == ast::CompilerGenerated {
+                    self.unsafe_context.root = UnsafeBlock(block.id)
                 }
             }
+            ast::PushUnsafeBlock(..) => {
+                self.unsafe_context.push_unsafe_count =
+                    self.unsafe_context.push_unsafe_count.saturating_add(1);
+            }
+            ast::PopUnsafeBlock(..) => {
+                self.unsafe_context.push_unsafe_count =
+                    self.unsafe_context.push_unsafe_count.saturating_sub(1);
+            }
         }
 
         visit::walk_block(self, block);
@@ -162,7 +183,7 @@ fn visit_expr(&mut self, expr: &ast::Expr) {
 pub fn check_crate(tcx: &ty::ctxt) {
     let mut visitor = EffectCheckVisitor {
         tcx: tcx,
-        unsafe_context: SafeContext,
+        unsafe_context: UnsafeContext::new(SafeContext),
     };
 
     visit::walk_crate(&mut visitor, tcx.map.krate());
index fdeed657e071b7e0dfdd8d0b9d4c65efe251b430..d6afd72d3b4dbb429d48e64069469eb811bf7170 100644 (file)
@@ -231,12 +231,13 @@ fn adjust_for_branches<'a>(&self, fcx: &FnCtxt<'a, 'tcx>) -> Expectation<'tcx> {
 pub struct UnsafetyState {
     pub def: ast::NodeId,
     pub unsafety: ast::Unsafety,
+    pub unsafe_push_count: u32,
     from_fn: bool
 }
 
 impl UnsafetyState {
     pub fn function(unsafety: ast::Unsafety, def: ast::NodeId) -> UnsafetyState {
-        UnsafetyState { def: def, unsafety: unsafety, from_fn: true }
+        UnsafetyState { def: def, unsafety: unsafety, unsafe_push_count: 0, from_fn: true }
     }
 
     pub fn recurse(&mut self, blk: &ast::Block) -> UnsafetyState {
@@ -248,13 +249,20 @@ pub fn recurse(&mut self, blk: &ast::Block) -> UnsafetyState {
             ast::Unsafety::Unsafe if self.from_fn => *self,
 
             unsafety => {
-                let (unsafety, def) = match blk.rules {
-                    ast::UnsafeBlock(..) => (ast::Unsafety::Unsafe, blk.id),
-                    ast::DefaultBlock => (unsafety, self.def),
+                let (unsafety, def, count) = match blk.rules {
+                    ast::PushUnsafeBlock(..) =>
+                        (unsafety, blk.id, self.unsafe_push_count.saturating_add(1)),
+                    ast::PopUnsafeBlock(..) =>
+                        (unsafety, blk.id, self.unsafe_push_count.saturating_sub(1)),
+                    ast::UnsafeBlock(..) =>
+                        (ast::Unsafety::Unsafe, blk.id, self.unsafe_push_count),
+                    ast::DefaultBlock =>
+                        (unsafety, self.def, self.unsafe_push_count),
                 };
                 UnsafetyState{ def: def,
-                             unsafety: unsafety,
-                             from_fn: false }
+                               unsafety: unsafety,
+                               unsafe_push_count: count,
+                               from_fn: false }
             }
         }
     }
index a0059d33bed840c53d80a0c64cc12e0b8aee029d..fba9db401dbb1a2423762c0980beccd87d74f67c 100644 (file)
@@ -810,6 +810,8 @@ pub struct Field {
 pub enum BlockCheckMode {
     DefaultBlock,
     UnsafeBlock(UnsafeSource),
+    PushUnsafeBlock(UnsafeSource),
+    PopUnsafeBlock(UnsafeSource),
 }
 
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
index 499562edc0cc9e153b62837126396f14b252d564..409ae86db35d4ebe53404379c8a674e8b3e35c0f 100644 (file)
@@ -591,6 +591,12 @@ fn builtin_normal_expander(f: MacroExpanderFn) -> SyntaxExtension {
     syntax_expanders.insert(intern("cfg"),
                             builtin_normal_expander(
                                     ext::cfg::expand_cfg));
+    syntax_expanders.insert(intern("push_unsafe"),
+                            builtin_normal_expander(
+                                ext::pushpop_safe::expand_push_unsafe));
+    syntax_expanders.insert(intern("pop_unsafe"),
+                            builtin_normal_expander(
+                                ext::pushpop_safe::expand_pop_unsafe));
     syntax_expanders.insert(intern("trace_macros"),
                             builtin_normal_expander(
                                     ext::trace_macros::expand_trace_macros));
index 53befc092da88f363d9e10b91fa8a4d1a48f36d7..b540e0adeea87b0ea678a81236b556c4fcf1f273 100644 (file)
@@ -1504,6 +1504,7 @@ fn enable_concat_idents = allow_concat_idents,
         fn enable_trace_macros = allow_trace_macros,
         fn enable_allow_internal_unstable = allow_internal_unstable,
         fn enable_custom_derive = allow_custom_derive,
+        fn enable_pushpop_unsafe = allow_pushpop_unsafe,
     }
 }
 
diff --git a/src/libsyntax/ext/pushpop_safe.rs b/src/libsyntax/ext/pushpop_safe.rs
new file mode 100644 (file)
index 0000000..fee445c
--- /dev/null
@@ -0,0 +1,94 @@
+// Copyright 2015 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.
+
+/*
+ * The compiler code necessary to support the `push_unsafe!` and
+ * `pop_unsafe!` macros.
+ *
+ * This is a hack to allow a kind of "safety hygiene", where a macro
+ * can generate code with an interior expression that inherits the
+ * safety of some outer context.
+ *
+ * For example, in:
+ *
+ * ```rust
+ * fn foo() { push_unsafe!( { EXPR_1; pop_unsafe!( EXPR_2 ) } ) }
+ * ```
+ *
+ * the `EXPR_1` is considered to be in an `unsafe` context,
+ * but `EXPR_2` is considered to be in a "safe" (i.e. checked) context.
+ *
+ * For comparison, in:
+ *
+ * ```rust
+ * fn foo() { unsafe { push_unsafe!( { EXPR_1; pop_unsafe!( EXPR_2 ) } ) } }
+ * ```
+ *
+ * both `EXPR_1` and `EXPR_2` are considered to be in `unsafe`
+ * contexts.
+ *
+ */
+
+use ast;
+use codemap::Span;
+use ext::base::*;
+use ext::base;
+use ext::build::AstBuilder;
+use feature_gate;
+use ptr::P;
+
+enum PushPop { Push, Pop }
+
+pub fn expand_push_unsafe<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
+                               -> Box<base::MacResult+'cx> {
+    feature_gate::check_for_pushpop_syntax(
+        cx.ecfg.features, &cx.parse_sess.span_diagnostic, sp);
+    expand_pushpop_unsafe(cx, sp, tts, PushPop::Push)
+}
+
+pub fn expand_pop_unsafe<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
+                               -> Box<base::MacResult+'cx> {
+    feature_gate::check_for_pushpop_syntax(
+        cx.ecfg.features, &cx.parse_sess.span_diagnostic, sp);
+    expand_pushpop_unsafe(cx, sp, tts, PushPop::Pop)
+}
+
+fn expand_pushpop_unsafe<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree],
+                                  pp: PushPop) -> Box<base::MacResult+'cx> {
+    let mut exprs = match get_exprs_from_tts(cx, sp, tts) {
+        Some(exprs) => exprs.into_iter(),
+        None => return DummyResult::expr(sp),
+    };
+    let expr = match (exprs.next(), exprs.next()) {
+        (Some(expr), None) => expr,
+        _ => {
+            let msg = match pp {
+                PushPop::Push => "push_unsafe! takes 1 arguments",
+                PushPop::Pop => "pop_unsafe! takes 1 arguments",
+            };
+            cx.span_err(sp, msg);
+            return DummyResult::expr(sp);
+        }
+    };
+
+    let source = ast::UnsafeSource::CompilerGenerated;
+    let check_mode = match pp {
+        PushPop::Push => ast::BlockCheckMode::PushUnsafeBlock(source),
+        PushPop::Pop => ast::BlockCheckMode::PopUnsafeBlock(source),
+    };
+
+    MacEager::expr(cx.expr_block(P(ast::Block {
+        stmts: vec![],
+        expr: Some(expr),
+        id: ast::DUMMY_NODE_ID,
+        rules: check_mode,
+        span: sp
+    })))
+}
index ab8cf9ae6b64f98ad861a423f3f820ca129fe433..69d120cff601cacd2f1bcc024082dd7d983faee2 100644 (file)
@@ -80,6 +80,7 @@
     ("visible_private_types", "1.0.0", Active),
     ("slicing_syntax", "1.0.0", Accepted),
     ("box_syntax", "1.0.0", Active),
+    ("pushpop_unsafe", "1.2.0", Active),
     ("on_unimplemented", "1.0.0", Active),
     ("simd_ffi", "1.0.0", Active),
     ("allocator", "1.0.0", Active),
@@ -325,6 +326,7 @@ pub struct Features {
     pub allow_trace_macros: bool,
     pub allow_internal_unstable: bool,
     pub allow_custom_derive: bool,
+    pub allow_pushpop_unsafe: bool,
     pub simd_ffi: bool,
     pub unmarked_api: bool,
     pub negate_unsigned: bool,
@@ -348,6 +350,7 @@ pub fn new() -> Features {
             allow_trace_macros: false,
             allow_internal_unstable: false,
             allow_custom_derive: false,
+            allow_pushpop_unsafe: false,
             simd_ffi: false,
             unmarked_api: false,
             negate_unsigned: false,
@@ -358,6 +361,13 @@ pub fn new() -> Features {
     }
 }
 
+pub fn check_for_pushpop_syntax(f: Option<&Features>, diag: &SpanHandler, span: Span) {
+    if let Some(&Features { allow_pushpop_unsafe: true, .. }) = f {
+        return;
+    }
+    emit_feature_err(diag, "pushpop_unsafe", span, EXPLAIN_PUSHPOP_UNSAFE);
+}
+
 struct Context<'a> {
     features: Vec<&'static str>,
     span_handler: &'a SpanHandler,
@@ -787,6 +797,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler,
         allow_trace_macros: cx.has_feature("trace_macros"),
         allow_internal_unstable: cx.has_feature("allow_internal_unstable"),
         allow_custom_derive: cx.has_feature("custom_derive"),
+        allow_pushpop_unsafe: cx.has_feature("pushpop_unsafe"),
         simd_ffi: cx.has_feature("simd_ffi"),
         unmarked_api: cx.has_feature("unmarked_api"),
         negate_unsigned: cx.has_feature("negate_unsigned"),
index d93af5da13c1e2803a2aa263e9340d3df035a42b..5424c0b214a4ee980222d7b4fae62b73dfccb3e0 100644 (file)
@@ -120,6 +120,7 @@ pub mod ext {
     pub mod log_syntax;
     pub mod mtwt;
     pub mod quote;
+    pub mod pushpop_safe;
     pub mod source_util;
     pub mod trace_macros;
 
index 6693eed6aced5b63f7c67b72df03c224509b5ab3..448857389da6172b652d71a16b5704aa90c0164c 100644 (file)
@@ -1434,8 +1434,8 @@ pub fn print_block_maybe_unclosed(&mut self,
                                       attrs: &[ast::Attribute],
                                       close_box: bool) -> io::Result<()> {
         match blk.rules {
-            ast::UnsafeBlock(..) => try!(self.word_space("unsafe")),
-            ast::DefaultBlock => ()
+            ast::UnsafeBlock(..) | ast::PushUnsafeBlock(..) => try!(self.word_space("unsafe")),
+            ast::DefaultBlock    | ast::PopUnsafeBlock(..) => ()
         }
         try!(self.maybe_print_comment(blk.span.lo));
         try!(self.ann.pre(self, NodeBlock(blk)));
diff --git a/src/test/compile-fail/feature-gate-pushpop-unsafe.rs b/src/test/compile-fail/feature-gate-pushpop-unsafe.rs
new file mode 100644 (file)
index 0000000..e317b4c
--- /dev/null
@@ -0,0 +1,14 @@
+// Copyright 2015 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.
+
+fn main() {
+    let c = push_unsafe!('c'); //~ ERROR push/pop_unsafe macros are experimental
+    let c = pop_unsafe!('c'); //~ ERROR push/pop_unsafe macros are experimental
+}
diff --git a/src/test/compile-fail/pushpop-unsafe-rejects.rs b/src/test/compile-fail/pushpop-unsafe-rejects.rs
new file mode 100644 (file)
index 0000000..b009a67
--- /dev/null
@@ -0,0 +1,71 @@
+// 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.
+//
+// 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.
+
+// Basic sanity check for `push_unsafe!(EXPR)` and
+// `pop_unsafe!(EXPR)`: we can call unsafe code when there are a
+// positive number of pushes in the stack, or if we are within a
+// normal `unsafe` block, but otherwise cannot.
+
+static mut X: i32 = 0;
+
+unsafe fn f() { X += 1; return; }
+fn g() { unsafe { X += 1_000; } return; }
+
+fn main() {
+    push_unsafe!( {
+        f(); pop_unsafe!({
+            f() //~ ERROR: call to unsafe function
+        })
+    } );
+
+    push_unsafe!({
+        f();
+        pop_unsafe!({
+            g();
+            f(); //~ ERROR: call to unsafe function
+        })
+    } );
+
+    push_unsafe!({
+        g(); pop_unsafe!({
+            unsafe {
+                f();
+            }
+            f(); //~ ERROR: call to unsafe function
+        })
+    });
+
+
+    // Note: For implementation simplicity I have chosen to just have
+    // the stack do "saturated pop", but perhaps we would prefer to
+    // have cases like these two here be errors:
+
+    pop_unsafe!{ g() };
+
+    push_unsafe!({
+        pop_unsafe!(pop_unsafe!{ g() })
+    });
+
+
+    // Okay, back to examples that do error, even in the presence of
+    // "saturated pop"
+
+    push_unsafe!({
+        g();
+        pop_unsafe!(pop_unsafe!({
+            f() //~ ERROR: call to unsafe function
+        }))
+    });
+
+    pop_unsafe!({
+        f(); //~ ERROR: call to unsafe function
+    })
+
+}
diff --git a/src/test/run-pass/pushpop-unsafe-okay.rs b/src/test/run-pass/pushpop-unsafe-okay.rs
new file mode 100644 (file)
index 0000000..fc402d4
--- /dev/null
@@ -0,0 +1,56 @@
+// Copyright 2015 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.
+
+// Basic sanity check for `push_unsafe!(EXPR)` and
+// `pop_unsafe!(EXPR)`: we can call unsafe code when there are a
+// positive number of pushes in the stack, or if we are within a
+// normal `unsafe` block, but otherwise cannot.
+
+// ignore-pretty because the `push_unsafe!` and `pop_unsafe!` macros
+// are not integrated with the pretty-printer.
+
+#![feature(pushpop_unsafe)]
+
+static mut X: i32 = 0;
+
+unsafe fn f() { X += 1; return; }
+fn g() { unsafe { X += 1_000; } return; }
+
+fn check_reset_x(x: i32) -> bool {
+    #![allow(unused_parens)] // dont you judge my style choices!
+    unsafe {
+        let ret = (x == X);
+        X = 0;
+        ret
+    }
+}
+
+fn main() {
+    // double-check test infrastructure
+    assert!(check_reset_x(0));
+    unsafe { f(); }
+    assert!(check_reset_x(1));
+    assert!(check_reset_x(0));
+    { g(); }
+    assert!(check_reset_x(1000));
+    assert!(check_reset_x(0));
+    unsafe { f(); g(); g(); }
+    assert!(check_reset_x(2001));
+
+    push_unsafe!( { f(); pop_unsafe!( g() ) } );
+    assert!(check_reset_x(1_001));
+    push_unsafe!( { g(); pop_unsafe!( unsafe { f(); f(); } ) } );
+    assert!(check_reset_x(1_002));
+
+    unsafe { push_unsafe!( { f(); pop_unsafe!( { f(); f(); } ) } ); }
+    assert!(check_reset_x(3));
+    push_unsafe!( { f(); push_unsafe!( { pop_unsafe!( { f(); f(); f(); } ) } ); } );
+    assert!(check_reset_x(4));
+}