Skip to content

Correctly deny late-bound lifetimes from parent in anon consts and TAITs #115486

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

Merged
merged 2 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Correctly deny late-bound lifetimes from parent in anon consts and TAITs
  • Loading branch information
compiler-errors committed Sep 5, 2023
commit 52aff538123137fb395a52c4ddb6fba2ef6fed07
12 changes: 8 additions & 4 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,16 @@ hir_analysis_auto_deref_reached_recursion_limit = reached the recursion limit wh
.label = deref recursion limit reached
.help = consider increasing the recursion limit by adding a `#![recursion_limit = "{$suggested_limit}"]` attribute to your crate (`{$crate_name}`)

hir_analysis_cannot_capture_late_bound_const_in_anon_const =
cannot capture late-bound const parameter in a constant
hir_analysis_cannot_capture_late_bound_const =
cannot capture late-bound const parameter in {$what}
.label = parameter defined here

hir_analysis_cannot_capture_late_bound_ty_in_anon_const =
cannot capture late-bound type parameter in a constant
hir_analysis_cannot_capture_late_bound_lifetime =
cannot capture late-bound lifetime in {$what}
.label = lifetime defined here

hir_analysis_cannot_capture_late_bound_ty =
cannot capture late-bound type parameter in {$what}
.label = parameter defined here

hir_analysis_cast_thin_pointer_to_fat_pointer = cannot cast thin pointer `{$expr_ty}` to fat pointer `{$cast_ty}`
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
// (*) -- not late-bound, won't change
}

Some(rbv::ResolvedArg::Error(_)) => {
bug!("only ty/ct should resolve as ResolvedArg::Error")
}
Some(rbv::ResolvedArg::Error(guar)) => ty::Region::new_error(tcx, guar),

