Auto merge of #27215 - pnkfelix:fsk-placer-take-5-just-in, r=nikomatsakis

Macro desugaring of `in PLACE { BLOCK }` into "simpler" expressions following the in-development "Placer" protocol.

Includes Placer API that one can override to integrate support for `in` into one's own type.  (See [RFC 809].)

[RFC 809]: https://github.com/rust-lang/rfcs/blob/master/text/0809-box-and-in-for-stdlib.md

Part of #22181

Replaced PR #26180.

Turns on the `in PLACE { BLOCK }` syntax, while leaving in support for the old `box (PLACE) EXPR` syntax (since we need to support that at least until we have a snapshot with support for `in PLACE { BLOCK }`.

(Note that we are not 100% committed to the `in PLACE { BLOCK }` syntax.  In particular I still want to play around with some other alternatives.  Still, I want to get the fundamental framework for the protocol landed so we can play with implementing it for non `Box` types.)

----

Also, this PR leaves out support for desugaring-based `box EXPR`.  We will hopefully land that in the future, but for the short term there are type-inference issues injected by that change that we want to resolve separately.

9 files changed, 371 insertions, 12 deletions

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index a0059d33bed..fba9db401db 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -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)]
diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index e6bc3218897..17e6b2c2e12 100644
--- a/src/libsyntax/codemap.rs
+++ b/src/libsyntax/codemap.rs
@@ -980,6 +980,10 @@ impl CodeMap {
                             mac_span.lo <= span.lo && span.hi <= mac_span.hi
                         });
 
+                    debug!("span_allows_unstable: span: {:?} call_site: {:?} callee: {:?}",
+                           (span.lo, span.hi),
+                           (info.call_site.lo, info.call_site.hi),
+                           info.callee.span.map(|x| (x.lo, x.hi)));
                     debug!("span_allows_unstable: from this expansion? {}, allows unstable? {}",
                            span_comes_from_this_expansion,
                            info.callee.allow_internal_unstable);
diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs
index 499562edc0c..409ae86db35 100644
--- a/src/libsyntax/ext/base.rs
+++ b/src/libsyntax/ext/base.rs
@@ -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));
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 53befc092da..faa1e5b2f51 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/src/libsyntax/ext/expand.rs
@@ -33,6 +33,16 @@ use visit;
 use visit::Visitor;
 use std_inject;
 
