NLL: Review compile-fail migrated tests

[davidtwco]

As part of #53196, compile-fail tests were migrated to ui tests. In doing this, a handful of tests compile successfully in the NLL compare mode, but not in the normal AST compare mode. These tests were marked as "ignore on NLL compare mode" as there isn't an easy way to assert that a test compiles successfully in one compare mode and not in another. These tests are listed below and should be checked to make sure that it is correct that these tests ran successfully in NLL compare mode or if they have a bug:

(exact locations of tests have changed)

borrowck/

• ui/compile-fail-migration/borrow-tuple-fields.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrow-tuple-fields.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-borrow-mut-object-twice.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-borrow-mut-object-twice.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-closures-two-mut.rs make more ported compile fail tests more robust w.r.t. NLL #53407

  rust/src/test/ui/borrowck/borrowck-closures-two-mut-fail.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

  • but also note there is

    rust/src/test/ui/borrowck/borrowck-closures-two-mut-fail.rs

    Line 17 in a573305

    // ignore-compare-mode-nll

  • and yet there is also this version which uses borrowck=compare, (a mode which I think we really should retire at this point...)

    rust/src/test/ui/borrowck/borrowck-closures-two-mut.rs

    Line 15 in a573305

    // compile-flags: -Z borrowck=compare

• ui/compile-fail-migration/borrowck/borrowck-closures-unique-imm.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-closures-unique-imm.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-closures-unique.rs make more ported compile fail tests more robust w.r.t. NLL #53407

  rust/src/test/ui/borrowck/borrowck-closures-unique.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-issue-2657-1.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-issue-2657-1.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-lend-flow-if.rs

  rust/src/test/ui/borrowck/borrowck-lend-flow-if.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-lend-flow.rs

  rust/src/test/ui/borrowck/borrowck-lend-flow.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-loan-blocks-move-cc.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-loan-blocks-move-cc.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-loan-blocks-move.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-loan-blocks-move.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-move-from-subpath-of-borrowed-path.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-move-from-subpath-of-borrowed-path.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-move-mut-base-ptr.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-move-mut-base-ptr.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-mut-borrow-of-mut-base-ptr.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-mut-borrow-of-mut-base-ptr.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-object-lifetime.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-object-lifetime.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-overloaded-index-autoderef.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-overloaded-index-autoderef.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-swap-mut-base-ptr.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-swap-mut-base-ptr.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-union-borrow-nested.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-union-borrow-nested.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/borrowck/borrowck-uniq-via-lend.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/borrowck/borrowck-uniq-via-lend.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

regions/

• ui/compile-fail-migration/regions-assoc-type-in-supertrait-outlives-container.rs

  rust/src/test/ui/regions/regions-assoc-type-in-supertrait-outlives-container.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-bounded-by-trait-requiring-static.rs

  rust/src/test/ui/regions/regions-bounded-by-trait-requiring-static.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-bounded-method-type-parameters.rs

  rust/src/test/ui/regions/regions-bounded-method-type-parameters.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-free-region-ordering-caller.rs

  rust/src/test/ui/regions/regions-free-region-ordering-caller.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-implied-bounds-projection-gap-1.rs

  rust/src/test/ui/regions/regions-implied-bounds-projection-gap-1.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-infer-contravariance-due-to-decl.rs

  rust/src/test/ui/regions/regions-infer-contravariance-due-to-decl.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-infer-covariance-due-to-decl.rs

  rust/src/test/ui/regions/regions-infer-covariance-due-to-decl.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-outlives-projection-container-wc.rs

  rust/src/test/ui/regions/regions-outlives-projection-container-wc.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-outlives-projection-container.rs

  rust/src/test/ui/regions/regions-outlives-projection-container.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-variance-contravariant-use-covariant-in-second-position.rs

  rust/src/test/ui/regions/regions-variance-contravariant-use-covariant-in-second-position.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-variance-contravariant-use-covariant.rs

  rust/src/test/ui/regions/regions-variance-contravariant-use-covariant.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-variance-covariant-use-contravariant.rs

  rust/src/test/ui/regions/regions-variance-covariant-use-contravariant.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-variance-invariant-use-contravariant.rs

  rust/src/test/ui/regions/regions-variance-invariant-use-contravariant.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/regions-variance-invariant-use-covariant.rs

  rust/src/test/ui/regions/regions-variance-invariant-use-covariant.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

variance/

• ui/compile-fail-migration/variance-cell-is-invariant.rs

  rust/src/test/ui/variance/variance-cell-is-invariant.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/variance-contravariant-arg-trait-match.rs

  rust/src/test/ui/variance/variance-contravariant-arg-trait-match.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/variance-contravariant-self-trait-match.rs

  rust/src/test/ui/variance/variance-contravariant-self-trait-match.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/variance-covariant-arg-trait-match.rs

  rust/src/test/ui/variance/variance-covariant-arg-trait-match.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/variance-covariant-self-trait-match.rs

  rust/src/test/ui/variance/variance-covariant-self-trait-match.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/variance-invariant-arg-trait-match.rs

  rust/src/test/ui/variance/variance-invariant-arg-trait-match.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/variance-invariant-self-trait-match.rs

  rust/src/test/ui/variance/variance-invariant-self-trait-match.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

