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

Merge collect_mod_item_types query into check_well_formed #121500

Merged
merged 4 commits into from
Mar 8, 2024
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
25 changes: 20 additions & 5 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use crate::autoderef::Autoderef;
use crate::collect::CollectItemTypesVisitor;
use crate::constrained_generic_params::{identify_constrained_generic_params, Parameter};
use crate::errors;

use hir::intravisit::Visitor;
use rustc_ast as ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_errors::{codes::*, pluralize, struct_span_code_err, Applicability, ErrorGuaranteed};
Expand Down Expand Up @@ -225,6 +227,7 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) -> Result<()
?item.owner_id,
item.name = ? tcx.def_path_str(def_id)
);
CollectItemTypesVisitor { tcx }.visit_item(item);

let res = match item.kind {
// Right now we check that every default trait implementation
Expand Down Expand Up @@ -336,9 +339,14 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) -> Result<()
res
}

fn check_foreign_item(tcx: TyCtxt<'_>, item: &hir::ForeignItem<'_>) -> Result<(), ErrorGuaranteed> {
fn check_foreign_item<'tcx>(
tcx: TyCtxt<'tcx>,
item: &'tcx hir::ForeignItem<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let def_id = item.owner_id.def_id;

CollectItemTypesVisitor { tcx }.visit_foreign_item(item);

debug!(
?item.owner_id,
item.name = ? tcx.def_path_str(def_id)
Expand All @@ -355,12 +363,14 @@ fn check_foreign_item(tcx: TyCtxt<'_>, item: &hir::ForeignItem<'_>) -> Result<()
}
}

