Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NLL issue 50716 and add a regression test. #51139

Merged
merged 4 commits into from
Jun 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,12 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.def_key(def_id).disambiguated_data.data == DefPathData::ClosureExpr
}

/// True if this def-id refers to the implicit constructor for
/// a tuple struct like `struct Foo(u32)`.
pub fn is_struct_constructor(self, def_id: DefId) -> bool {
self.def_key(def_id).disambiguated_data.data == DefPathData::StructCtor
}

/// Given the `DefId` of a fn or closure, returns the `DefId` of
/// the innermost fn item that the closure is contained within.
/// This is a significant def-id because, when we do
Expand Down
32 changes: 31 additions & 1 deletion src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,37 @@ fn mir_borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> BorrowC
let input_mir = tcx.mir_validated(def_id);
debug!("run query mir_borrowck: {}", tcx.item_path_str(def_id));

if !tcx.has_attr(def_id, "rustc_mir_borrowck") && !tcx.use_mir_borrowck() {
let mut return_early;

// Return early if we are not supposed to use MIR borrow checker for this function.
return_early = !tcx.has_attr(def_id, "rustc_mir_borrowck") && !tcx.use_mir_borrowck();

if tcx.is_struct_constructor(def_id) {
// We are not borrow checking the automatically generated struct constructors
// because we want to accept structs such as this (taken from the `linked-hash-map`
// crate):
// ```rust
// struct Qey<Q: ?Sized>(Q);
// ```
// MIR of this struct constructor looks something like this:
// ```rust
// fn Qey(_1: Q) -> Qey<Q>{
// let mut _0: Qey<Q>; // return place
//
// bb0: {
// (_0.0: Q) = move _1; // bb0[0]: scope 0 at src/main.rs:1:1: 1:26
// return; // bb0[1]: scope 0 at src/main.rs:1:1: 1:26
// }
// }
// ```
// The problem here is that `(_0.0: Q) = move _1;` is valid only if `Q` is
// of statically known size, which is not known to be true because of the
// `Q: ?Sized` constraint. However, it is true because the constructor can be
// called only when `Q` is of statically known size.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice comment =)

return_early = true;
}

if return_early {
return BorrowCheckResult {
closure_requirements: None,
used_mut_upvars: SmallVec::new(),
Expand Down
13 changes: 13 additions & 0 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,11 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
);
}
self.check_rvalue(mir, rv, location);
let trait_ref = ty::TraitRef {
def_id: tcx.lang_items().sized_trait().unwrap(),
substs: tcx.mk_substs_trait(place_ty, &[]),
};
self.prove_trait_ref(trait_ref, location);
}
StatementKind::SetDiscriminant {
ref place,
Expand Down Expand Up @@ -1720,6 +1725,14 @@ impl MirPass for TypeckMir {
// broken MIR, so try not to report duplicate errors.
return;
}

if tcx.is_struct_constructor(def_id) {
// We just assume that the automatically generated struct constructors are
// correct. See the comment in the `mir_borrowck` implementation for an
// explanation why we need this.
return;
}

let param_env = tcx.param_env(def_id);
tcx.infer_ctxt().enter(|infcx| {
let _ = type_check_internal(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ LL | | });
= note: where '_#1r: '_#0r

error: free region `ReFree(DefId(0/0:6 ~ propagate_approximated_shorter_to_static_no_bound[317d]::supply[0]), BrNamed(crate0:DefIndex(1:16), 'a))` does not outlive free region `ReStatic`
--> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:45:5
--> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:45:47
|
LL | / establish_relationships(&cell_a, &cell_b, |_outlives, x, y| {
LL | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| {
| _______________________________________________^
LL | | //~^ ERROR does not outlive free region
LL | |
LL | | // Only works if 'x: 'y:
LL | | demand_y(x, y, x.get()) //~ WARNING not reporting region error due to nll
LL | | });
| |______^
| |_____^

note: No external requirements
--> $DIR/propagate-approximated-shorter-to-static-no-bound.rs:44:1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ LL | | });
= note: where '_#1r: '_#0r

error: free region `ReFree(DefId(0/0:6 ~ propagate_approximated_shorter_to_static_wrong_bound[317d]::supply[0]), BrNamed(crate0:DefIndex(1:16), 'a))` does not outlive free region `ReStatic`
--> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:48:5
--> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:48:47
|
LL | / establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| {
LL | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| {
| _______________________________________________^
LL | | //~^ ERROR does not outlive free region
LL | | // Only works if 'x: 'y:
LL | | demand_y(x, y, x.get())
LL | | //~^ WARNING not reporting region error due to nll
LL | | });
| |______^
| |_____^

note: No external requirements
--> $DIR/propagate-approximated-shorter-to-static-wrong-bound.rs:47:1
Expand Down
23 changes: 23 additions & 0 deletions src/test/ui/nll/issue-50716-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2012 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.
//
// An additional regression test for the issue #50716 “NLL ignores lifetimes
// bounds derived from `Sized` requirements” that checks that the fixed compiler
// accepts this code fragment with both AST and MIR borrow checkers.
//
// revisions: ast mir
//
// compile-pass

#![cfg_attr(mir, feature(nll))]

struct Qey<Q: ?Sized>(Q);

fn main() {}
28 changes: 28 additions & 0 deletions src/test/ui/nll/issue-50716.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2012 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.
//
// Regression test for the issue #50716: NLL ignores lifetimes bounds
// derived from `Sized` requirements

#![feature(nll)]

trait A {
type X: ?Sized;
}

fn foo<'a, T: 'static>(s: Box<<&'a T as A>::X>)
where
for<'b> &'b T: A,
<&'static T as A>::X: Sized
{
let _x = *s; //~ ERROR free region `'a` does not outlive free region `'static`
}

fn main() {}
8 changes: 8 additions & 0 deletions src/test/ui/nll/issue-50716.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: free region `'a` does not outlive free region `'static`
--> $DIR/issue-50716.rs:25:14
|
LL | let _x = *s; //~ ERROR free region `'a` does not outlive free region `'static`
| ^^

error: aborting due to previous error