None => {
self.re_infer(def, lifetime.ident.span).unwrap_or_else(|| {
Expand Down
71 changes: 51 additions & 20 deletions compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,14 @@ enum Scope<'a> {
s: ScopeRef<'a>,
},

/// Disallows capturing non-lifetime binders from parent scopes.
/// Disallows capturing late-bound vars from parent scopes.
///
/// This is necessary for something like `for<T> [(); { /* references T */ }]:`,
/// since we don't do something more correct like replacing any captured
/// late-bound vars with early-bound params in the const's own generics.
AnonConstBoundary {
LateBoundary {
s: ScopeRef<'a>,
what: &'static str,
},

Root {
Expand Down Expand Up @@ -216,7 +217,9 @@ impl<'a> fmt::Debug for TruncatedScopeDebug<'a> {
.field("s", &"..")
.finish(),
Scope::TraitRefBoundary { s: _ } => f.debug_struct("TraitRefBoundary").finish(),
Scope::AnonConstBoundary { s: _ } => f.debug_struct("AnonConstBoundary").finish(),
Scope::LateBoundary { s: _, what } => {
f.debug_struct("LateBoundary").field("what", what).finish()
}
Scope::Root { opt_parent_item } => {
f.debug_struct("Root").field("opt_parent_item", &opt_parent_item).finish()
}
Expand Down Expand Up @@ -318,7 +321,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
break (vec![], BinderScopeType::Normal);
}

Scope::ObjectLifetimeDefault { s, .. } | Scope::AnonConstBoundary { s } => {
Scope::ObjectLifetimeDefault { s, .. } | Scope::LateBoundary { s, .. } => {
scope = s;
}

Expand Down Expand Up @@ -697,9 +700,12 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
}) => {
intravisit::walk_ty(self, ty);

// Elided lifetimes are not allowed in non-return
// position impl Trait
let scope = Scope::TraitRefBoundary { s: self.scope };
// Elided lifetimes and late-bound lifetimes (from the parent)
// are not allowed in non-return position impl Trait
let scope = Scope::LateBoundary {
s: &Scope::TraitRefBoundary { s: self.scope },
what: "type alias impl trait",
};
self.with(scope, |this| intravisit::walk_item(this, opaque_ty));

return;
Expand Down Expand Up @@ -979,7 +985,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
}

fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) {
self.with(Scope::AnonConstBoundary { s: self.scope }, |this| {
self.with(Scope::LateBoundary { s: self.scope, what: "constant" }, |this| {
intravisit::walk_anon_const(this, c);
});
}
Expand Down Expand Up @@ -1174,6 +1180,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
let mut late_depth = 0;
let mut scope = self.scope;
let mut outermost_body = None;
let mut crossed_late_boundary = None;
let result = loop {
match *scope {
Scope::Body { id, s } => {
Expand Down Expand Up @@ -1258,8 +1265,12 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {

Scope::ObjectLifetimeDefault { s, .. }
| Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s, .. }
| Scope::AnonConstBoundary { s } => {
| Scope::TraitRefBoundary { s, .. } => {
scope = s;
}

Scope::LateBoundary { s, what } => {
crossed_late_boundary = Some(what);
scope = s;
}
}
Expand All @@ -1268,6 +1279,22 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
if let Some(mut def) = result {
if let ResolvedArg::EarlyBound(..) = def {
// Do not free early-bound regions, only late-bound ones.
} else if let ResolvedArg::LateBound(_, _, param_def_id) = def
Copy link
Contributor

Choose a reason for hiding this comment

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

Both this branch and the next (if let Some(body_id) = outermost_body) should only be executed for late-bound variables. Should they both be in an if let ResolvedArg::LateBound(_, _, param_def_id) = def to make it clearer?

For my curiosity, what would happen if we made lifetimes in anon-consts ResolvedArg::Free?

Copy link
Member Author

Choose a reason for hiding this comment

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

For my curiosity [...]

See my comment here #115486 (comment), I kinda went into it there -- this idea would, in fact, fix at least tests like const-generics/late-bound-vars/in_closure.rs and const-generics/late-bound-vars/simple.rs, and was how #79298 worked, afaict.

It doesn't seem like a general solution, though, since when it comes to, e.g., a binder on a where clause like where for<'a> [(); { /* const that uses 'a */ }]: ..., it doesn't really make sense to use ReFree for that usage of 'a, since the late-bound region does not originate from the function's generics (and therefore the fn_sig's binder).

This also doesn't lead us towards a correct solution for handling things like for<const C: usize> [(); C]: ..., since there's no equivalent notion of a "free" type or const var (nor does it make sense, since they must be substitutable).

&& let Some(what) = crossed_late_boundary
{
let use_span = lifetime_ref.ident.span;
let def_span = self.tcx.def_span(param_def_id);
let guar = match self.tcx.def_kind(param_def_id) {
DefKind::LifetimeParam => {
self.tcx.sess.emit_err(errors::CannotCaptureLateBound::Lifetime {
use_span,
def_span,
what,
})
}
_ => unreachable!(),
};
def = ResolvedArg::Error(guar);
} else if let Some(body_id) = outermost_body {
let fn_id = self.tcx.hir().body_owner(body_id);
match self.tcx.hir().get(fn_id) {
Expand Down Expand Up @@ -1322,7 +1349,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
| Scope::ObjectLifetimeDefault { s, .. }
| Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s, .. }
| Scope::AnonConstBoundary { s } => {
| Scope::LateBoundary { s, .. } => {
scope = s;
}
}
Expand All @@ -1341,7 +1368,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
// search.
let mut late_depth = 0;
let mut scope = self.scope;
let mut crossed_anon_const = false;
let mut crossed_late_boundary = None;

let result = loop {
match *scope {
Expand Down Expand Up @@ -1376,28 +1403,32 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
scope = s;
}

Scope::AnonConstBoundary { s } => {
crossed_anon_const = true;
Scope::LateBoundary { s, what } => {
crossed_late_boundary = Some(what);
scope = s;
}
}
};

if let Some(def) = result {
if let ResolvedArg::LateBound(..) = def && crossed_anon_const {
if let ResolvedArg::LateBound(..) = def
&& let Some(what) = crossed_late_boundary
{
let use_span = self.tcx.hir().span(hir_id);
let def_span = self.tcx.def_span(param_def_id);
let guar = match self.tcx.def_kind(param_def_id) {
DefKind::ConstParam => {
self.tcx.sess.emit_err(errors::CannotCaptureLateBoundInAnonConst::Const {
self.tcx.sess.emit_err(errors::CannotCaptureLateBound::Const {
use_span,
def_span,
what,
})
}
DefKind::TyParam => {
self.tcx.sess.emit_err(errors::CannotCaptureLateBoundInAnonConst::Type {
self.tcx.sess.emit_err(errors::CannotCaptureLateBound::Type {
use_span,
def_span,
what,
})
}
_ => unreachable!(),
Expand Down Expand Up @@ -1446,7 +1477,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
| Scope::ObjectLifetimeDefault { s, .. }
| Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s, .. }
| Scope::AnonConstBoundary { s } => {
| Scope::LateBoundary { s, .. } => {
scope = s;
}
}
Expand Down Expand Up @@ -1526,7 +1557,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {
| Scope::ObjectLifetimeDefault { s, .. }
| Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s, .. }
| Scope::AnonConstBoundary { s } => {
| Scope::LateBoundary { s, .. } => {
scope = s;
}
}
Expand Down Expand Up @@ -1831,7 +1862,7 @@ impl<'a, 'tcx> BoundVarContext<'a, 'tcx> {

Scope::Supertrait { s, .. }
| Scope::TraitRefBoundary { s, .. }
| Scope::AnonConstBoundary { s } => {
| Scope::LateBoundary { s, .. } => {
scope = s;
}
}
Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,20 +430,30 @@ pub(crate) struct VariadicFunctionCompatibleConvention<'a> {
}

#[derive(Diagnostic)]
pub(crate) enum CannotCaptureLateBoundInAnonConst {
#[diag(hir_analysis_cannot_capture_late_bound_ty_in_anon_const)]
pub(crate) enum CannotCaptureLateBound {
#[diag(hir_analysis_cannot_capture_late_bound_ty)]
Type {
#[primary_span]
use_span: Span,
#[label]
def_span: Span,
what: &'static str,
},
#[diag(hir_analysis_cannot_capture_late_bound_const_in_anon_const)]
#[diag(hir_analysis_cannot_capture_late_bound_const)]
Const {
#[primary_span]
use_span: Span,
#[label]
def_span: Span,
what: &'static str,
},
#[diag(hir_analysis_cannot_capture_late_bound_lifetime)]
Lifetime {
#[primary_span]
use_span: Span,
#[label]
def_span: Span,
what: &'static str,
},
}

Expand Down
28 changes: 9 additions & 19 deletions tests/ui/const-generics/late-bound-vars/in_closure.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,13 @@
// failure-status: 1
// known-bug: unknown
// error-pattern:internal compiler error
// normalize-stderr-test "internal compiler error.*" -> ""
// normalize-stderr-test "DefId\([^)]*\)" -> "..."
// normalize-stderr-test "\nerror: internal compiler error.*\n\n" -> ""
// normalize-stderr-test "note:.*unexpectedly panicked.*\n\n" -> ""
// normalize-stderr-test "note: we would appreciate a bug report.*\n\n" -> ""
// normalize-stderr-test "note: compiler flags.*\n\n" -> ""
// normalize-stderr-test "note: rustc.*running on.*\n\n" -> ""
// normalize-stderr-test "thread.*panicked.*:\n.*\n" -> ""
// normalize-stderr-test "stack backtrace:\n" -> ""
// normalize-stderr-test "\s\d{1,}: .*\n" -> ""
// normalize-stderr-test "\s at .*\n" -> ""
// normalize-stderr-test ".*note: Some details.*\n" -> ""
// normalize-stderr-test "\n[ ]*\n" -> ""
// normalize-stderr-test "compiler/.*: projection" -> "projection"
// normalize-stderr-test ".*omitted \d{1,} frame.*\n" -> ""
// normalize-stderr-test "error: [\s\n]*query stack during panic:\n" -> ""
// this should run-pass

// If we want this to compile, then we'd need to do something like RPITs do,
// where nested associated constants have early-bound versions of their captured
// late-bound vars inserted into their generics. This gives us substitutable
// lifetimes to actually use when borrow-checking the associated const, which is
// lowered as a totally separate body from its parent. Since this doesn't exist,
// so we should just error rather than resolving this late-bound var with no
// binder to actually attach it to, or worse, as a free region that can't even be
// substituted correctly, and ICEing. - @compiler-errors
Copy link
Member Author

@compiler-errors compiler-errors Sep 2, 2023

Choose a reason for hiding this comment

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

The purpose of this comment is to ramble a bit about a more general solution than what would be needed to fix just this test and its related test (in_closure.rs and simple.rs) -- the simplest fix for just those tests would be to just go restore behavior of resolving late-bound region captured by anon consts as free regions in the parent function's body, and fix NLL to do the free region -> vid mapping correctly again (#111215 discusses this regression in the way that NLL maps free regions, cc @BoxyUwU).

But, for the record, #79298 never really correctly supported referencing late-bound regions in anon consts that show up in general positions (i.e. other positions where the const wasn't just referencing a free region in the scope of a body), e.g.:

// ICEs even on 1.51 after #79298

#![feature(const_generics)] // `generic_const_exprs` after it was renamed.
#![allow(incomplete_features)]

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

fn test<'a>() -> [u8; { inner::<'a>() }] {
    [1, 2, 3]
}

fn main() {
    test();
}

or in other binders, like:

trait Trait {
    type Assoc;
}

fn test() {
    let x: dyn for<'a> Trait<Assoc = [u8; { inner::<'a>() }]>;
}

This isn't necessarily the only solution to fix this -- I guess we could (ab)use free regions to correctly borrowck consts that show up in the binders above, but it seems pretty obvious to me that the clearest and easiest way to do this is to just represent late-bound vars captured by nested consts as early-bound regions owned by those consts.


#![feature(generic_const_exprs)]
#![allow(incomplete_features)]
Expand Down
30 changes: 20 additions & 10 deletions tests/ui/const-generics/late-bound-vars/in_closure.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
#0 [mir_borrowck] borrow-checking `test::{closure#0}::{constant#1}`
#1 [mir_drops_elaborated_and_const_checked] elaborating drops for `test::{closure#0}::{constant#1}`
#2 [mir_for_ctfe] caching mir of `test::{closure#0}::{constant#1}` for CTFE
#3 [eval_to_allocation_raw] const-evaluating + checking `test::{closure#0}::{constant#1}`
#4 [eval_to_allocation_raw] const-evaluating + checking `test::{closure#0}::{constant#1}`
#5 [eval_to_valtree] evaluating type-level constant
#6 [typeck] type-checking `test`
#7 [analysis] running analysis passes on this crate
end of query stack
error: aborting due to previous error
error: cannot capture late-bound lifetime in constant
--> $DIR/in_closure.rs:24:29
|
LL | fn test<'a>() {
| -- lifetime defined here
LL | let _ = || {
LL | let _: [u8; inner::<'a>()];
| ^^

error: cannot capture late-bound lifetime in constant
--> $DIR/in_closure.rs:25:29
|
LL | fn test<'a>() {
| -- lifetime defined here
...
LL | let _ = [0; inner::<'a>()];
| ^^

error: aborting due to 2 previous errors

27 changes: 9 additions & 18 deletions tests/ui/const-generics/late-bound-vars/simple.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,13 @@
// failure-status: 101
// known-bug: unknown
// error-pattern:internal compiler error
// normalize-stderr-test "internal compiler error.*" -> ""
// normalize-stderr-test "DefId\([^)]*\)" -> "..."
// normalize-stderr-test "\nerror: internal compiler error.*\n\n" -> ""
// normalize-stderr-test "note:.*unexpectedly panicked.*\n\n" -> ""
// normalize-stderr-test "note: we would appreciate a bug report.*\n\n" -> ""
// normalize-stderr-test "note: compiler flags.*\n\n" -> ""
// normalize-stderr-test "note: rustc.*running on.*\n\n" -> ""
// normalize-stderr-test "thread.*panicked.*:\n.*\n" -> ""
// normalize-stderr-test "stack backtrace:\n" -> ""
// normalize-stderr-test "\s\d{1,}: .*\n" -> ""
// normalize-stderr-test "\s at .*\n" -> ""
// normalize-stderr-test ".*note: Some details.*\n" -> ""
// normalize-stderr-test "\n\n[ ]*\n" -> ""
// normalize-stderr-test "compiler/.*: projection" -> "projection"
// normalize-stderr-test ".*omitted \d{1,} frame.*\n" -> ""
// normalize-stderr-test "error: [\s\n]*query stack" -> "error: query stack"

// If we want this to compile, then we'd need to do something like RPITs do,
// where nested associated constants have early-bound versions of their captured
// late-bound vars inserted into their generics. This gives us substitutable
// lifetimes to actually use when borrow-checking the associated const, which is
// lowered as a totally separate body from its parent. Since this doesn't exist,
// so we should just error rather than resolving this late-bound var with no
// binder to actually attach it to, or worse, as a free region that can't even be
// substituted correctly, and ICEing. - @compiler-errors

#![feature(generic_const_exprs)]
#![allow(incomplete_features)]
Expand Down
29 changes: 18 additions & 11 deletions tests/ui/const-generics/late-bound-vars/simple.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
error: query stack during panic:
#0 [mir_borrowck] borrow-checking `test::{constant#1}`
#1 [mir_drops_elaborated_and_const_checked] elaborating drops for `test::{constant#1}`
#2 [mir_for_ctfe] caching mir of `test::{constant#1}` for CTFE
#3 [eval_to_allocation_raw] const-evaluating + checking `test::{constant#1}`
#4 [eval_to_allocation_raw] const-evaluating + checking `test::{constant#1}`
#5 [eval_to_valtree] evaluating type-level constant
#6 [typeck] type-checking `test`
#7 [analysis] running analysis passes on this crate
end of query stack
error: aborting due to previous error
error: cannot capture late-bound lifetime in constant
--> $DIR/simple.rs:20:25
|
LL | fn test<'a>() {
| -- lifetime defined here
LL | let _: [u8; inner::<'a>()];
| ^^

error: cannot capture late-bound lifetime in constant
--> $DIR/simple.rs:21:25
|
LL | fn test<'a>() {
| -- lifetime defined here
LL | let _: [u8; inner::<'a>()];
LL | let _ = [0; inner::<'a>()];
| ^^

error: aborting due to 2 previous errors

13 changes: 13 additions & 0 deletions tests/ui/consts/escaping-bound-var.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![feature(generic_const_exprs)]
//~^ WARN the feature `generic_const_exprs` is incomplete

fn test<'a>(
_: &'a (),
) -> [(); {
let x: &'a ();
//~^ ERROR cannot capture late-bound lifetime in constant
1
}] {
}

fn main() {}
Loading