Implements this (previously overlooked) note from [RFC 769]:
> (Note: When encountering a D of the form `Box<Trait+'b>`, we
> conservatively assume that such a type has a Drop implementation
> parametric in 'b.)
Fix#25199.
[breaking-change]
The breakage here falls into both obvious and non-obvious cases.
The obvious case: if you were relying on the unsoundness this exposes
(namely being able to reference dead storage from a destructor, by
doing it via a boxed trait object bounded by the lifetime of the dead
storage), then this change disallows that.
The non-obvious cases: The way dropck works, it causes lifetimes to be
extended to longer extents than they covered before. I.e. lifetimes
that are attached as trait-bounds may become longer than they were
previously.
* This includes lifetimes that are only *implicitly* attached as
trait-bounds (due to [RFC 599]). So you may have code that was
e.g. taking a parameter of type `&'a Box<Trait>` (which expands to
`&'a Box<Trait+'a>`), that now may need to be assigned type `&'a
Box<Trait+'static>` to ensure that `'a` is not inadvertantly
inferred to a region that is actually too long. (See earlier commit
in this PR for an example of this.)
[RFC 769]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#the-drop-check-rule
[RFC 599]: https://github.com/rust-lang/rfcs/blob/master/text/0599-default-object-bound.md
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 #3511 and PR
#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
If you have 0 references (`&T`) to a resource, presumably, you could
have a mutable reference (`&mut T`). So this only start to make sense
at having 1 reference to a resource.
Ideally this trait implementation would be unstable, requiring crates to opt-in
if they would like the functionality, but that's not currently how stability
works so the implementation needs to be removed entirely.
This may come back at a future date, but for now the conservative option is to
remove it.
[breaking-change]
My change in #24797 had a bug, described in that issue’s comments, and first discovered in issue #24918. This fixes it.
I tested this new `main.js` by changing the `main.js` content of [a rendered docs page](https://doc.rust-lang.org/std/option/) to this new content. The ‘[−]’ button worked again.
I am also including another related fix, because it would require manual merging if I made a separate pull request for it. The page-global ‘[−]’ button currently adds `#` to the end of the URL whenever it is clicked. I am changing its `href` from `#` to `javascript:void(0)` (the same as the `href` for section-specific ‘[−]’ links) to fix that.
Now that `std::old_io` has been removed for quite some time the naming real
estate here has opened up to allow these modules to move back to their proper
names.
Guard against overflow in `codemap::span_to_lines`.
(Revised/expanded version of PR #24976)
Make `span_to_lines` to return a `Result`.
In `diagnostic`, catch `Err` from `span_to_lines` and print `"(unprintable span)"` instead.
----
There a number of recent issues that report the bug here. See e.g. #24761 and #24954.
This change *might* fix them. However, that is *not* its main goal. The main goals are:
1. Make it possible for callers to recover from an error here, and
2. Insert a more conservative check, in that we are also checking that the files match up.
----
As a drive-by, fix#24997 , which was causing my attempts to `make check-stage1` on an `--enable-debug` build to fail.
The error message was misleading, so I adjusted it, and I also added the long diagnostics for this error (resolves one point in #24407).
I was unsure about how to phrase the error message. Is “generic parameter binding” the correct term for this?