other

• ui/compile-fail-migration/associated-types-subtyping-1.rs

  rust/src/test/ui/associated-types/associated-types-subtyping-1.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/cleanup-rvalue-scopes-cf.rs

  rust/src/test/ui/cleanup-rvalue-scopes-cf.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/closure_promotion.rs

  rust/src/test/ui/closure_promotion.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/hashmap-lifetimes.rs make more ported compile fail tests more robust w.r.t. NLL #53407

  rust/src/test/ui/hashmap-lifetimes.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/issue-28848.rs

  rust/src/test/ui/issues/issue-28848.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/issue-5500-1.rs#ast

  rust/src/test/ui/issues/issue-5500-1.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/method-self-arg-2.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/methods/method-self-arg-2.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/mut-cant-alias.rs Make some ported cfail tests robust w.r.t. NLL #53369

  rust/src/test/ui/mut/mut-cant-alias.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/unboxed-closure-region.rs make more ported compile fail tests more robust w.r.t. NLL #53407

  rust/src/test/ui/unboxed-closures/unboxed-closure-region.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

• ui/compile-fail-migration/unboxed-closures-borrow-conflict.rs make more ported compile fail tests more robust w.r.t. NLL #53407

  rust/src/test/ui/unboxed-closures/unboxed-closures-borrow-conflict.rs

  Line 11 in a573305

  // ignore-compare-mode-nll

[pnkfelix]

