Consider alias bounds when computing liveness in NLL (but this time sound hopefully) #116733

compiler-errors · 2023-10-14T15:06:37Z

This is a revival of #116040, except removing the changes to opaque lifetime captures check to make sure that we're not triggering any unsoundness due to the lack of general existential regions and the currently-existing ReErased hack we use instead.

r? @aliemjay -- I appreciate you pointing out the unsoundenss in the previous iteration of this PR, and I'd like to hear that you're happy with this iteration of this PR before this goes back into FCP :>

Fixes #116794 as well

(mostly copied from #116040 and reworked slightly)

Background

Right now, liveness analysis in NLL is a bit simplistic. It simply walks through all of the regions of a type and marks them as being live at points. This is problematic in the case of aliases, since it requires that we mark all of the regions in their args¹ as live, leading to bugs like #42940.

In reality, we may be able to deduce that fewer regions are allowed to be present in the projected type (or "hidden type" for opaques) via item bounds or where clauses, and therefore ideally, we should be able to soundly require fewer regions to be live in the alias.

For example:

trait Captures<'a> {}
impl<T> Captures<'_> for T {}

fn capture<'o>(_: &'o mut ()) -> impl Sized + Captures<'o> + 'static {}

fn test_two_mut(mut x: ()) {
    let _f1 = capture(&mut x);
    let _f2 = capture(&mut x);
    //~^ ERROR cannot borrow `x` as mutable more than once at a time
}

In the example above, we should be able to deduce from the 'static bound on capture's opaque that even though 'o is a captured region, it can never show up in the opaque's hidden type, and can soundly be ignored for liveness purposes.

The Fix

We apply a simple version of RFC 1214's OutlivesProjectionEnv and OutlivesProjectionTraitDef rules to NLL's make_all_regions_live computation.

Specifically, when we encounter an alias type, we:

Look for a unique outlives bound in the param-env or item bounds for that alias. If there is more than one unique region, bail, unless any of the outlives bound's regions is 'static, and in that case, prefer 'static. If we find such a unique region, we can mark that outlives region as live and skip walking through the args of the opaque.
Otherwise, walk through the alias's args recursively, as we do today.

Limitation: Multiple choices

This approach has some limitations. Firstly, since liveness doesn't use the same type-test logic as outlives bounds do, we can't really try several options when we're faced with a choice.

If we encounter two unique outlives regions in the param-env or bounds, we simply fall back to walking the opaque via its args. I expect this to be mostly mitigated by the special treatment of 'static, and can be fixed in a forwards-compatible by a more sophisticated analysis in the future.

Limitation: Opaque hidden types

Secondly, we do not employ any of these rules when considering whether the regions captured by a hidden type are valid. That causes this code (cc #42940) to fail:

trait Captures<'a> {}
impl<T> Captures<'_> for T {}

fn a() -> impl Sized + 'static {
    b(&vec![])
}

fn b<'o>(_: &'o Vec<i32>) -> impl Sized + Captures<'o> + 'static {}

We need to have existential regions to avoid unsoundness when an opaque captures a region which is not represented in its own substs but which outlives a region that does.

