Skip to content

Improve errors for recursive type aliases #88121

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 6 commits into from
Sep 1, 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
13 changes: 12 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,18 @@ rustc_queries! {

/// Records the type of every item.
query type_of(key: DefId) -> Ty<'tcx> {
desc { |tcx| "computing type of `{}`", tcx.def_path_str(key) }
desc { |tcx|
"{action} `{path}`",
action = {
use rustc_hir::def::DefKind;
match tcx.def_kind(key) {
DefKind::TyAlias => "expanding type alias",
DefKind::TraitAlias => "expanding trait alias",
_ => "computing type of",
}
},
path = tcx.def_path_str(key),
}
cache_on_disk_if { key.is_local() }
}

Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_query_impl/src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ pub trait Key {
/// In the event that a cycle occurs, if no explicit span has been
/// given for a query with key `self`, what span should we use?
fn default_span(&self, tcx: TyCtxt<'_>) -> Span;

/// If the key is a [`DefId`] or `DefId`--equivalent, return that `DefId`.
/// Otherwise, return `None`.
fn key_as_def_id(&self) -> Option<DefId> {
None
}
}

impl Key for () {
Expand Down Expand Up @@ -95,6 +101,9 @@ impl Key for LocalDefId {
fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
self.to_def_id().default_span(tcx)
}
fn key_as_def_id(&self) -> Option<DefId> {
Some(self.to_def_id())
}
}

impl Key for DefId {
Expand All @@ -105,6 +114,10 @@ impl Key for DefId {
fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
tcx.def_span(*self)
}
#[inline(always)]
fn key_as_def_id(&self) -> Option<DefId> {
Some(*self)
}
}

impl Key for ty::WithOptConstParam<LocalDefId> {
Expand Down Expand Up @@ -165,6 +178,10 @@ impl Key for (DefId, Option<Ident>) {
fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
tcx.def_span(self.0)
}
#[inline(always)]
fn key_as_def_id(&self) -> Option<DefId> {
Some(self.0)
}
}

impl Key for (DefId, LocalDefId, Ident) {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_query_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ pub use on_disk_cache::OnDiskCache;
mod profiling_support;
pub use self::profiling_support::alloc_self_profile_query_strings;

mod util;

rustc_query_append! { [define_queries!][<'tcx>] }

impl<'tcx> Queries<'tcx> {
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,13 @@ macro_rules! define_queries {
} else {
Some(key.default_span(*tcx))
};
let def_id = key.key_as_def_id();
let def_kind = def_id
.and_then(|def_id| def_id.as_local())
// Use `tcx.hir().opt_def_kind()` to reduce the chance of
// accidentally triggering an infinite query loop.
.and_then(|def_id| tcx.hir().opt_def_kind(def_id))
.map(|def_kind| $crate::util::def_kind_to_simple_def_kind(def_kind));
let hash = || {
let mut hcx = tcx.create_stable_hashing_context();
let mut hasher = StableHasher::new();
Expand All @@ -345,7 +352,7 @@ macro_rules! define_queries {
hasher.finish::<u64>()
};

QueryStackFrame::new(name, description, span, hash)
QueryStackFrame::new(name, description, span, def_kind, hash)
})*
}

Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_query_impl/src/util.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use rustc_hir::def::DefKind;
use rustc_query_system::query::SimpleDefKind;

/// Convert a [`DefKind`] to a [`SimpleDefKind`].
///
/// *See [`SimpleDefKind`]'s docs for more information.*
pub(crate) fn def_kind_to_simple_def_kind(def_kind: DefKind) -> SimpleDefKind {
match def_kind {
DefKind::Struct => SimpleDefKind::Struct,
DefKind::Enum => SimpleDefKind::Enum,
DefKind::Union => SimpleDefKind::Union,
DefKind::Trait => SimpleDefKind::Trait,
DefKind::TyAlias => SimpleDefKind::TyAlias,
DefKind::TraitAlias => SimpleDefKind::TraitAlias,

_ => SimpleDefKind::Other,
}
}
33 changes: 28 additions & 5 deletions compiler/rustc_query_system/src/query/job.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::dep_graph::DepContext;
use crate::query::plumbing::CycleError;
use crate::query::{QueryContext, QueryStackFrame};
use crate::query::{QueryContext, QueryStackFrame, SimpleDefKind};

use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, Handler, Level};
Expand Down Expand Up @@ -591,10 +591,33 @@ pub(crate) fn report_cycle<'a>(
err.span_note(span, &format!("...which requires {}...", query.description));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm intrigued if the iteration above doesn't need to be reverted to be correct 🤔 (no need to address now)


err.note(&format!(
"...which again requires {}, completing the cycle",
stack[0].query.description
));
if stack.len() == 1 {
err.note(&format!("...which immediately requires {} again", stack[0].query.description));
} else {
err.note(&format!(
"...which again requires {}, completing the cycle",
stack[0].query.description
));
}

if stack.iter().all(|entry| {
entry.query.def_kind.map_or(false, |def_kind| {
matches!(def_kind, SimpleDefKind::TyAlias | SimpleDefKind::TraitAlias)
})
}) {
if stack.iter().all(|entry| {
entry
.query
.def_kind
.map_or(false, |def_kind| matches!(def_kind, SimpleDefKind::TyAlias))
}) {
err.note("type aliases cannot be recursive");
err.help("consider using a struct, enum, or union instead to break the cycle");
err.help("see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information");
} else {
err.note("trait aliases cannot be recursive");
}
}

if let Some((span, query)) = usage {
err.span_note(fix_span(span, &query), &format!("cycle used when {}", query.description));
Expand Down
29 changes: 29 additions & 0 deletions compiler/rustc_query_system/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,53 @@ pub struct QueryStackFrame {
pub name: &'static str,
pub description: String,
span: Option<Span>,
/// The `DefKind` this query frame is associated with, if applicable.
///
/// We can't use `rustc_hir::def::DefKind` because `rustc_hir` is not
/// available in `rustc_query_system`. Instead, we have a simplified
/// custom version of it, called [`SimpleDefKind`].
def_kind: Option<SimpleDefKind>,
/// This hash is used to deterministically pick
/// a query to remove cycles in the parallel compiler.
#[cfg(parallel_compiler)]
hash: u64,
}

/// A simplified version of `rustc_hir::def::DefKind`.
///
/// It was added to help improve cycle errors caused by recursive type aliases.
/// As of August 2021, `rustc_query_system` cannot depend on `rustc_hir`
/// because it would create a dependency cycle. So, instead, a simplified
/// version of `DefKind` was added to `rustc_query_system`.
///
/// `DefKind`s are converted to `SimpleDefKind`s in `rustc_query_impl`.
#[derive(Debug, Copy, Clone)]
pub enum SimpleDefKind {
Struct,
Enum,
Union,
Trait,
TyAlias,
TraitAlias,

// FIXME: add more from `rustc_hir::def::DefKind` and then remove `Other`
Other,
}

impl QueryStackFrame {
#[inline]
pub fn new(
name: &'static str,
description: String,
span: Option<Span>,
def_kind: Option<SimpleDefKind>,
_hash: impl FnOnce() -> u64,
) -> Self {
Self {
name,
description,
span,
def_kind,
#[cfg(parallel_compiler)]
hash: _hash(),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0391]: cycle detected when computing the super traits of `Baz` with assoc
LL | trait Baz: Foo + Bar<Self::Item> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ...which again requires computing the super traits of `Baz` with associated type name `Item`, completing the cycle
= note: ...which immediately requires computing the super traits of `Baz` with associated type name `Item` again
note: cycle used when computing the super traits of `Baz`
--> $DIR/ambiguous-associated-type2.rs:7:1
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ error[E0391]: cycle detected when building specialization graph of trait `Trait`
LL | trait Trait<T> { type Assoc; }
| ^^^^^^^^^^^^^^
|
= note: ...which again requires building specialization graph of trait `Trait`, completing the cycle
= note: ...which immediately requires building specialization graph of trait `Trait` again
note: cycle used when coherence checking all impls of trait `Trait`
--> $DIR/coherence-inherited-assoc-ty-cycle-err.rs:9:1
|
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/cycle-trait/cycle-trait-default-type-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0391]: cycle detected when computing type of `Foo::X`
LL | trait Foo<X = Box<dyn Foo>> {
| ^^^
|
= note: ...which again requires computing type of `Foo::X`, completing the cycle
= note: ...which immediately requires computing type of `Foo::X` again
note: cycle used when collecting item types in top-level module
--> $DIR/cycle-trait-default-type-trait.rs:4:1
|
Expand All @@ -17,7 +17,7 @@ error[E0391]: cycle detected when computing type of `Foo::X`
LL | trait Foo<X = Box<dyn Foo>> {
| ^^^
|
= note: ...which again requires computing type of `Foo::X`, completing the cycle
= note: ...which immediately requires computing type of `Foo::X` again
note: cycle used when collecting item types in top-level module
--> $DIR/cycle-trait-default-type-trait.rs:4:1
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/infinite/infinite-struct.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ error[E0391]: cycle detected when computing drop-check constraints for `Take`
LL | struct Take(Take);
| ^^^^^^^^^^^^^^^^^^
|
= note: ...which again requires computing drop-check constraints for `Take`, completing the cycle
= note: ...which immediately requires computing drop-check constraints for `Take` again
= note: cycle used when computing dropck types for `Canonical { max_universe: U0, variables: [], value: ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: UserFacing }, value: Take } }`

error: aborting due to 2 previous errors
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/infinite/infinite-tag-type-recursion.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ error[E0391]: cycle detected when computing drop-check constraints for `MList`
LL | enum MList { Cons(isize, MList), Nil }
| ^^^^^^^^^^
|
= note: ...which again requires computing drop-check constraints for `MList`, completing the cycle
= note: ...which immediately requires computing drop-check constraints for `MList` again
= note: cycle used when computing dropck types for `Canonical { max_universe: U0, variables: [], value: ParamEnvAnd { param_env: ParamEnv { caller_bounds: [], reveal: UserFacing }, value: MList } }`

error: aborting due to 2 previous errors
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/infinite/infinite-trait-alias-recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#![feature(trait_alias)]

trait T1 = T2;
//~^ ERROR cycle detected when computing the super predicates of `T1`

trait T2 = T3;

trait T3 = T1 + T3;

fn main() {}
42 changes: 42 additions & 0 deletions src/test/ui/infinite/infinite-trait-alias-recursion.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
error[E0391]: cycle detected when computing the super predicates of `T1`
--> $DIR/infinite-trait-alias-recursion.rs:3:1
|
LL | trait T1 = T2;
| ^^^^^^^^^^^^^^
Comment on lines +1 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

This should likely point at only T1.

|
note: ...which requires computing the super traits of `T1`...
--> $DIR/infinite-trait-alias-recursion.rs:3:12
|
LL | trait T1 = T2;
| ^^
note: ...which requires computing the super predicates of `T2`...
--> $DIR/infinite-trait-alias-recursion.rs:6:1
|
LL | trait T2 = T3;
| ^^^^^^^^^^^^^^
note: ...which requires computing the super traits of `T2`...
--> $DIR/infinite-trait-alias-recursion.rs:6:12
|
LL | trait T2 = T3;
| ^^
note: ...which requires computing the super predicates of `T3`...
--> $DIR/infinite-trait-alias-recursion.rs:8:1
|
LL | trait T3 = T1 + T3;
| ^^^^^^^^^^^^^^^^^^^
note: ...which requires computing the super traits of `T3`...
--> $DIR/infinite-trait-alias-recursion.rs:8:12
|
LL | trait T3 = T1 + T3;
| ^^
= note: ...which again requires computing the super predicates of `T1`, completing the cycle
= note: trait aliases cannot be recursive
note: cycle used when collecting item types in top-level module
--> $DIR/infinite-trait-alias-recursion.rs:3:1
|
LL | trait T1 = T2;
| ^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
6 changes: 6 additions & 0 deletions src/test/ui/infinite/infinite-type-alias-mutual-recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type X1 = X2;
//~^ ERROR cycle detected when expanding type alias `X1`
type X2 = X3;
type X3 = X1;

fn main() {}
34 changes: 34 additions & 0 deletions src/test/ui/infinite/infinite-type-alias-mutual-recursion.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error[E0391]: cycle detected when expanding type alias `X1`
--> $DIR/infinite-type-alias-mutual-recursion.rs:1:11
|
LL | type X1 = X2;
| ^^
|
note: ...which requires expanding type alias `X2`...
--> $DIR/infinite-type-alias-mutual-recursion.rs:3:11
|
LL | type X2 = X3;
| ^^
note: ...which requires expanding type alias `X3`...
--> $DIR/infinite-type-alias-mutual-recursion.rs:4:11
|
LL | type X3 = X1;
| ^^
= note: ...which again requires expanding type alias `X1`, completing the cycle
= note: type aliases cannot be recursive
= help: consider using a struct, enum, or union instead to break the cycle
= help: see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information
note: cycle used when collecting item types in top-level module
--> $DIR/infinite-type-alias-mutual-recursion.rs:1:1
|
LL | / type X1 = X2;
LL | |
LL | | type X2 = X3;
LL | | type X3 = X1;
LL | |
LL | | fn main() {}
| |____________^
Comment on lines +21 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

This reminds me, we need a way to talk about files without showing a snippet.


error: aborting due to previous error

For more information about this error, try `rustc --explain E0391`.
7 changes: 5 additions & 2 deletions src/test/ui/infinite/infinite-vec-type-recursion.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
error[E0391]: cycle detected when computing type of `X`
error[E0391]: cycle detected when expanding type alias `X`
--> $DIR/infinite-vec-type-recursion.rs:1:14
|
LL | type X = Vec<X>;
| ^
|
= note: ...which again requires computing type of `X`, completing the cycle
= note: ...which immediately requires expanding type alias `X` again
= note: type aliases cannot be recursive
= help: consider using a struct, enum, or union instead to break the cycle
= help: see <https://doc.rust-lang.org/reference/types.html#recursive-types> for more information
note: cycle used when collecting item types in top-level module
--> $DIR/infinite-vec-type-recursion.rs:1:1
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-20772.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | |
LL | | {}
| |__^
|
= note: ...which again requires computing the super traits of `T` with associated type name `Item`, completing the cycle
= note: ...which immediately requires computing the super traits of `T` with associated type name `Item` again
note: cycle used when computing the super traits of `T`
--> $DIR/issue-20772.rs:1:1
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-20825.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0391]: cycle detected when computing the super traits of `Processor` with
LL | pub trait Processor: Subscriber<Input = Self::Input> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: ...which again requires computing the super traits of `Processor` with associated type name `Input`, completing the cycle
= note: ...which immediately requires computing the super traits of `Processor` with associated type name `Input` again
note: cycle used when computing the super traits of `Processor`
--> $DIR/issue-20825.rs:5:1
|
Expand Down
Loading