From 111e092481ec8a6e958f3f4d3e13eaccd844d99e Mon Sep 17 00:00:00 2001 From: Edward Wang Date: Tue, 18 Feb 2014 03:32:12 +0800 Subject: [PATCH] Change the format_args! macro expansion for temporaries Currently, the format_args! macro and its downstream macros in turn expand to series of let statements, one for each of its arguments, and then the invocation of the macro function. If one or more of the arguments are RefCell's, the enclosing statement for the temporary of the let is the let itself, which leads to scope problem. This patch changes let's to a match expression. Closes #12239. --- src/librustc/middle/ty.rs | 2 +- src/librustc/middle/typeck/check/method.rs | 6 +-- src/librustc/middle/typeck/check/mod.rs | 2 +- src/librustc/util/ppaux.rs | 4 +- src/libsyntax/ext/format.rs | 46 +++++++++++++++++++--- src/test/run-pass/format-ref-cell.rs | 18 +++++++++ 6 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 src/test/run-pass/format-ref-cell.rs diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 0dcbfe491c3..5bc3d047c9f 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -4142,7 +4142,7 @@ pub fn enum_variants(cx: ctxt, id: ast::DefId) -> @~[@VariantInfo] { .span_err(e.span, format!("expected \ constant: {}", - (*err))); + *err)); } }, None => {} diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs index 21877bda1c1..9c43af4f13a 100644 --- a/src/librustc/middle/typeck/check/method.rs +++ b/src/librustc/middle/typeck/check/method.rs @@ -1303,7 +1303,7 @@ impl<'a> LookupContext<'a> { self.tcx().sess.span_note( span, format!("candidate \\#{} is `{}`", - (idx+1u), + idx+1u, ty::item_path_str(self.tcx(), did))); } @@ -1311,7 +1311,7 @@ impl<'a> LookupContext<'a> { self.tcx().sess.span_note( self.expr.span, format!("candidate \\#{} derives from the bound `{}`", - (idx+1u), + idx+1u, ty::item_path_str(self.tcx(), did))); } @@ -1320,7 +1320,7 @@ impl<'a> LookupContext<'a> { self.expr.span, format!("candidate \\#{} derives from the type of the receiver, \ which is the trait `{}`", - (idx+1u), + idx+1u, ty::item_path_str(self.tcx(), did))); } diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs index 177eba5aa1b..eab34b56060 100644 --- a/src/librustc/middle/typeck/check/mod.rs +++ b/src/librustc/middle/typeck/check/mod.rs @@ -3551,7 +3551,7 @@ pub fn check_enum_variants(ccx: @CrateCtxt, ccx.tcx.sess.span_err(e.span, "expected signed integer constant"); } Err(ref err) => { - ccx.tcx.sess.span_err(e.span, format!("expected constant: {}", (*err))); + ccx.tcx.sess.span_err(e.span, format!("expected constant: {}", *err)); } } }, diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index dd270885b0e..4608fa649b5 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -49,11 +49,11 @@ pub fn note_and_explain_region(cx: ctxt, (ref str, Some(span)) => { cx.sess.span_note( span, - format!("{}{}{}", prefix, (*str), suffix)); + format!("{}{}{}", prefix, *str, suffix)); } (ref str, None) => { cx.sess.note( - format!("{}{}{}", prefix, (*str), suffix)); + format!("{}{}{}", prefix, *str, suffix)); } } } diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index 01d348595b8..db81731a476 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -607,6 +607,8 @@ impl<'a> Context<'a> { let mut lets = ~[]; let mut locals = ~[]; let mut names = vec::from_fn(self.name_positions.len(), |_| None); + let mut pats = ~[]; + let mut heads = ~[]; // First, declare all of our methods that are statics for &method in self.method_statics.iter() { @@ -653,8 +655,8 @@ impl<'a> Context<'a> { if self.arg_types[i].is_none() { continue } // error already generated let name = self.ecx.ident_of(format!("__arg{}", i)); - let e = self.ecx.expr_addr_of(e.span, e); - lets.push(self.ecx.stmt_let(e.span, false, name, e)); + pats.push(self.ecx.pat_ident(e.span, name)); + heads.push(self.ecx.expr_addr_of(e.span, e)); locals.push(self.format_arg(e.span, Exact(i), self.ecx.expr_ident(e.span, name))); } @@ -664,8 +666,8 @@ impl<'a> Context<'a> { } let lname = self.ecx.ident_of(format!("__arg{}", *name)); - let e = self.ecx.expr_addr_of(e.span, e); - lets.push(self.ecx.stmt_let(e.span, false, lname, e)); + pats.push(self.ecx.pat_ident(e.span, lname)); + heads.push(self.ecx.expr_addr_of(e.span, e)); names[*self.name_positions.get(name)] = Some(self.format_arg(e.span, Named((*name).clone()), @@ -706,8 +708,40 @@ impl<'a> Context<'a> { let res = self.ecx.expr_ident(self.fmtsp, resname); let result = self.ecx.expr_call(extra.span, extra, ~[ self.ecx.expr_addr_of(extra.span, res)]); - self.ecx.expr_block(self.ecx.block(self.fmtsp, lets, - Some(result))) + let body = self.ecx.expr_block(self.ecx.block(self.fmtsp, lets, + Some(result))); + + // Constructs an AST equivalent to: + // + // match (&arg0, &arg1) { + // (tmp0, tmp1) => body + // } + // + // It was: + // + // let tmp0 = &arg0; + // let tmp1 = &arg1; + // body + // + // Because of #11585 the new temporary lifetime rule, the enclosing + // statements for these temporaries become the let's themselves. + // If one or more of them are RefCell's, RefCell borrow() will also + // end there; they don't last long enough for body to use them. The + // match expression solves the scope problem. + // + // Note, it may also very well be transformed to: + // + // match arg0 { + // ref tmp0 => { + // match arg1 => { + // ref tmp1 => body } } } + // + // But the nested match expression is proved to perform not as well + // as series of let's; the first approach does. + let pat = self.ecx.pat(self.fmtsp, ast::PatTup(pats)); + let arm = self.ecx.arm(self.fmtsp, ~[pat], body); + let head = self.ecx.expr(self.fmtsp, ast::ExprTup(heads)); + self.ecx.expr_match(self.fmtsp, head, ~[arm]) } fn format_arg(&self, sp: Span, argno: Position, arg: @ast::Expr) diff --git a/src/test/run-pass/format-ref-cell.rs b/src/test/run-pass/format-ref-cell.rs new file mode 100644 index 00000000000..6428d496382 --- /dev/null +++ b/src/test/run-pass/format-ref-cell.rs @@ -0,0 +1,18 @@ +// Copyright 2012-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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::cell::RefCell; + +pub fn main() { + let name = RefCell::new("rust"); + let what = RefCell::new("rocks"); + let msg = format!("{name:?} {:?}", what.borrow().get(), name=name.borrow().get()); + assert_eq!(msg, ~"&\"rust\" &\"rocks\""); +}