Skip to content

Commit

Permalink
Change the format_args! macro expansion for temporaries
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
edwardw committed Feb 19, 2014
1 parent c4afcf4 commit 111e092
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/librustc/middle/ty.rs
Expand Up @@ -4142,7 +4142,7 @@ pub fn enum_variants(cx: ctxt, id: ast::DefId) -> @~[@VariantInfo] {
.span_err(e.span,
format!("expected \
constant: {}",
(*err)));
*err));
}
},
None => {}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/typeck/check/method.rs
Expand Up @@ -1303,15 +1303,15 @@ impl<'a> LookupContext<'a> {
self.tcx().sess.span_note(
span,
format!("candidate \\#{} is `{}`",
(idx+1u),
idx+1u,
ty::item_path_str(self.tcx(), did)));
}

fn report_param_candidate(&self, idx: uint, did: DefId) {
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)));
}

Expand All @@ -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)));
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/check/mod.rs
Expand Up @@ -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));
}
}
},
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/util/ppaux.rs
Expand Up @@ -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));
}
}
}
Expand Down
46 changes: 40 additions & 6 deletions src/libsyntax/ext/format.rs
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)));
}
Expand All @@ -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()),
Expand Down Expand Up @@ -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)
Expand Down
18 changes: 18 additions & 0 deletions 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 <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.

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\"");
}

5 comments on commit 111e092

@bors
Copy link
Contributor

@bors bors commented on 111e092 Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from huonw
at edwardw@111e092

@bors
Copy link
Contributor

@bors bors commented on 111e092 Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging edwardw/rust/debug-expansion = 111e092 into auto

@bors
Copy link
Contributor

@bors bors commented on 111e092 Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edwardw/rust/debug-expansion = 111e092 merged ok, testing candidate = ace204a

@bors
Copy link
Contributor

@bors bors commented on 111e092 Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 111e092 Feb 19, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = ace204a

Please sign in to comment.