+// Given suffix ["b","c","d"], returns path `::std::b::c::d` when
+// `fld.cx.use_std`, and `::core::b::c::d` otherwise.
+fn mk_core_path(fld: &mut MacroExpander,
+                span: Span,
+                suffix: &[&'static str]) -> ast::Path {
+    let mut idents = vec![fld.cx.ident_of_std("core")];
+    for s in suffix.iter() { idents.push(fld.cx.ident_of(*s)); }
+    fld.cx.path_global(span, idents)
+}
+
 pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
     fn push_compiler_expansion(fld: &mut MacroExpander, span: Span, expansion_desc: &str) {
         fld.cx.bt_push(ExpnInfo {
@@ -40,13 +50,26 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
             callee: NameAndSpan {
                 name: expansion_desc.to_string(),
                 format: CompilerExpansion,
+
+                // This does *not* mean code generated after
+                // `push_compiler_expansion` is automatically exempt
+                // from stability lints; must also tag such code with
+                // an appropriate span from `fld.cx.backtrace()`.
                 allow_internal_unstable: true,
+
                 span: None,
             },
         });
     }
 
-    e.and_then(|ast::Expr {id, node, span}| match node {
+    // Sets the expn_id so that we can use unstable methods.
+    fn allow_unstable(fld: &mut MacroExpander, span: Span) -> Span {
+        Span { expn_id: fld.cx.backtrace(), ..span }
+    }
+
+    let expr_span = e.span;
+    return e.and_then(|ast::Expr {id, node, span}| match node {
+
         // expr_mac should really be expr_ext or something; it's the
         // entry-point for all syntax extensions.
         ast::ExprMac(mac) => {
@@ -71,6 +94,118 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
             })
         }
 
+        // Desugar ExprBox: `in (PLACE) EXPR`
+        ast::ExprBox(Some(placer), value_expr) => {
+            // to:
+            //
+            // let p = PLACE;
+            // let mut place = Placer::make_place(p);
+            // let raw_place = Place::pointer(&mut place);
+            // push_unsafe!({
+            //     std::intrinsics::move_val_init(raw_place, pop_unsafe!( EXPR ));
+            //     InPlace::finalize(place)
+            // })
+
+            // Ensure feature-gate is enabled
+            feature_gate::check_for_placement_in(
+                fld.cx.ecfg.features,
+                &fld.cx.parse_sess.span_diagnostic,
+                expr_span);
+
+            push_compiler_expansion(fld, expr_span, "placement-in expansion");
+
+            let value_span = value_expr.span;
+            let placer_span = placer.span;
+
+            let placer_expr = fld.fold_expr(placer);
+            let value_expr = fld.fold_expr(value_expr);
+
+            let placer_ident = token::gensym_ident("placer");
+            let agent_ident = token::gensym_ident("place");
+            let p_ptr_ident = token::gensym_ident("p_ptr");
+
+            let placer = fld.cx.expr_ident(span, placer_ident);
+            let agent = fld.cx.expr_ident(span, agent_ident);
+            let p_ptr = fld.cx.expr_ident(span, p_ptr_ident);
+
+            let make_place = ["ops", "Placer", "make_place"];
+            let place_pointer = ["ops", "Place", "pointer"];
+            let move_val_init = ["intrinsics", "move_val_init"];
+            let inplace_finalize = ["ops", "InPlace", "finalize"];
+
+            let make_call = |fld: &mut MacroExpander, p, args| {
+                // We feed in the `expr_span` because codemap's span_allows_unstable
+                // allows the call_site span to inherit the `allow_internal_unstable`
+                // setting.
+                let span_unstable = allow_unstable(fld, expr_span);
+                let path = mk_core_path(fld, span_unstable, p);
+                let path = fld.cx.expr_path(path);
+                let expr_span_unstable = allow_unstable(fld, span);
+                fld.cx.expr_call(expr_span_unstable, path, args)
+            };
+
+            let stmt_let = |fld: &mut MacroExpander, bind, expr| {
+                fld.cx.stmt_let(placer_span, false, bind, expr)
+            };
+            let stmt_let_mut = |fld: &mut MacroExpander, bind, expr| {
+                fld.cx.stmt_let(placer_span, true, bind, expr)
+            };
+
+            // let placer = <placer_expr> ;
+            let s1 = stmt_let(fld, placer_ident, placer_expr);
+
+            // let mut place = Placer::make_place(placer);
+            let s2 = {
+                let call = make_call(fld, &make_place, vec![placer]);
+                stmt_let_mut(fld, agent_ident, call)
+            };
+
+            // let p_ptr = Place::pointer(&mut place);
+            let s3 = {
+                let args = vec![fld.cx.expr_mut_addr_of(placer_span, agent.clone())];
+                let call = make_call(fld, &place_pointer, args);
+                stmt_let(fld, p_ptr_ident, call)
+            };
+
+            // pop_unsafe!(EXPR));
+            let pop_unsafe_expr = pop_unsafe_expr(fld.cx, value_expr, value_span);
+
+            // push_unsafe!({
+            //     ptr::write(p_ptr, pop_unsafe!(<value_expr>));
+            //     InPlace::finalize(place)
+            // })
+            let expr = {
+                let call_move_val_init = StmtSemi(make_call(
+                    fld, &move_val_init, vec![p_ptr, pop_unsafe_expr]), ast::DUMMY_NODE_ID);
+                let call_move_val_init = codemap::respan(value_span, call_move_val_init);
+
+                let call = make_call(fld, &inplace_finalize, vec![agent]);
+                Some(push_unsafe_expr(fld.cx, vec![P(call_move_val_init)], call, span))
+            };
+
+            let block = fld.cx.block_all(span, vec![s1, s2, s3], expr);
+            let result = fld.cx.expr_block(block);
+            fld.cx.bt_pop();
+            result
+        }
+
+        // Issue #22181:
+        // Eventually a desugaring for `box EXPR`
+        // (similar to the desugaring above for `in PLACE BLOCK`)
+        // should go here, desugaring
+        //
+        // to:
+        //
+        // let mut place = BoxPlace::make_place();
+        // let raw_place = Place::pointer(&mut place);
+        // let value = $value;
+        // unsafe {
+        //     ::std::ptr::write(raw_place, value);
+        //     Boxed::finalize(place)
+        // }
+        //
+        // But for now there are type-inference issues doing that.
+
         ast::ExprWhile(cond, body, opt_ident) => {
             let cond = fld.fold_expr(cond);
             let (body, opt_ident) = expand_loop_block(body, opt_ident, fld);
@@ -360,7 +495,26 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
                 span: span
             }, fld))
         }
