Skip to content

Commit

Permalink
Rollup merge of rust-lang#137399 - lukas-code:oopsie-woopsie, r=compi…
Browse files Browse the repository at this point in the history
…ler-errors

fix ICE in layout computation with unnormalizable const

The first commit reverts half of 7a667d2, where I removed a case from `layout_of` for handling non-generic unevaluated consts in array length, that I incorrectly assumed to be unreachable. This can actually happen with the combination of `feature(generic_const_exprs)` and `feature(trivial_bounds)`, because GCE makes anon consts inherit their parent's predicates and with an impossible predicate like `u8: A` it's possible to have an array whose length is an associated const like `<u8 as A>::B` that is not generic, but also can't be normalized:

```rust
#![feature(generic_const_exprs)]
#![feature(trivial_bounds)]

trait A {
    const B: usize;
}

// With GCE + trivial bounds this definition is not a compile error.
// Computing the layout of this type shouldn't ICE.
struct S([u8; <u8 as A>::B])
where
    u8: A;
```

---

The first commit also incidentally fixes rust-lang#137308, which also managed to get an unnormalizable assoc const into an array length:

```rust
trait A {
    const B: usize;
}

impl<C: ?Sized> A for u8 { //~ ERROR: the type parameter `C` is not constrained
    const B: usize = 42;
}

// Computing the layout of this type shouldn't ICE, even with the compile error above.
struct S([u8; <u8 as A>::B]);
```

This happens, because we bail out from `codegen_select_candidate` with an error if the selected impl has unconstrained params to avoid leaking infer vars out of a query. `Instance::try_resolve` will then return `Ok(None)`, which for assoc consts roughly means "this const can't be evaluated in a generic context" and is treated as such: https://github.com/rust-lang/rust/blob/71e06b9c59d6af50fdc55aed75620493d29baf98/compiler/rustc_middle/src/mir/interpret/queries.rs#L84 (and this can ICE if the const isn't generic: rust-lang#135617).

However, here `<u8 as A>::B` is definitely not "too generic" and also not unresolvable due to an unsatisfiable `u8: A` bound, so I've included the second commit to change the result of `Instance::try_resolve` from `Ok(None)` to `Err(ErrorGuaranteed)` when resolving an assoc item to an impl with unconstrained generic params. This has the effect that `<u8 as A>::B` will now be normalized to `ConstKind::Error` in the example above.

This properly fixes rust-lang#137308, by no longer treating `<u8 as A>::B` as unresolvable even though it clearly has a unique impl that it resolves to. It also has the effect of changing the layout error from `Unknown` ("the type may be valid but has no sensible layout") to `ReferencesError` ("a non-layout error is reported elsewhere") which seems more appropriate.

r? ```@compiler-errors```
  • Loading branch information
matthiaskrgr authored Feb 22, 2025
2 parents fa62fbe + 7fea935 commit 6352044
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 11 deletions.
5 changes: 4 additions & 1 deletion compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::borrow::Cow;
use std::hash::{Hash, Hasher};
use std::sync::Arc;

use rustc_errors::{Applicability, Diag, EmissionGuarantee};
use rustc_errors::{Applicability, Diag, EmissionGuarantee, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_hir::HirId;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -996,4 +996,7 @@ pub enum CodegenObligationError {
/// but was included during typeck due to the trivial_bounds feature.
Unimplemented,
FulfillmentError,
/// The selected impl has unconstrained generic parameters. This will emit an error
/// during impl WF checking.
UnconstrainedParam(ErrorGuaranteed),
}
16 changes: 7 additions & 9 deletions compiler/rustc_traits/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,14 @@ pub(crate) fn codegen_select_candidate<'tcx>(
// but never resolved, causing the return value of a query to contain inference
// vars. We do not have a concept for this and will in fact ICE in stable hashing
// of the return value. So bail out instead.
match impl_source {
ImplSource::UserDefined(impl_) => {
tcx.dcx().span_delayed_bug(
tcx.def_span(impl_.impl_def_id),
"this impl has unconstrained generic parameters",
);
}
let guar = match impl_source {
ImplSource::UserDefined(impl_) => tcx.dcx().span_delayed_bug(
tcx.def_span(impl_.impl_def_id),
"this impl has unconstrained generic parameters",
),
_ => unreachable!(),
}
return Err(CodegenObligationError::FulfillmentError);
};
return Err(CodegenObligationError::UnconstrainedParam(guar));
}

Ok(&*tcx.arena.alloc(impl_source))
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ fn resolve_associated_item<'tcx>(
| CodegenObligationError::Unimplemented
| CodegenObligationError::FulfillmentError,
) => return Ok(None),
Err(CodegenObligationError::UnconstrainedParam(guar)) => return Err(guar),
};