aassociated-types-subtyping-1.rs might be test weakness (in the sense of #51025), or an artifact of #47184, or both. In particular, I'm a little nervous about some of the code that is trying to encode some constraint also being clearly unreachable (due to loop { } blocks preceding them).

[pnkfelix]

I finished going through the remainder of the list.

I have distributed the entries into the following buckets:

Glaring test weakness (in the sense of #51025); DONE: see PR #53369

For these, the test presumes lexical lifetimes (e.g. it writes let _w = &stuff; or let _w = &mut stuff; early on and assumes that borrows lasts until the end of the block). To fix this for NLL, we usually need to add one or more uses of some borrowed variable. (The use of a leading underscore on such variables is often a dead giveaway...) I've tried to link to the first such occurrence I noticed in the file.

rust/src/test/ui/borrowck/borrow-tuple-fields.rs

Line 21 in a573305

let r = &x.0;

rust/src/test/ui/borrowck/borrowck-borrow-mut-object-twice.rs

Line 22 in a573305

let _y = x.f1();

rust/src/test/ui/borrowck/borrowck-closures-unique-imm.rs

Line 22 in a573305

let p = &this.x;

rust/src/test/ui/borrowck/borrowck-issue-2657-1.rs

Line 18 in a573305

Some(ref _y) => {

rust/src/test/ui/borrowck/borrowck-loan-blocks-move-cc.rs

Line 23 in a573305

let _w = &v;

rust/src/test/ui/borrowck/borrowck-loan-blocks-move.rs

Line 20 in a573305

let _w = &v;

rust/src/test/ui/borrowck/borrowck-move-from-subpath-of-borrowed-path.rs

Line 20 in a573305

let b = &a;

rust/src/test/ui/borrowck/borrowck-move-mut-base-ptr.rs

Line 19 in a573305

let p: &isize = &*t0; // Freezes `*t0`

rust/src/test/ui/borrowck/borrowck-mut-borrow-of-mut-base-ptr.rs

Line 20 in a573305

let p: &isize = &*t0; // Freezes `*t0`

rust/src/test/ui/borrowck/borrowck-object-lifetime.rs

Line 22 in a573305

let _y = x.borrowed();

rust/src/test/ui/borrowck/borrowck-overloaded-index-autoderef.rs

Line 46 in a573305

let _p = &mut f[&s];

rust/src/test/ui/borrowck/borrowck-swap-mut-base-ptr.rs

Line 22 in a573305

let p: &isize = &*t0; // Freezes `*t0`

rust/src/test/ui/borrowck/borrowck-union-borrow-nested.rs

Line 29 in a573305

let ra = &mut u.s.a;

rust/src/test/ui/borrowck/borrowck-uniq-via-lend.rs

Line 44 in a573305

let _w = &mut v;

rust/src/test/ui/methods/method-self-arg-2.rs

Line 24 in a573305

let y = &mut x;

rust/src/test/ui/mut/mut-cant-alias.rs

Line 18 in a573305

let b1 = &mut *b;

Possible test weakness #51025 (e.g., do we need to add some calls of the closures or object methods, or make code reachable?); (almost done, see PR #53407)

Update: adding calls is not needed; it suffices to merely add (no-op) references to the variables that hold the first closure immediately after the second one is defined, and you see the error then.

Update 2: I've covered all the cases except associated-types-subtyping-1.rs in PR #53407

rust/src/test/ui/associated-types/associated-types-subtyping-1.rs

Line 25 in a573305

let a: <T as Trait<'a>>::Type = loop { };

rust/src/test/ui/borrowck/borrowck-closures-two-mut-fail.rs

Line 25 in a573305

let c1 = to_fn_mut(|| x = 4);

rust/src/test/ui/borrowck/borrowck-closures-unique.rs

Line 33 in a573305

let c1 = || get(x);

rust/src/test/ui/hashmap-lifetimes.rs

Line 17 in a573305

let mut it = my_stuff.iter();

rust/src/test/ui/unboxed-closures/unboxed-closure-region.rs

Line 16 in a573305

let _f = {

rust/src/test/ui/unboxed-closures/unboxed-closures-borrow-conflict.rs

Line 18 in a573305

let f = || x += 1;

Fixed (at least somewhat) by NLL

For these cases, we should just add a #[rustc_error] to the fn main() (assuming the existing fn main() is trivial; if the fn main is not trivial, then rename it and add a trivial fn main)

rust/src/test/ui/borrowck/borrowck-lend-flow-if.rs

Lines 31 to 41 in a573305

	fn pre_freeze_cond() {
	// In this instance, the freeze is conditional and starts before
	// the mut borrow.
	let mut v: Box<_> = box 3;
	let _w;
	if cond() {
	_w = &v;
	}
	borrow_mut(&mut *v); //~ ERROR cannot borrow
	}

rust/src/test/ui/borrowck/borrowck-lend-flow.rs

Lines 31 to 37 in a573305

	fn pre_freeze() {
	// In this instance, the freeze starts before the mut borrow.
	let mut v: Box<_> = box 3;
	let _w = &v;
	borrow_mut(&mut *v); //~ ERROR cannot borrow
	}

Have our rvalue lifetime rules changed...?

• Update The reason this test is passing under NLL is almost certainly due to NLL allows creating variables that are immediately unusable. #53695

I skimmed over this test but I cannot immediately tell whether NLL should have changed anything here... I'll need to read it again with a fresh pair of eyes (and perhaps review some of the doc and/or blog posts discussing our rvalue lifetime rules...)

rust/src/test/ui/cleanup-rvalue-scopes-cf.rs

Lines 37 to 45 in a573305

	pub fn main() {
	let _x = arg(&AddFlags(1)); //~ ERROR value does not live long enough
	let _x = AddFlags(1).get(); //~ ERROR value does not live long enough
	let _x = &*arg(&AddFlags(1)); //~ ERROR value does not live long enough
	let ref _x = *arg(&AddFlags(1)); //~ ERROR value does not live long enough
	let &ref _x = arg(&AddFlags(1)); //~ ERROR value does not live long enough
	let _x = AddFlags(1).get(); //~ ERROR value does not live long enough
	let Box { f: _x } = Box { f: AddFlags(1).get() }; //~ ERROR value does not live long enough
	}

Known bug

These cases all fall into some form of #47184

I link to the first obvious example of some type ascription that is currently unchecked:

rust/src/test/ui/associated-types/associated-types-subtyping-1.rs

Line 27 in a573305

let _c: <T as Trait<'a>>::Type = a;

rust/src/test/ui/closure_promotion.rs

Line 18 in a573305

let x: &'static _ = &|| { let z = 3; z }; //~ ERROR does not live long enough

rust/src/test/ui/issues/issue-28848.rs

Line 22 in a573305

Foo::<'a, 'b>::xmute(u) //~ ERROR lifetime bound not satisfied

Finally, all of the tests in regions/ and all of the tests in variance/ use type ascription (in arguably advanced ways) as a way to encode the desired (potentially subtle checks).

I think all of these should be fixed when #47184 is fixed, or potentially sooner as subtasks of that bug are completed.

Already using -Z borrowck=XXX in a different revision.

In this case, the test is already requesting some form of MIR-borrowck via explicit compiler flags and presumably does not need to go through the compare-mode=nll system. HOWEVER, there are instances of this in the test suite where the only reason we used the revision system and compiler flags is that those tests predate the compare-mode=nll. So we should consider trying to update them anyway, if possible.

rust/src/test/ui/issues/issue-5500-1.rs

Line 13 in a573305

//[mir]compile-flags: -Z borrowck=compare

[pnkfelix]

There isn't that much left to do here in the short term; IMO all of the regions/ and variance/ are effectively blocked on #47184 -- or at least, I do not think we should invest any effort trying to find a different way to encode those tests. All such effort should be spent instead on addressing #47184, and then once that is addressed, we should be able to remove the // ignore-test-compare-nll on those files with relative ease.

I think once you remove those files and the others blocked by #47184, then there's only maybe nine or tests left to be either investigated or updated.

[pnkfelix]

(having said that, I'm unassigning myself since I am not sure I will have time during the remainder of this week to investigate the remaining tests here.)