Pkgsrc changes: * Adjust patches and cargo checksums to new versions. * For an external LLVM, set dependency of llvm >= 16, in accordance with the upstream changes. * Mark that on NetBSD we now need >= 9.0, so 8.x is no longer supported. * On NetBSD/sparc64 10.x, we now need GCC 12 to build the embedded LLVM, which is version 17; apparently GCC 10.4 or 10.5 mis-compiles it, resulting in an illegal instruction fault during the build. Ref. rust-lang/rust#117231 Upstream changes: Version 1.75.0 (2023-12-28) ========================== - [Stabilize `async fn` and return-position `impl Trait` in traits.] (rust-lang/rust#115822) - [Allow function pointer signatures containing `&mut T` in `const` contexts.] (rust-lang/rust#116015) - [Match `usize`/`isize` exhaustively with half-open ranges.] (rust-lang/rust#116692) - [Guarantee that `char` has the same size and alignment as `u32`.] (rust-lang/rust#116894) - [Document that the null pointer has the 0 address.] (rust-lang/rust#116988) - [Allow partially moved values in `match`.] (rust-lang/rust#103208) - [Add notes about non-compliant FP behavior on 32bit x86 targets.] (rust-lang/rust#113053) - [Stabilize ratified RISC-V target features.] (rust-lang/rust#116485) Compiler -------- - [Rework negative coherence to properly consider impls that only partly overlap.] (rust-lang/rust#112875) - [Bump `COINDUCTIVE_OVERLAP_IN_COHERENCE` to deny, and warn in dependencies.] (rust-lang/rust#116493) - [Consider alias bounds when computing liveness in NLL.] (rust-lang/rust#116733) - [Add the V (vector) extension to the `riscv64-linux-android` target spec.] (rust-lang/rust#116618) - [Automatically enable cross-crate inlining for small functions] (rust-lang/rust#116505) - Add several new tier 3 targets: - [`csky-unknown-linux-gnuabiv2hf`] (rust-lang/rust#117049) - [`i586-unknown-netbsd`] (rust-lang/rust#117170) - [`mipsel-unknown-netbsd`] (rust-lang/rust#117356) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries --------- - [Override `Waker::clone_from` to avoid cloning `Waker`s unnecessarily.] (rust-lang/rust#96979) - [Implement `BufRead` for `VecDeque<u8>`.] (rust-lang/rust#110604) - [Implement `FusedIterator` for `DecodeUtf16` when the inner iterator does.] (rust-lang/rust#110729) - [Implement `Not, Bit{And,Or}{,Assign}` for IP addresses.] (rust-lang/rust#113747) - [Implement `Default` for `ExitCode`.] (rust-lang/rust#114589) - [Guarantee representation of None in NPO] (rust-lang/rust#115333) - [Document when atomic loads are guaranteed read-only.] (rust-lang/rust#115577) - [Broaden the consequences of recursive TLS initialization.] (rust-lang/rust#116172) - [Windows: Support sub-millisecond sleep.] (rust-lang/rust#116461) - [Fix generic bound of `str::SplitInclusive`'s `DoubleEndedIterator` impl] (rust-lang/rust#100806) - [Fix exit status / wait status on non-Unix `cfg(unix)` platforms.] (rust-lang/rust#115108) Stabilized APIs --------------- - [`Atomic*::from_ptr`] (https://doc.rust-lang.org/stable/core/sync/atomic/struct.AtomicUsize.html#method.from_ptr) - [`FileTimes`] (https://doc.rust-lang.org/stable/std/fs/struct.FileTimes.html) - [`FileTimesExt`] (https://doc.rust-lang.org/stable/std/os/windows/fs/trait.FileTimesExt.html) - [`File::set_modified`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_modified) - [`File::set_times`] (https://doc.rust-lang.org/stable/std/fs/struct.File.html#method.set_times) - [`IpAddr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/enum.IpAddr.html#method.to_canonical) - [`Ipv6Addr::to_canonical`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_canonical) - [`Option::as_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_slice) - [`Option::as_mut_slice`] (https://doc.rust-lang.org/stable/core/option/enum.Option.html#method.as_mut_slice) - [`pointer::byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_add) - [`pointer::byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset) - [`pointer::byte_offset_from`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_offset_from) - [`pointer::byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.byte_sub) - [`pointer::wrapping_byte_add`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_add) - [`pointer::wrapping_byte_offset`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_offset) - [`pointer::wrapping_byte_sub`] (https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.wrapping_byte_sub) These APIs are now stable in const contexts: - [`Ipv6Addr::to_ipv4_mapped`] (https://doc.rust-lang.org/stable/core/net/struct.Ipv6Addr.html#method.to_ipv4_mapped) - [`MaybeUninit::assume_init_read`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.assume_init_read) - [`MaybeUninit::zeroed`] (https://doc.rust-lang.org/stable/core/mem/union.MaybeUninit.html#method.zeroed) - [`mem::discriminant`] (https://doc.rust-lang.org/stable/core/mem/fn.discriminant.html) - [`mem::zeroed`] (https://doc.rust-lang.org/stable/core/mem/fn.zeroed.html) Cargo ----- - [Add new packages to `[workspace.members]` automatically.] (rust-lang/cargo#12779) - [Allow version-less `Cargo.toml` manifests.] (rust-lang/cargo#12786) - [Make browser links out of HTML file paths.] (rust-lang/cargo#12889) Rustdoc ------- - [Accept less invalid Rust in rustdoc.] (rust-lang/rust#117450) - [Document lack of object safety on affected traits.] (rust-lang/rust#113241) - [Hide `#[repr(transparent)]` if it isn't part of the public ABI.] (rust-lang/rust#115439) - [Show enum discriminant if it is a C-like variant.] (rust-lang/rust#116142) Compatibility Notes ------------------- - [FreeBSD targets now require at least version 12.] (rust-lang/rust#114521) - [Formally demote tier 2 MIPS targets to tier 3.] (rust-lang/rust#115238) - [Make misalignment a hard error in `const` contexts.] (rust-lang/rust#115524) - [Fix detecting references to packed unsized fields.] (rust-lang/rust#115583) - [Remove support for compiler plugins.] (rust-lang/rust#116412)

