Skip to content

Commit

Permalink
Fallout from fixing Issue 25199.
Browse files Browse the repository at this point in the history
There are two interesting kinds of breakage illustrated here:

1. `Box<Trait>` in many contexts is treated as `Box<Trait + 'static>`,
   due to [RFC 599]. However, in a type like `&'a Box<Trait>`, the
   `Box<Trait>` type will be expanded to `Box<Trait + 'a>`, again due
   to [RFC 599]. This, combined with the fix to Issue 25199, leads to
   a borrowck problem due the combination of this function signature
   (in src/libstd/net/parser.rs):

   ```rust
   fn read_or<T>(&mut self, parsers: &mut [Box<FnMut(&mut Parser) -> Option<T>>]) -> Option<T>;
   ```

   with this call site (again in src/libstd/net/parser.rs):

   ```rust
   fn read_ip_addr(&mut self) -> Option<IpAddr> {
       let ipv4_addr = |p: &mut Parser| p.read_ipv4_addr().map(|v4| IpAddr::V4(v4));
       let ipv6_addr = |p: &mut Parser| p.read_ipv6_addr().map(|v6| IpAddr::V6(v6));
       self.read_or(&mut [Box::new(ipv4_addr), Box::new(ipv6_addr)])
   }
   ```

   yielding borrowck errors like:

   ```
   parser.rs:265:27: 265:69 error: borrowed value does not live long enough
   parser.rs:265         self.read_or(&mut [Box::new(ipv4_addr), Box::new(ipv6_addr)])
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   ```

   (full log at: https://gist.github.com/pnkfelix/e2e80f1a71580f5d3103 )

   The issue here is perhaps subtle: the `parsers` argument is
   inferred to be taking a slice of boxed objects with the implicit
   lifetime bound attached to the `self` parameter to `read_or`.

   Meanwhile, the fix to Issue 25199 (added in a forth-coming commit)
   is forcing us to assume that each boxed object may have a
   destructor that could refer to state of that lifetime, and
   *therefore* that inferred lifetime is required to outlive the boxed
   object itself.

   In this case, the relevant boxed object here is not going to make
   any such references; I believe it is just an artifact of how the
   expression was built that it is not assigned type:

     `Box<FnMut(&mut Parser) -> Option<T> + 'static>`.

   (i.e., mucking with the expression is probably one way to fix this
   problem).

   But the other way to fix it, adopted here, is to change the
   `read_or` method type to force make the (presumably-intended)
   `'static` bound explicit on the boxed `FnMut` object.

   (Note: this is still just the *first* example of breakage.)

2. In `macro_rules.rs`, the `TTMacroExpander` trait defines a method
   with signature:

   ```rust
   fn expand<'cx>(&self, cx: &'cx mut ExtCtxt, ...) -> Box<MacResult+'cx>;
   ```

   taking a `&'cx mut ExtCtxt` as an argument and returning a
   `Box<MacResult'cx>`.

   The fix to Issue 25199 (added in aforementioned forth-coming
   commit) assumes that a value of type `Box<MacResult+'cx>` may, in
   its destructor, refer to a reference of lifetime `'cx`; thus the
   `'cx` lifetime is forced to outlive the returned value.

   Meanwhile, within `expand.rs`, the old code was doing:

   ```rust
   match expander.expand(fld.cx, ...).make_pat() { ... => immutable borrow of fld.cx ... }
   ```

   The problem is that the `'cx` lifetime, inferred for the
   `expander.expand` call, has now been extended so that it has to
   outlive the temporary R-value returned by `expanded.expand`.  But
   call is also reborrowing `fld.cx` *mutably*, which means that this
   reborrow must end before any immutable borrow of `fld.cx`; but
   there is one of those within the match body. (Note that the
   temporary R-values for the input expression to `match` all live as
   long as the whole `match` expression itself (see Issue rust-lang#3511 and PR
   rust-lang#11585).

   To address this, I moved the construction of the pat value into its
   own `let`-statement, so that the `Box<MacResult>` will only live
   for as long as the initializing expression for the `let`-statement,
   and thus allow the subsequent immutable borrow within the `match`.

[RFC 599]: https://github.com/rust-lang/rfcs/blob/master/text/0599-default-object-bound.md
  • Loading branch information
pnkfelix committed May 8, 2015
1 parent b402c43 commit ee06263
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/libstd/net/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<'a> Parser<'a> {
}

// Return result of first successful parser
fn read_or<T>(&mut self, parsers: &mut [Box<FnMut(&mut Parser) -> Option<T>>])
fn read_or<T>(&mut self, parsers: &mut [Box<FnMut(&mut Parser) -> Option<T> + 'static>])
-> Option<T> {
for pf in parsers.iter_mut() {
match self.read_atomically(|p: &mut Parser| pf(p)) {
Expand Down
7 changes: 4 additions & 3 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,9 +986,10 @@ fn expand_pat(p: P<ast::Pat>, fld: &mut MacroExpander) -> P<ast::Pat> {
let fm = fresh_mark();
let marked_before = mark_tts(&tts[..], fm);
let mac_span = fld.cx.original_span();
let expanded = match expander.expand(fld.cx,
mac_span,
&marked_before[..]).make_pat() {
let pat = expander.expand(fld.cx,
mac_span,
&marked_before[..]).make_pat();
let expanded = match pat {
Some(e) => e,
None => {
fld.cx.span_err(
Expand Down

0 comments on commit ee06263

Please sign in to comment.