// Now that we know which impl is being used, we can dispatch to
Expand Down
15 changes: 14 additions & 1 deletion compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,25 @@ fn extract_const_value<'tcx>(
) -> Result<ty::Value<'tcx>, &'tcx LayoutError<'tcx>> {
match ct.kind() {
ty::ConstKind::Value(cv) => Ok(cv),
ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) | ty::ConstKind::Unevaluated(_) => {
ty::ConstKind::Param(_) | ty::ConstKind::Expr(_) => {
if !ct.has_param() {
bug!("failed to normalize const, but it is not generic: {ct:?}");
}
Err(error(cx, LayoutError::TooGeneric(ty)))
}
ty::ConstKind::Unevaluated(_) => {
let err = if ct.has_param() {
LayoutError::TooGeneric(ty)
} else {
// This case is reachable with unsatisfiable predicates and GCE (which will
// cause anon consts to inherit the unsatisfiable predicates). For example
// if we have an unsatisfiable `u8: Trait` bound, then it's not a compile
// error to mention `[u8; <u8 as Trait>::CONST]`, but we can't compute its
// layout.
LayoutError::Unknown(ty)
};
Err(error(cx, err))
}
ty::ConstKind::Infer(_)
| ty::ConstKind::Bound(..)
| ty::ConstKind::Placeholder(_)
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/layout/gce-rigid-const-in-array-len.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//! With `feature(generic_const_exprs)`, anon consts (e.g. length in array types) will
//! inherit their parent's predicates. When combined with `feature(trivial_bounds)`, it
//! is possible to have an unevaluated constant that is rigid, but not generic.
//!
//! This is what happens below: `u8: A` does not hold in the global environment, but
//! with trivial bounds + GCE it it possible that `<u8 as A>::B` can appear in an array
//! length without causing a compile error. This constant is *rigid* (i.e. it cannot be
//! normalized further), but it is *not generic* (i.e. it does not depend on any generic
//! parameters).
//!
//! This test ensures that we do not ICE in layout computation when encountering such a
//! constant.
#![feature(rustc_attrs)]
#![feature(generic_const_exprs)] //~ WARNING: the feature `generic_const_exprs` is incomplete
#![feature(trivial_bounds)]

#![crate_type = "lib"]

trait A {
const B: usize;
}

#[rustc_layout(debug)]
struct S([u8; <u8 as A>::B]) //~ ERROR: the type `[u8; <u8 as A>::B]` has an unknown layout
where
u8: A;
17 changes: 17 additions & 0 deletions tests/ui/layout/gce-rigid-const-in-array-len.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
warning: the feature `generic_const_exprs` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/gce-rigid-const-in-array-len.rs:15:12
|
LL | #![feature(generic_const_exprs)]
| ^^^^^^^^^^^^^^^^^^^
|
= note: see issue #76560 <https://github.com/rust-lang/rust/issues/76560> for more information
= note: `#[warn(incomplete_features)]` on by default

error: the type `[u8; <u8 as A>::B]` has an unknown layout
--> $DIR/gce-rigid-const-in-array-len.rs:25:1
|
LL | struct S([u8; <u8 as A>::B])
| ^^^^^^^^

error: aborting due to 1 previous error; 1 warning emitted

18 changes: 18 additions & 0 deletions tests/ui/layout/unconstrained-param-ice-137308.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//! Regression test for <https://github.com/rust-lang/rust/issues/137308>.
//!
//! This used to ICE in layout computation, because `<u8 as A>::B` fails to normalize
//! due to the unconstrained param on the impl.
#![feature(rustc_attrs)]
#![crate_type = "lib"]

trait A {
const B: usize;
}

impl<C: ?Sized> A for u8 { //~ ERROR: the type parameter `C` is not constrained
const B: usize = 42;
}

#[rustc_layout(debug)]
struct S([u8; <u8 as A>::B]); //~ ERROR: the type has an unknown layout
15 changes: 15 additions & 0 deletions tests/ui/layout/unconstrained-param-ice-137308.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0207]: the type parameter `C` is not constrained by the impl trait, self type, or predicates
--> $DIR/unconstrained-param-ice-137308.rs:13:6
|
LL | impl<C: ?Sized> A for u8 {
| ^ unconstrained type parameter

error: the type has an unknown layout
--> $DIR/unconstrained-param-ice-137308.rs:18:1
|
LL | struct S([u8; <u8 as A>::B]);
| ^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0207`.

0 comments on commit 6352044

Please sign in to comment.