compiler-errors added 3 commits October 14, 2023 15:35

Consider alias bounds when considering lliveness for alias types in NLL

184d5ef

Consider param-env candidates, too

e425d85

Consider static specially

371d8a8

rustbot assigned aliemjay Oct 14, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 14, 2023

compiler-errors added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 14, 2023

aliemjay reviewed Oct 16, 2023

View reviewed changes

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 16, 2023

compiler-errors added 2 commits October 17, 2023 01:26

Use a visitor that could be reused by opaque type capture check later

1005970

Flesh out tests more

f0e385d

rust-cloud-vms bot force-pushed the alias-liveness-but-this-time-sound branch from 412d27d to f0e385d Compare October 17, 2023 01:27

aliemjay approved these changes Oct 17, 2023

View reviewed changes

rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Oct 19, 2023

rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Oct 19, 2023

rustbot added A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` labels Oct 19, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 29, 2023

bors merged commit ec2b311 into rust-lang:master Oct 29, 2023
12 checks passed

rustbot added this to the 1.75.0 milestone Oct 29, 2023

bors mentioned this pull request Oct 29, 2023

Only use dense bitsets in dataflow analyses #116152

Merged

github-actions bot mentioned this pull request Oct 31, 2023

Automated pull from upstream master ferrocene/ferrocene#74

Merged

Manishearth mentioned this pull request Oct 31, 2023

ICE: assertion failed: !verify_if_eq_b.has_escaping_bound_vars() #117455

Closed

apiraino removed the to-announce Announce this issue on triage meeting label Nov 9, 2023

oskgo mentioned this pull request Jan 25, 2024

Borrow checker determines borrows by examining lifetime parameters on unexpanded associated types instead of inspecting their lifetime bounds #50029

Closed

+                                  assert!(r.type_flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS));
+                                  r.visit_with(self)?;
+                              } else {
+                                  // Skip lifetime parameters that are not captures.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consider alias bounds when computing liveness in NLL (but this time sound hopefully) #116733

Consider alias bounds when computing liveness in NLL (but this time sound hopefully) #116733

compiler-errors commented Oct 14, 2023 •

edited

aliemjay left a comment

aliemjay Oct 16, 2023

aliemjay commented Oct 16, 2023

rfcbot commented Oct 16, 2023 •

edited by BoxyUwU

nikomatsakis commented Oct 19, 2023

rfcbot commented Oct 19, 2023

traviscross commented Oct 19, 2023

rfcbot commented Oct 29, 2023

aliemjay commented Oct 29, 2023

bors commented Oct 29, 2023

bors commented Oct 29, 2023

bors commented Oct 29, 2023

rust-timer commented Oct 29, 2023

Manishearth commented Oct 31, 2023

Consider alias bounds when computing liveness in NLL (but this time sound hopefully) #116733

Consider alias bounds when computing liveness in NLL (but this time sound hopefully) #116733

Conversation

compiler-errors commented Oct 14, 2023 • edited

Background

The Fix

Limitation: Multiple choices

Limitation: Opaque hidden types

Read more

Footnotes

aliemjay left a comment

Choose a reason for hiding this comment

aliemjay Oct 16, 2023

Choose a reason for hiding this comment

aliemjay commented Oct 16, 2023

rfcbot commented Oct 16, 2023 • edited by BoxyUwU

nikomatsakis commented Oct 19, 2023

rfcbot commented Oct 19, 2023

traviscross commented Oct 19, 2023

rfcbot commented Oct 29, 2023

aliemjay commented Oct 29, 2023

bors commented Oct 29, 2023

bors commented Oct 29, 2023

bors commented Oct 29, 2023

rust-timer commented Oct 29, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Manishearth commented Oct 31, 2023

compiler-errors commented Oct 14, 2023 •

edited

rfcbot commented Oct 16, 2023 •

edited by BoxyUwU