Skip to content
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_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ fn check_type_defn<'tcx>(
item: &hir::Item<'tcx>,
all_sized: bool,
) -> Result<(), ErrorGuaranteed> {
let _ = tcx.representability(item.owner_id.def_id);
let _ = tcx.check_representability(item.owner_id.def_id);
let adt_def = tcx.adt_def(item.owner_id);

enter_wf_checking_ctxt(tcx, item.owner_id.def_id, |wfcx| {
Expand Down
17 changes: 11 additions & 6 deletions compiler/rustc_middle/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,18 +593,23 @@ rustc_queries! {
}

/// Checks whether a type is representable or infinitely sized
query representability(key: LocalDefId) -> rustc_middle::ty::Representability {
query check_representability(key: LocalDefId) -> rustc_middle::ty::Representability {
desc { "checking if `{}` is representable", tcx.def_path_str(key) }
// infinitely sized types will cause a cycle
// Infinitely sized types will cause a cycle. The custom `FromCycleError` impl for
// `Representability` will print a custom error about the infinite size and then abort
// compilation. (In the past we recovered and continued, but in practice that leads to
// confusing subsequent error messages about cycles that then abort.)
cycle_delay_bug
// we don't want recursive representability calls to be forced with
// We don't want recursive representability calls to be forced with
// incremental compilation because, if a cycle occurs, we need the
// entire cycle to be in memory for diagnostics
// entire cycle to be in memory for diagnostics. This means we can't
// use `ensure_ok()` with this query.
anon
}

/// An implementation detail for the `representability` query
query representability_adt_ty(key: Ty<'tcx>) -> rustc_middle::ty::Representability {
/// An implementation detail for the `check_representability` query. See that query for more
/// details, particularly on the modifiers.
query check_representability_adt_ty(key: Ty<'tcx>) -> rustc_middle::ty::Representability {
desc { "checking if `{}` is representable", key }
cycle_delay_bug
anon
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,8 +741,7 @@ impl<'tcx> AdtDef<'tcx> {
}
}

/// This type exists just so a `FromCycleError` impl can be made for the `check_representability`
/// query.
#[derive(Clone, Copy, Debug, HashStable)]
pub enum Representability {
Representable,
Infinite(ErrorGuaranteed),
}
pub struct Representability;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like that you haven't got rid of this type, although I would have made it pub struct Representability(()) in order to make its construction a private detail for a query implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, but we cannot do that as query implementation is in another crate. I guess I would've done new_unchecked unsafe fn instead then if rustc was my project, but that's kinda a far too theoretical concern for an open project.

Copy link
Contributor

Choose a reason for hiding this comment

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

if you want to keep enforcing let _ = in front of the query invocations, then this struct should be #[must_use]. Otherwise just call the query like we call unit queries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I change this:

    let _ = tcx.check_representability(item.owner_id.def_id);

to this:

    tcx.check_representability(item.owner_id.def_id);

I get this warning:

warning: unused return value of `queries::<impl rustc_middle::ty::TyCtxt<'tcx>>::check_representability` that must be used
    --> compiler/rustc_hir_analysis/src/check/wfcheck.rs:1000:5
     |
1000 |     tcx.check_representability(item.owner_id.def_id);
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: `#[warn(unused_must_use)]` (part of `#[warn(unused)]`) on by default
help: use `let _ = ...` to ignore the resulting value
     |
1000 |     let _ = tcx.check_representability(item.owner_id.def_id);
     |     +++++++

I'm not sure where the must_use that triggers this comes from.

I could instead do this:

    tcx.ensure_ok().check_representability(item.owner_id.def_id);

which is probably better, given that ensure_ok can help with incremental perf. Sound ok to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I totally goldfished that I tried ensure_ok before.

Anyway, I don't see how to avoid the let _ = things, given that I get the warnings without them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. Unfortunate but makes sense for now

5 changes: 2 additions & 3 deletions compiler/rustc_middle/src/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,9 @@ pub(crate) fn provide(providers: &mut Providers) {
/// requires calling [`InhabitedPredicate::instantiate`]
fn inhabited_predicate_adt(tcx: TyCtxt<'_>, def_id: DefId) -> InhabitedPredicate<'_> {
if let Some(def_id) = def_id.as_local() {
if matches!(tcx.representability(def_id), ty::Representability::Infinite(_)) {
return InhabitedPredicate::True;
}
let _ = tcx.check_representability(def_id);
}

let adt = tcx.adt_def(def_id);
InhabitedPredicate::any(
tcx,
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_query_impl/src/from_cycle_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<'tcx> FromCycleError<'tcx> for Representability {
let mut item_and_field_ids = Vec::new();
let mut representable_ids = FxHashSet::default();
for info in &cycle_error.cycle {
if info.frame.dep_kind == DepKind::representability
if info.frame.dep_kind == DepKind::check_representability
&& let Some(field_id) = info.frame.def_id
&& let Some(field_id) = field_id.as_local()
&& let Some(DefKind::Field) = info.frame.info.def_kind
Expand All @@ -109,16 +109,18 @@ impl<'tcx> FromCycleError<'tcx> for Representability {
}
}
for info in &cycle_error.cycle {
if info.frame.dep_kind == DepKind::representability_adt_ty
if info.frame.dep_kind == DepKind::check_representability_adt_ty
&& let Some(def_id) = info.frame.def_id_for_ty_in_cycle
&& let Some(def_id) = def_id.as_local()
&& !item_and_field_ids.iter().any(|&(id, _)| id == def_id)
{
representable_ids.insert(def_id);
}
}
// We used to continue here, but the cycle error printed next is actually less useful than
// the error produced by `recursive_type_error`.
let guar = recursive_type_error(tcx, item_and_field_ids, &representable_ids);
Representability::Infinite(guar)
guar.raise_fatal();
}
}

Expand Down
82 changes: 42 additions & 40 deletions compiler/rustc_ty_utils/src/representability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,81 +6,83 @@ use rustc_middle::ty::{self, Representability, Ty, TyCtxt};
use rustc_span::def_id::LocalDefId;

pub(crate) fn provide(providers: &mut Providers) {
*providers =
Providers { representability, representability_adt_ty, params_in_repr, ..*providers };
}

macro_rules! rtry {
($e:expr) => {
match $e {
e @ Representability::Infinite(_) => return e,
Representability::Representable => {}
}
*providers = Providers {
check_representability,
check_representability_adt_ty,
params_in_repr,
..*providers
};
}

fn representability(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Representability {
fn check_representability(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Representability {
match tcx.def_kind(def_id) {
DefKind::Struct | DefKind::Union | DefKind::Enum => {
for variant in tcx.adt_def(def_id).variants() {
for field in variant.fields.iter() {
rtry!(tcx.representability(field.did.expect_local()));
let _ = tcx.check_representability(field.did.expect_local());
}
}
Representability::Representable
}
DefKind::Field => representability_ty(tcx, tcx.type_of(def_id).instantiate_identity()),
DefKind::Field => {
check_representability_ty(tcx, tcx.type_of(def_id).instantiate_identity());
}
def_kind => bug!("unexpected {def_kind:?}"),
}
Representability
}

fn representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
fn check_representability_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) {
match *ty.kind() {
ty::Adt(..) => tcx.representability_adt_ty(ty),
// This one must be a query rather than a vanilla `check_representability_adt_ty` call. See
// the comment on `check_representability_adt_ty` below for why.
ty::Adt(..) => {
let _ = tcx.check_representability_adt_ty(ty);
}
// FIXME(#11924) allow zero-length arrays?
ty::Array(ty, _) => representability_ty(tcx, ty),
ty::Array(ty, _) => {
check_representability_ty(tcx, ty);
}
ty::Tuple(tys) => {
for ty in tys {
rtry!(representability_ty(tcx, ty));
check_representability_ty(tcx, ty);
}
Representability::Representable
}
_ => Representability::Representable,
_ => {}
}
}

/*
The reason for this being a separate query is very subtle:
Consider this infinitely sized struct: `struct Foo(Box<Foo>, Bar<Foo>)`:
When calling representability(Foo), a query cycle will occur:
representability(Foo)
-> representability_adt_ty(Bar<Foo>)
-> representability(Foo)
For the diagnostic output (in `Value::from_cycle_error`), we want to detect that
the `Foo` in the *second* field of the struct is culpable. This requires
traversing the HIR of the struct and calling `params_in_repr(Bar)`. But we can't
call params_in_repr for a given type unless it is known to be representable.
params_in_repr will cycle/panic on infinitely sized types. Looking at the query
cycle above, we know that `Bar` is representable because
representability_adt_ty(Bar<..>) is in the cycle and representability(Bar) is
*not* in the cycle.
*/
fn representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
// The reason for this being a separate query is very subtle. Consider this
// infinitely sized struct: `struct Foo(Box<Foo>, Bar<Foo>)`. When calling
// check_representability(Foo), a query cycle will occur:
//
// check_representability(Foo)
// -> check_representability_adt_ty(Bar<Foo>)
// -> check_representability(Foo)
//
// For the diagnostic output (in `Value::from_cycle_error`), we want to detect
// that the `Foo` in the *second* field of the struct is culpable. This
// requires traversing the HIR of the struct and calling `params_in_repr(Bar)`.
// But we can't call params_in_repr for a given type unless it is known to be
// representable. params_in_repr will cycle/panic on infinitely sized types.
// Looking at the query cycle above, we know that `Bar` is representable
// because `check_representability_adt_ty(Bar<..>)` is in the cycle and
// `check_representability(Bar)` is *not* in the cycle.
fn check_representability_adt_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Representability {
let ty::Adt(adt, args) = ty.kind() else { bug!("expected adt") };
if let Some(def_id) = adt.did().as_local() {
rtry!(tcx.representability(def_id));
let _ = tcx.check_representability(def_id);
}
// At this point, we know that the item of the ADT type is representable;
// but the type parameters may cause a cycle with an upstream type
let params_in_repr = tcx.params_in_repr(adt.did());
for (i, arg) in args.iter().enumerate() {
if let ty::GenericArgKind::Type(ty) = arg.kind() {
if params_in_repr.contains(i as u32) {
rtry!(representability_ty(tcx, ty));
check_representability_ty(tcx, ty);
}
}
}
Representability::Representable
Representability
}

fn params_in_repr(tcx: TyCtxt<'_>, def_id: LocalDefId) -> DenseBitSet<u32> {
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_ty_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,10 @@ fn adt_sizedness_constraint<'tcx>(
tcx: TyCtxt<'tcx>,
(def_id, sizedness): (DefId, SizedTraitKind),
) -> Option<ty::EarlyBinder<'tcx, Ty<'tcx>>> {
if let Some(def_id) = def_id.as_local()
&& let ty::Representability::Infinite(_) = tcx.representability(def_id)
{
return None;
if let Some(def_id) = def_id.as_local() {
let _ = tcx.check_representability(def_id);
}

let def = tcx.adt_def(def_id);

if !def.is_struct() {
Expand Down
1 change: 0 additions & 1 deletion tests/ui/enum-discriminant/issue-72554.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::collections::BTreeSet;
#[derive(Hash)]
pub enum ElemDerived {
//~^ ERROR recursive type `ElemDerived` has infinite size
//~| ERROR cycle detected
A(ElemDerived)
}

Expand Down
21 changes: 3 additions & 18 deletions tests/ui/enum-discriminant/issue-72554.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ error[E0072]: recursive type `ElemDerived` has infinite size
|
LL | pub enum ElemDerived {
| ^^^^^^^^^^^^^^^^^^^^
...
LL |
LL | A(ElemDerived)
| ----------- recursive without indirection
|
Expand All @@ -12,21 +12,6 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle
LL | A(Box<ElemDerived>)
| ++++ +

error[E0391]: cycle detected when computing drop-check constraints for `ElemDerived`
--> $DIR/issue-72554.rs:4:1
|
LL | pub enum ElemDerived {
| ^^^^^^^^^^^^^^^^^^^^
|
= note: ...which immediately requires computing drop-check constraints for `ElemDerived` again
note: cycle used when computing drop-check constraints for `Elem`
--> $DIR/issue-72554.rs:11:1
|
LL | pub enum Elem {
| ^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error: aborting due to 2 previous errors
error: aborting due to 1 previous error

Some errors have detailed explanations: E0072, E0391.
For more information about an error, try `rustc --explain E0072`.
For more information about this error, try `rustc --explain E0072`.
2 changes: 0 additions & 2 deletions tests/ui/infinite/infinite-struct.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
struct Take(Take);
//~^ ERROR has infinite size
//~| ERROR cycle
//~| ERROR reached the recursion limit finding the struct tail for `Take`

// check that we don't hang trying to find the tail of a recursive struct (#79437)
fn foo() -> Take {
Expand Down
25 changes: 3 additions & 22 deletions tests/ui/infinite/infinite-struct.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ LL | struct Take(Box<Take>);
| ++++ +

error[E0072]: recursive type `Foo` has infinite size
--> $DIR/infinite-struct.rs:12:1
--> $DIR/infinite-struct.rs:10:1
|
LL | struct Foo {
| ^^^^^^^^^^
Expand All @@ -22,25 +22,6 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle
LL | x: Bar<Box<Foo>>,
| ++++ +

error: reached the recursion limit finding the struct tail for `Take`
--> $DIR/infinite-struct.rs:1:1
|
LL | struct Take(Take);
| ^^^^^^^^^^^
|
= help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]`

error[E0391]: cycle detected when computing when `Take` needs drop
--> $DIR/infinite-struct.rs:1:1
|
LL | struct Take(Take);
| ^^^^^^^^^^^
|
= note: ...which immediately requires computing when `Take` needs drop again
= note: cycle used when computing whether `Take` needs drop
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error: aborting due to 4 previous errors
error: aborting due to 2 previous errors

Some errors have detailed explanations: E0072, E0391.
For more information about an error, try `rustc --explain E0072`.
For more information about this error, try `rustc --explain E0072`.
1 change: 0 additions & 1 deletion tests/ui/infinite/infinite-tag-type-recursion.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
enum MList { Cons(isize, MList), Nil }
//~^ ERROR recursive type `MList` has infinite size
//~| ERROR cycle

fn main() { let a = MList::Cons(10, MList::Cons(11, MList::Nil)); }
15 changes: 2 additions & 13 deletions tests/ui/infinite/infinite-tag-type-recursion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,6 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle
LL | enum MList { Cons(isize, Box<MList>), Nil }
| ++++ +

error[E0391]: cycle detected when computing when `MList` needs drop
--> $DIR/infinite-tag-type-recursion.rs:1:1
|
LL | enum MList { Cons(isize, MList), Nil }
| ^^^^^^^^^^
|
= note: ...which immediately requires computing when `MList` needs drop again
= note: cycle used when computing whether `MList` needs drop
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error: aborting due to 2 previous errors
error: aborting due to 1 previous error

Some errors have detailed explanations: E0072, E0391.
For more information about an error, try `rustc --explain E0072`.
For more information about this error, try `rustc --explain E0072`.
2 changes: 0 additions & 2 deletions tests/ui/query-system/query-cycle-printing-issue-151226.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
struct A<T>(std::sync::OnceLock<Self>);
//~^ ERROR recursive type `A` has infinite size
//~| ERROR cycle detected when computing layout of `A<()>`

static B: A<()> = todo!();
//~^ ERROR cycle occurred during layout computation

fn main() {}
26 changes: 2 additions & 24 deletions tests/ui/query-system/query-cycle-printing-issue-151226.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,6 @@ help: insert some indirection (e.g., a `Box`, `Rc`, or `&`) to break the cycle
LL | struct A<T>(Box<std::sync::OnceLock<Self>>);
| ++++ +

error[E0391]: cycle detected when computing layout of `A<()>`
|
= note: ...which requires computing layout of `std::sync::once_lock::OnceLock<A<()>>`...
= note: ...which requires computing layout of `core::cell::UnsafeCell<core::mem::maybe_uninit::MaybeUninit<A<()>>>`...
= note: ...which requires computing layout of `core::mem::maybe_uninit::MaybeUninit<A<()>>`...
= note: ...which requires computing layout of `core::mem::manually_drop::ManuallyDrop<A<()>>`...
= note: ...which requires computing layout of `core::mem::maybe_dangling::MaybeDangling<A<()>>`...
= note: ...which again requires computing layout of `A<()>`, completing the cycle
note: cycle used when checking that `B` is well-formed
--> $DIR/query-cycle-printing-issue-151226.rs:5:1
|
LL | static B: A<()> = todo!();
| ^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error[E0080]: a cycle occurred during layout computation
--> $DIR/query-cycle-printing-issue-151226.rs:5:1
|
LL | static B: A<()> = todo!();
| ^^^^^^^^^^^^^^^ evaluation of `B` failed here

error: aborting due to 3 previous errors
error: aborting due to 1 previous error

Some errors have detailed explanations: E0072, E0080, E0391.
For more information about an error, try `rustc --explain E0072`.
For more information about this error, try `rustc --explain E0072`.
1 change: 0 additions & 1 deletion tests/ui/structs-enums/enum-rec/issue-17431-6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::cell::UnsafeCell;

enum Foo { X(UnsafeCell<Option<Foo>>) }
//~^ ERROR recursive type `Foo` has infinite size
//~| ERROR cycle detected

impl Foo { fn bar(self) {} }

Expand Down
Loading
Loading