fn check_trait_item(
tcx: TyCtxt<'_>,
trait_item: &hir::TraitItem<'_>,
fn check_trait_item<'tcx>(
tcx: TyCtxt<'tcx>,
trait_item: &'tcx hir::TraitItem<'tcx>,
) -> Result<(), ErrorGuaranteed> {
let def_id = trait_item.owner_id.def_id;

CollectItemTypesVisitor { tcx }.visit_trait_item(trait_item);

let (method_sig, span) = match trait_item.kind {
hir::TraitItemKind::Fn(ref sig, _) => (Some(sig), trait_item.span),
hir::TraitItemKind::Type(_bounds, Some(ty)) => (None, ty.span),
Expand Down Expand Up @@ -895,7 +905,12 @@ fn check_object_unsafe_self_trait_by_name(tcx: TyCtxt<'_>, item: &hir::TraitItem
}
}

fn check_impl_item(tcx: TyCtxt<'_>, impl_item: &hir::ImplItem<'_>) -> Result<(), ErrorGuaranteed> {
fn check_impl_item<'tcx>(
tcx: TyCtxt<'tcx>,
impl_item: &'tcx hir::ImplItem<'tcx>,
) -> Result<(), ErrorGuaranteed> {
CollectItemTypesVisitor { tcx }.visit_impl_item(impl_item);

let (method_sig, span) = match impl_item.kind {
hir::ImplItemKind::Fn(ref sig, _) => (Some(sig), impl_item.span),
// Constrain binding and overflow error spans to `<Ty>` in `type foo = <Ty>`.
Expand Down
12 changes: 3 additions & 9 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_data_structures::unord::UnordMap;
use rustc_errors::{Applicability, Diag, ErrorGuaranteed, StashKey};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{GenericParamKind, Node};
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
Expand Down Expand Up @@ -52,11 +52,6 @@ mod resolve_bound_vars;
mod type_of;

///////////////////////////////////////////////////////////////////////////
// Main entry point

fn collect_mod_item_types(tcx: TyCtxt<'_>, module_def_id: LocalModDefId) {
tcx.hir().visit_item_likes_in_module(module_def_id, &mut CollectItemTypesVisitor { tcx });
}

pub fn provide(providers: &mut Providers) {
resolve_bound_vars::provide(providers);
Expand All @@ -82,7 +77,6 @@ pub fn provide(providers: &mut Providers) {
impl_trait_header,
coroutine_kind,
coroutine_for_closure,
collect_mod_item_types,
is_type_alias_impl_trait,
find_field,
..*providers
Expand Down Expand Up @@ -155,8 +149,8 @@ impl<'v> Visitor<'v> for HirPlaceholderCollector {
}
}

struct CollectItemTypesVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
pub struct CollectItemTypesVisitor<'tcx> {
pub tcx: TyCtxt<'tcx>,
}

/// If there are any placeholder types (`_`), emit an error explaining that this is not allowed
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,6 @@ pub fn provide(providers: &mut Providers) {
pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
let _prof_timer = tcx.sess.timer("type_check_crate");

// this ensures that later parts of type checking can assume that items
// have valid types and not error
tcx.sess.time("type_collecting", || {
tcx.hir().for_each_module(|module| tcx.ensure().collect_mod_item_types(module))
});

if tcx.features().rustc_attrs {
tcx.sess.time("outlives_testing", || outlives::test::test_inferred_outlives(tcx))?;
}
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,7 @@ rustc_queries! {
desc { |tcx| "computing the variances of `{}`", tcx.def_path_str(def_id) }
cache_on_disk_if { def_id.is_local() }
separate_provide_extern
cycle_delay_bug
}

/// Maps from thee `DefId` of a type to its (inferred) outlives.
Expand Down Expand Up @@ -960,10 +961,6 @@ rustc_queries! {
ensure_forwards_result_if_red
}

query collect_mod_item_types(key: LocalModDefId) {
desc { |tcx| "collecting item types in {}", describe_as_module(key, tcx) }
}

/// Caches `CoerceUnsized` kinds for impls on custom types.
query coerce_unsized_info(key: DefId) -> Result<ty::adjustment::CoerceUnsizedInfo, ErrorGuaranteed> {
desc { |tcx| "computing CoerceUnsized info for `{}`", tcx.def_path_str(key) }
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_middle/src/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,27 @@ impl<'tcx> Value<TyCtxt<'tcx>> for ty::EarlyBinder<ty::Binder<'_, ty::FnSig<'_>>
}
}

impl<'tcx> Value<TyCtxt<'tcx>> for &[ty::Variance] {
fn from_cycle_error(
tcx: TyCtxt<'tcx>,
cycle_error: &CycleError,
_guar: ErrorGuaranteed,
) -> Self {
if let Some(frame) = cycle_error.cycle.get(0)
&& frame.query.dep_kind == dep_kinds::variances_of
&& let Some(def_id) = frame.query.def_id
{
let n = tcx.generics_of(def_id).params.len();
vec![ty::Variance::Bivariant; n].leak()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain what happens here?
When will this impl be called? Is this logic for error recovery?

(Also tcx.arena.allocate could probably be used instead of leak.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When will this impl be called? Is this logic for error recovery?

This is not part of recovery, but of cycle_delay_bug. In order to hide cycle errors in favor of more useful errors, we can make the query cycle turn into a span_delayed_bug. But then we need a value to return from the query, which is where all the impls in this module are invoked. They produce a dummy value for the query.

(Also tcx.arena.allocate could probably be used instead of leak.)

Then I would have to transmute the result 😆

I guess if someone is running rustc in a loop from within the same process, and keep hitting this cycle error, they would slowly lose memory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

edit: leak is already being used elsewhere in this file, so this is more of a general point that we could figure out, but it seems low priority, so I think just avoiding the unsafe code is best

Copy link
Contributor

@petrochenkov petrochenkov Mar 7, 2024

Choose a reason for hiding this comment

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

This is not part of recovery

I mean ErrorGuaranteed is passed to this function, so some error already happened (or at least was stashed)?
So this dummy value is never created on a good path.

Edit: the bit about span_delayed_bug is also not clear, if we have an ErrorGuaranteed then the delayed bug will never be reported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

span_delayed_bug creates an ErrorGuaranteed without emitting an error. It will ICE the compiler if you forgot to emit an error later

} else {
span_bug!(
cycle_error.usage.as_ref().unwrap().0,
"only `variances_of` returns `&[ty::Variance]`"
);
}
}
}

// Take a cycle of `Q` and try `try_cycle` on every permutation, falling back to `otherwise`.
fn search_for_cycle_permutation<Q, T>(
cycle: &[Q],
Expand Down
11 changes: 0 additions & 11 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,21 +314,10 @@ pub(crate) fn run_global_ctxt(
// typeck function bodies or run the default rustc lints.
// (see `override_queries` in the `config`)

// HACK(jynelson) this calls an _extremely_ limited subset of `typeck`
// and might break if queries change their assumptions in the future.
tcx.sess.time("type_collecting", || {
tcx.hir().for_each_module(|module| tcx.ensure().collect_mod_item_types(module))
});

// NOTE: These are copy/pasted from typeck/lib.rs and should be kept in sync with those changes.
let _ = tcx.sess.time("wf_checking", || {
tcx.hir().try_par_for_each_module(|module| tcx.ensure().check_mod_type_wf(module))
});
tcx.sess.time("item_types_checking", || {
tcx.hir().for_each_module(|module| {
let _ = tcx.ensure().check_mod_type_wf(module);
});
});

if let Some(guar) = tcx.dcx().has_errors() {
return Err(guar);
Expand Down
12 changes: 3 additions & 9 deletions tests/rustdoc-ui/issue-110629-private-type-cycle-dyn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,11 @@ LL | type Bar<'a, 'b> = Box<dyn PartialEq<Bar<'a, 'b>>>;
= 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
note: cycle used when checking that `Bar` is well-formed
--> $DIR/issue-110629-private-type-cycle-dyn.rs:1:1
|
LL | / type Bar<'a, 'b> = Box<dyn PartialEq<Bar<'a, 'b>>>;
LL | |
LL | |
LL | | fn bar<'a, 'b>(i: &'a i32) -> Bar<'a, 'b> {
... |
LL | | assert!(bar(&meh) == bar(&muh));
LL | | }
| |_^
LL | type Bar<'a, 'b> = Box<dyn PartialEq<Bar<'a, 'b>>>;
| ^^^^^^^^^^^^^^^^
= 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 1 previous error
Expand Down
64 changes: 32 additions & 32 deletions tests/rustdoc-ui/issues/issue-105742.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -464,38 +464,6 @@ help: add missing generic argument
LL | Output = <Self as SVec>::Item> as SVec>::Item<T>,
| +++

error[E0107]: missing generics for associated type `SVec::Item`
--> $DIR/issue-105742.rs:61:38
|
LL | fn len(&self) -> <Self as SVec>::Item;
| ^^^^ expected 1 lifetime argument
|
note: associated type defined here, with 1 lifetime parameter: `'a`
--> $DIR/issue-105742.rs:59:10
|
LL | type Item<'a, T>;
| ^^^^ --
help: add missing lifetime argument
|
LL | fn len(&self) -> <Self as SVec>::Item<'_>;
| ++++

error[E0107]: missing generics for associated type `SVec::Item`
--> $DIR/issue-105742.rs:61:38
|
LL | fn len(&self) -> <Self as SVec>::Item;
| ^^^^ expected 1 generic argument
|
note: associated type defined here, with 1 generic parameter: `T`
--> $DIR/issue-105742.rs:59:10
|
LL | type Item<'a, T>;
| ^^^^ -
help: add missing generic argument
|
LL | fn len(&self) -> <Self as SVec>::Item<T>;
| +++

error[E0107]: missing generics for associated type `SVec::Item`
--> $DIR/issue-105742.rs:15:21
|
Expand Down Expand Up @@ -632,6 +600,38 @@ help: add missing generic argument
LL | Output = <Self as SVec>::Item> as SVec>::Item<T>,
| +++

error[E0107]: missing generics for associated type `SVec::Item`
--> $DIR/issue-105742.rs:61:38
|
LL | fn len(&self) -> <Self as SVec>::Item;
| ^^^^ expected 1 lifetime argument
|
note: associated type defined here, with 1 lifetime parameter: `'a`
--> $DIR/issue-105742.rs:59:10
|
LL | type Item<'a, T>;
| ^^^^ --
help: add missing lifetime argument
|
LL | fn len(&self) -> <Self as SVec>::Item<'_>;
| ++++

error[E0107]: missing generics for associated type `SVec::Item`
--> $DIR/issue-105742.rs:61:38
|
LL | fn len(&self) -> <Self as SVec>::Item;
| ^^^^ expected 1 generic argument
|
note: associated type defined here, with 1 generic parameter: `T`
--> $DIR/issue-105742.rs:59:10
|
LL | type Item<'a, T>;
| ^^^^ -
help: add missing generic argument
|
LL | fn len(&self) -> <Self as SVec>::Item<T>;
| +++

error: aborting due to 37 previous errors

Some errors have detailed explanations: E0038, E0107.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,11 @@ note: ...which requires computing normalized predicates of `Foo`...
LL | struct Foo {
| ^^^^^^^^^^
= note: ...which again requires computing predicates of `Foo`, completing the cycle
note: cycle used when collecting item types in top-level module
--> $DIR/cycle-iat-inside-of-adt.rs:3:1
note: cycle used when checking that `Foo` is well-formed
--> $DIR/cycle-iat-inside-of-adt.rs:7:1
|
LL | / #![feature(inherent_associated_types)]
LL | | #![allow(incomplete_features)]
LL | | // FIXME(inherent_associated_types): This should pass.
LL | |
... |
LL | |
LL | | fn main() {}
| |____________^
LL | struct Foo {
| ^^^^^^^^^^
= 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 1 previous error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,23 @@ note: ...which requires computing normalized predicates of `user`...
LL | fn user<T>() where S<T>::P: std::fmt::Debug {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which again requires computing predicates of `user`, completing the cycle
note: cycle used when collecting item types in top-level module
--> $DIR/cycle-iat-inside-of-where-predicate.rs:3:1
|
LL | / #![feature(inherent_associated_types)]
LL | | #![allow(incomplete_features)]
LL | |
LL | | // FIXME(inherent_associated_types): This shouldn't lead to a cycle error.
... |
LL | |
LL | | fn main() {}
| |____________^
note: cycle used when checking that `user` is well-formed
--> $DIR/cycle-iat-inside-of-where-predicate.rs:8:1
|
LL | fn user<T>() where S<T>::P: std::fmt::Debug {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= 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 1 previous error
error[E0392]: type parameter `T` is never used
--> $DIR/cycle-iat-inside-of-where-predicate.rs:10:10
|
LL | struct S<T>;
| ^ unused type parameter
|
= help: consider removing `T`, referring to it in a field, or using a marker such as `PhantomData`
= help: if you intended `T` to be a const parameter, use `const T: /* Type */` instead

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0391`.
Some errors have detailed explanations: E0391, E0392.
For more information about an error, try `rustc --explain E0391`.
20 changes: 10 additions & 10 deletions tests/ui/associated-inherent-types/issue-109071.no_gate.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,6 @@ help: add missing generic argument
LL | impl<T> Windows<T> {
| +++

error[E0658]: inherent associated types are unstable
--> $DIR/issue-109071.rs:8:5
|
LL | type Item = &[T];
| ^^^^^^^^^^^^^^^^^
|
= note: see issue #8995 </~https://github.com/rust-lang/rust/issues/8995> for more information
= help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0223]: ambiguous associated type
--> $DIR/issue-109071.rs:15:22
|
Expand All @@ -43,6 +33,16 @@ LL | fn T() -> Option<<Windows<T> as IntoAsyncIterator>::Item> {}
LL | fn T() -> Option<<Windows<T> as IntoIterator>::Item> {}
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error[E0658]: inherent associated types are unstable
--> $DIR/issue-109071.rs:8:5
|
LL | type Item = &[T];
| ^^^^^^^^^^^^^^^^^
|
= note: see issue #8995 </~https://github.com/rust-lang/rust/issues/8995> for more information
= help: add `#![feature(inherent_associated_types)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error: aborting due to 4 previous errors

Some errors have detailed explanations: E0107, E0223, E0637, E0658.
Expand Down
Loading
Loading