-    })
+    });
+
+    fn push_unsafe_expr(cx: &mut ExtCtxt, stmts: Vec<P<ast::Stmt>>,
+                        expr: P<ast::Expr>, span: Span)
+                        -> P<ast::Expr> {
+        let rules = ast::PushUnsafeBlock(ast::CompilerGenerated);
+        cx.expr_block(P(ast::Block {
+            rules: rules, span: span, id: ast::DUMMY_NODE_ID,
+            stmts: stmts, expr: Some(expr),
+        }))
+    }
+
+    fn pop_unsafe_expr(cx: &mut ExtCtxt, expr: P<ast::Expr>, span: Span)
+                       -> P<ast::Expr> {
+        let rules = ast::PopUnsafeBlock(ast::CompilerGenerated);
+        cx.expr_block(P(ast::Block {
+            rules: rules, span: span, id: ast::DUMMY_NODE_ID,
+            stmts: vec![], expr: Some(expr),
+        }))
+    }
 }
 
 /// Expand a (not-ident-style) macro invocation. Returns the result
@@ -1504,6 +1658,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,
     }
 }
 
diff --git a/src/libsyntax/ext/pushpop_safe.rs b/src/libsyntax/ext/pushpop_safe.rs
new file mode 100644
index 00000000000..a67d550d3cd
--- /dev/null
+++ b/src/libsyntax/ext/pushpop_safe.rs
@@ -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> {
+    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> {
+    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> {
+    feature_gate::check_for_pushpop_syntax(
+        cx.ecfg.features, &cx.parse_sess.span_diagnostic, sp);
+
+    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
+    })))
+}
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
index ab8cf9ae6b6..8c6855036f6 100644
--- a/src/libsyntax/feature_gate.rs
+++ b/src/libsyntax/feature_gate.rs
@@ -80,6 +80,8 @@ 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),
+    ("placement_in_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 +327,9 @@ pub struct Features {
     pub allow_trace_macros: bool,
     pub allow_internal_unstable: bool,
     pub allow_custom_derive: bool,
+    pub allow_placement_in: bool,
+    pub allow_box: bool,
+    pub allow_pushpop_unsafe: bool,
     pub simd_ffi: bool,
     pub unmarked_api: bool,
     pub negate_unsigned: bool,
@@ -348,6 +353,9 @@ impl Features {
             allow_trace_macros: false,
             allow_internal_unstable: false,
             allow_custom_derive: false,
+            allow_placement_in: false,
+            allow_box: false,
+            allow_pushpop_unsafe: false,
             simd_ffi: false,
             unmarked_api: false,
             negate_unsigned: false,
@@ -358,6 +366,36 @@ impl Features {
     }
 }
 
+const EXPLAIN_BOX_SYNTAX: &'static str =
+    "box expression syntax is experimental; you can call `Box::new` instead.";
+
+const EXPLAIN_PLACEMENT_IN: &'static str =
+    "placement-in expression syntax is experimental and subject to change.";
+
+const EXPLAIN_PUSHPOP_UNSAFE: &'static str =
+    "push/pop_unsafe macros are experimental and subject to change.";
+
+pub fn check_for_box_syntax(f: Option<&Features>, diag: &SpanHandler, span: Span) {
+    if let Some(&Features { allow_box: true, .. }) = f {
+        return;
+    }
+    emit_feature_err(diag, "box_syntax", span, EXPLAIN_BOX_SYNTAX);
+}
+
+pub fn check_for_placement_in(f: Option<&Features>, diag: &SpanHandler, span: Span) {
+    if let Some(&Features { allow_placement_in: true, .. }) = f {
+        return;
+    }
+    emit_feature_err(diag, "placement_in_syntax", span, EXPLAIN_PLACEMENT_IN);
+}
+
+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,
@@ -366,6 +404,11 @@ struct Context<'a> {
 }
 
 impl<'a> Context<'a> {
+    fn enable_feature(&mut self, feature: &'static str) {
+        debug!("enabling feature: {}", feature);
+        self.features.push(feature);
+    }
+
     fn gate_feature(&self, feature: &str, span: Span, explain: &str) {
         let has_feature = self.has_feature(feature);
         debug!("gate_feature(feature = {:?}, span = {:?}); has? {}", feature, span, has_feature);
@@ -488,6 +531,26 @@ impl<'a, 'v> Visitor<'v> for MacroVisitor<'a> {
     fn visit_attribute(&mut self, attr: &'v ast::Attribute) {
         self.context.check_attribute(attr, true);
     }
+
+    fn visit_expr(&mut self, e: &ast::Expr) {
+        // Issue 22181: overloaded-`box` and placement-`in` are
+        // implemented via a desugaring expansion, so their feature
+        // gates go into MacroVisitor since that works pre-expansion.
+        //
+        // Issue 22234: we also check during expansion as well.
+        // But we keep these checks as a pre-expansion check to catch
+        // uses in e.g. conditionalized code.
+
+        if let ast::ExprBox(None, _) = e.node {
+            self.context.gate_feature("box_syntax", e.span, EXPLAIN_BOX_SYNTAX);
+        }
+
+        if let ast::ExprBox(Some(_), _) = e.node {
+            self.context.gate_feature("placement_in_syntax", e.span, EXPLAIN_PLACEMENT_IN);
+        }
+
+        visit::walk_expr(self, e);
+    }
 }
 
 struct PostExpansionVisitor<'a> {
@@ -754,7 +817,7 @@ fn check_crate_inner<F>(cm: &CodeMap, span_handler: &SpanHandler,
                     match KNOWN_FEATURES.iter()
                                         .find(|& &(n, _, _)| name == n) {
                         Some(&(name, _, Active)) => {
-                            cx.features.push(name);
+                            cx.enable_feature(name);
                         }
                         Some(&(_, _, Removed)) => {
                             span_handler.span_err(mi.span, "feature has been removed");
@@ -787,6 +850,9 @@ 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_placement_in: cx.has_feature("placement_in_syntax"),
+        allow_box: cx.has_feature("box_syntax"),
+        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"),
diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs
index d93af5da13c..5424c0b214a 100644
--- a/src/libsyntax/lib.rs
+++ b/src/libsyntax/lib.rs
@@ -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;
 
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 4adfa3c3119..c2a2259a80a 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -2612,18 +2612,43 @@ impl<'a> Parser<'a> {
             ex = ExprAddrOf(m, e);
           }
           token::Ident(_, _) => {
-            if !self.check_keyword(keywords::Box) {
+            if !self.check_keyword(keywords::Box) && !self.check_keyword(keywords::In) {
                 return self.parse_dot_or_call_expr();
             }
 
             let lo = self.span.lo;
-            let box_hi = self.span.hi;
+            let keyword_hi = self.span.hi;
 
+            let is_in = self.token.is_keyword(keywords::In);
             try!(self.bump());
 
-            // Check for a place: `box(PLACE) EXPR`.
+            if is_in {
+              let place = try!(self.parse_expr_res(Restrictions::RESTRICTION_NO_STRUCT_LITERAL));
+              let blk = try!(self.parse_block());
+              hi = blk.span.hi;
+              let blk_expr = self.mk_expr(blk.span.lo, blk.span.hi, ExprBlock(blk));
+              ex = ExprBox(Some(place), blk_expr);
+              return Ok(self.mk_expr(lo, hi, ex));
+            }
+
+            // FIXME (#22181) Remove `box (PLACE) EXPR` support
+            // entirely after next release (enabling `(box (EXPR))`),
+            // since it will be replaced by `in PLACE { EXPR }`, ...
+            //
+            // ... but for now: check for a place: `box(PLACE) EXPR`.
+
             if try!(self.eat(&token::OpenDelim(token::Paren)) ){
-                // Support `box() EXPR` as the default.
+                // SNAP d4432b3
+                // Enable this warning after snapshot ...
+                //
+                // let box_span = mk_sp(lo, self.last_span.hi);
+                // self.span_warn(
+                //     box_span,
+                //     "deprecated syntax; use the `in` keyword now \
+                //            (e.g. change `box (<expr>) <expr>` to \
+                //                         `in <expr> { <expr> }`)");
+
+                // Continue supporting `box () EXPR` (temporarily)
                 if !try!(self.eat(&token::CloseDelim(token::Paren)) ){
                     let place = try!(self.parse_expr_nopanic());
                     try!(self.expect(&token::CloseDelim(token::Paren)));
@@ -2634,10 +2659,15 @@ impl<'a> Parser<'a> {
                         self.span_err(span,
                                       &format!("expected expression, found `{}`",
                                               this_token_to_string));
-                        let box_span = mk_sp(lo, box_hi);
+
+                        // Spanning just keyword avoids constructing
+                        // printout of arg expression (which starts
+                        // with parenthesis, as established above).
+
+                        let box_span = mk_sp(lo, keyword_hi);
                         self.span_suggestion(box_span,
-                                             "try using `box()` instead:",
-                                             "box()".to_string());
+                                             "try using `box ()` instead:",
+                                             format!("box ()"));
                         self.abort_if_errors();
                     }
                     let subexpression = try!(self.parse_prefix_expr());
@@ -2650,6 +2680,7 @@ impl<'a> Parser<'a> {
             // Otherwise, we use the unique pointer default.
             let subexpression = try!(self.parse_prefix_expr());
             hi = subexpression.span.hi;
+
             // FIXME (pnkfelix): After working out kinks with box
             // desugaring, should be `ExprBox(None, subexpression)`
             // instead.
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index 6693eed6ace..448857389da 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -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)));