Skip to content

Commit

Permalink
Hack for "unsafety hygiene" -- push_unsafe! and pop_unsafe!.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pnkfelix committed Jul 22, 2015
1 parent 25281b1 commit 1829fa5
Show file tree
Hide file tree
Showing 12 changed files with 301 additions and 16 deletions.
37 changes: 29 additions & 8 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -21,8 +21,20 @@ use syntax::codemap::Span;
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),
Expand All @@ -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,
Expand Down Expand Up @@ -75,9 +88,9 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {

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);
Expand Down Expand Up @@ -105,10 +118,18 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {
// 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);
Expand Down Expand Up @@ -162,7 +183,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {
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());
Expand Down
20 changes: 14 additions & 6 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,13 @@ impl<'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 {
Expand All @@ -248,13 +249,20 @@ impl 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 }
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,8 @@ pub type SpannedIdent = Spanned<Ident>;
pub enum BlockCheckMode {
DefaultBlock,
UnsafeBlock(UnsafeSource),
PushUnsafeBlock(UnsafeSource),
PopUnsafeBlock(UnsafeSource),
}

#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
Expand Down
6 changes: 6 additions & 0 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,12 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>)
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));
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,7 @@ impl<'feat> ExpansionConfig<'feat> {
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,
}
}

Expand Down
94 changes: 94 additions & 0 deletions src/libsyntax/ext/pushpop_safe.rs
Original file line number Diff line number Diff line change
@@ -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
})))
}
11 changes: 11 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Status)] = &[
("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),
Expand Down Expand Up @@ -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,
Expand All @@ -348,6 +350,7 @@ impl 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,
Expand All @@ -358,6 +361,13 @@ impl 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,
Expand Down Expand Up @@ -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"),
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/print/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1434,8 +1434,8 @@ impl<'a> State<'a> {
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)));
Expand Down
14 changes: 14 additions & 0 deletions src/test/compile-fail/feature-gate-pushpop-unsafe.rs
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 1829fa5

Please sign in to comment.