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

correctly deal with late-bound lifetimes in anon consts #79298

Merged
merged 1 commit into from
Jan 17, 2021
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
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1316,7 +1316,7 @@ rustc_queries! {
desc { "looking up a named region" }
}
query is_late_bound_map(_: LocalDefId) ->
Option<&'tcx FxHashSet<ItemLocalId>> {
Option<(LocalDefId, &'tcx FxHashSet<ItemLocalId>)> {
desc { "testing if a region is late bound" }
}
query object_lifetime_defaults_map(_: LocalDefId)
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2578,7 +2578,8 @@ impl<'tcx> TyCtxt<'tcx> {
}

pub fn is_late_bound(self, id: HirId) -> bool {
self.is_late_bound_map(id.owner).map_or(false, |set| set.contains(&id.local_id))
self.is_late_bound_map(id.owner)
.map_or(false, |(owner, set)| owner == id.owner && set.contains(&id.local_id))
}

pub fn object_lifetime_defaults(self, id: HirId) -> Option<&'tcx [ObjectLifetimeDefault]> {
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir/src/borrow_check/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -788,13 +788,13 @@ fn for_each_late_bound_region_defined_on<'tcx>(
fn_def_id: DefId,
mut f: impl FnMut(ty::Region<'tcx>),
) {
if let Some(late_bounds) = tcx.is_late_bound_map(fn_def_id.expect_local()) {
for late_bound in late_bounds.iter() {
let hir_id = HirId { owner: fn_def_id.expect_local(), local_id: *late_bound };
if let Some((owner, late_bounds)) = tcx.is_late_bound_map(fn_def_id.expect_local()) {
for &late_bound in late_bounds.iter() {
let hir_id = HirId { owner, local_id: late_bound };
let name = tcx.hir().name(hir_id);
let region_def_id = tcx.hir().local_def_id(hir_id);
let liberated_region = tcx.mk_region(ty::ReFree(ty::FreeRegion {
scope: fn_def_id,
scope: owner.to_def_id(),
bound_region: ty::BoundRegionKind::BrNamed(region_def_id.to_def_id(), name),
}));
f(liberated_region);
Expand Down
32 changes: 30 additions & 2 deletions compiler/rustc_resolve/src/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, LOCAL_CRATE};
use rustc_hir::def_id::{CrateNum, DefIdMap, LOCAL_CRATE};
use rustc_hir::hir_id::ItemLocalId;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{GenericArg, GenericParam, LifetimeName, Node, ParamName, QPath};
use rustc_hir::{GenericParamKind, HirIdMap, HirIdSet, LifetimeParamKind};
Expand All @@ -20,6 +21,7 @@ use rustc_middle::middle::resolve_lifetime::*;
use rustc_middle::ty::{self, DefIdTree, GenericParamDefKind, TyCtxt};
use rustc_middle::{bug, span_bug};
use rustc_session::lint;
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use std::borrow::Cow;
Expand Down Expand Up @@ -284,7 +286,7 @@ pub fn provide(providers: &mut ty::query::Providers) {
resolve_lifetimes,

named_region_map: |tcx, id| tcx.resolve_lifetimes(LOCAL_CRATE).defs.get(&id),
is_late_bound_map: |tcx, id| tcx.resolve_lifetimes(LOCAL_CRATE).late_bound.get(&id),
is_late_bound_map,
object_lifetime_defaults_map: |tcx, id| {
tcx.resolve_lifetimes(LOCAL_CRATE).object_lifetime_defaults.get(&id)
},
Expand Down Expand Up @@ -320,6 +322,32 @@ fn resolve_lifetimes(tcx: TyCtxt<'_>, for_krate: CrateNum) -> ResolveLifetimes {
rl
}

fn is_late_bound_map<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
) -> Option<(LocalDefId, &'tcx FxHashSet<ItemLocalId>)> {
match tcx.def_kind(def_id) {
DefKind::AnonConst => {
let mut def_id = tcx
.parent(def_id.to_def_id())
.unwrap_or_else(|| bug!("anon const or closure without a parent"));
// We search for the next outer anon const or fn here
// while skipping closures.
//
// Note that for `AnonConst` we still just recurse until we
// find a function body, but who cares :shrug:
while tcx.is_closure(def_id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this also need to check if the parent is another anon const for something like [1; [2; inner::<'a>].len()]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

            // Note that for `AnonConst` we still just recurse until we
            // find a function body, but who cares :shrug:

we could only call this query recursively when we find the containing function, but considering that after #79313 anon const can have additional early bound lifetimes i think this approach might be a bit cleaner 🤔 don't really care either way here

Copy link
Contributor

Choose a reason for hiding this comment

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

So, now I'm confused as to why this PR is correct or needed at all after #79313.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#79313 only deals with lifetimes introduced by explicit binders, while this PR deals with late bound lifetimes.

Updated the PR description, does this explanation help here?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I've gone through this and it looks fine. r=me with a rebase

def_id = tcx
.parent(def_id)
.unwrap_or_else(|| bug!("anon const or closure without a parent"));
}

tcx.is_late_bound_map(def_id.expect_local())
}
_ => tcx.resolve_lifetimes(LOCAL_CRATE).late_bound.get(&def_id).map(|lt| (def_id, lt)),
}
}

fn krate(tcx: TyCtxt<'_>) -> NamedRegionMap {
let krate = tcx.hir().krate();
let mut map = NamedRegionMap {
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/const-generics/late-bound-vars/in_closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-pass
#![feature(const_generics)]
#![allow(incomplete_features)]

const fn inner<'a>() -> usize where &'a (): Sized {
3
}

fn test<'a>() {
let _ = || {
let _: [u8; inner::<'a>()];
let _ = [0; inner::<'a>()];
};
}

fn main() {
test();
}
16 changes: 16 additions & 0 deletions src/test/ui/const-generics/late-bound-vars/simple.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// run-pass
#![feature(const_generics)]
#![allow(incomplete_features)]

const fn inner<'a>() -> usize where &'a (): Sized {
3
}

fn test<'a>() {
let _: [u8; inner::<'a>()];
let _ = [0; inner::<'a>()];
}

fn main() {
test();
}