Skip to content

Commit

Permalink
Auto merge of #116849 - oli-obk:error_shenanigans, r=<try>
Browse files Browse the repository at this point in the history
Avoid a `track_errors` by bubbling up most errors from `check_well_formed`

I believe `track_errors` is mostly papering over issues that a sufficiently convoluted query graph can hit. I made this change, while the actual change I want to do is to stop bailing out early on errors, and instead use this new `ErrorGuaranteed` to invoke `check_well_formed` for individual items before doing all the `typeck` logic on them.

This works towards resolving #97477 and various other ICEs, as well as allowing us to use parallel rustc more (which is currently rather limited/bottlenecked due to the very sequential nature in which we do `rustc_hir_analysis::check_crate`)

cc `@SparrowLii` `@Zoxc` for the new `try_par_for_each_in` function
  • Loading branch information
bors committed Oct 18, 2023
2 parents b9832e7 + 0da8104 commit c5dfd1e
Show file tree
Hide file tree
Showing 33 changed files with 335 additions and 305 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ pub use worker_local::{Registry, WorkerLocal};
mod parallel;
#[cfg(parallel_compiler)]
pub use parallel::scope;
pub use parallel::{join, par_for_each_in, par_map, parallel_guard};
pub use parallel::{join, par_for_each_in, par_map, parallel_guard, try_par_for_each_in};

pub use std::sync::atomic::Ordering;
pub use std::sync::atomic::Ordering::SeqCst;
Expand Down
25 changes: 25 additions & 0 deletions compiler/rustc_data_structures/src/sync/parallel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ mod disabled {
})
}

pub fn try_par_for_each_in<T: IntoIterator, E: Copy>(
t: T,
mut for_each: impl FnMut(T::Item) -> Result<(), E>,
) -> Result<(), E> {
parallel_guard(|guard| {
t.into_iter().fold(Ok(()), |ret, i| guard.run(|| for_each(i)).unwrap_or(ret).and(ret))
})
}

pub fn par_map<T: IntoIterator, R, C: FromIterator<R>>(
t: T,
mut map: impl FnMut(<<T as IntoIterator>::IntoIter as Iterator>::Item) -> R,
Expand Down Expand Up @@ -167,6 +176,22 @@ mod enabled {
});
}

pub fn try_par_for_each_in<I, T: IntoIterator<Item = I> + IntoParallelIterator<Item = I>, E>(
t: T,
for_each: impl Fn(I) -> Result<(), E> + DynSync + DynSend,
) -> Result<(), E> {
parallel_guard(|guard| {
if mode::is_dyn_thread_safe() {
let for_each = FromDyn::from(for_each);
t.into_par_iter()
.fold(Ok(()), |ret, i| guard.run(|| for_each(i)).unwrap_or(ret).and(ret))
} else {
t.into_iter()
.fold(Ok(()), |ret, i| guard.run(|| for_each(i)).unwrap_or(ret).and(ret))
}
});
}

pub fn par_map<
I,
T: IntoIterator<Item = I> + IntoParallelIterator<Item = I>,
Expand Down
230 changes: 124 additions & 106 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs

Large diffs are not rendered by default.

6 changes: 2 additions & 4 deletions compiler/rustc_hir_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,8 @@ pub fn check_crate(tcx: TyCtxt<'_>) -> Result<(), ErrorGuaranteed> {
})?;
}

tcx.sess.track_errors(|| {
tcx.sess.time("wf_checking", || {
tcx.hir().par_for_each_module(|module| tcx.ensure().check_mod_type_wf(module))
});
tcx.sess.time("wf_checking", || {
tcx.hir().try_par_for_each_module(|module| tcx.check_mod_type_wf(module))
})?;

// NOTE: This is copy/pasted in librustdoc/core.rs and should be kept in sync.
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_ast as ast;
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::svh::Svh;
use rustc_data_structures::sync::{par_for_each_in, DynSend, DynSync};
use rustc_data_structures::sync::{par_for_each_in, try_par_for_each_in, DynSend, DynSync};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId, LOCAL_CRATE};
use rustc_hir::definitions::{DefKey, DefPath, DefPathData, DefPathHash};
Expand All @@ -16,7 +16,7 @@ use rustc_index::Idx;
use rustc_middle::hir::nested_filter;
use rustc_span::def_id::StableCrateId;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;
use rustc_span::{ErrorGuaranteed, Span};
use rustc_target::spec::abi::Abi;

#[inline]
Expand Down Expand Up @@ -632,6 +632,17 @@ impl<'hir> Map<'hir> {
})
}

#[inline]
pub fn try_par_for_each_module(
self,
f: impl Fn(LocalModDefId) -> Result<(), ErrorGuaranteed> + DynSend + DynSync,
) -> Result<(), ErrorGuaranteed> {
let crate_items = self.tcx.hir_crate_items(());
try_par_for_each_in(&crate_items.submodules[..], |module| {
f(LocalModDefId::new_unchecked(module.def_id))
})
}

/// Returns an iterator for the nodes in the ancestor tree of the `current_id`
/// until the crate root is reached. Prefer this over your own loop using `parent_id`.
#[inline]
Expand Down
32 changes: 22 additions & 10 deletions compiler/rustc_middle/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ pub mod place;
use crate::query::Providers;
use crate::ty::{EarlyBinder, ImplSubject, TyCtxt};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::sync::{par_for_each_in, DynSend, DynSync};
use rustc_data_structures::sync::{try_par_for_each_in, DynSend, DynSync};
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::*;
use rustc_query_system::ich::StableHashingContext;
use rustc_span::{ExpnId, DUMMY_SP};
use rustc_span::{ErrorGuaranteed, ExpnId, DUMMY_SP};

/// Top-level HIR node for current owner. This only contains the node for which
/// `HirId::local_id == 0`, and excludes bodies.
Expand Down Expand Up @@ -78,20 +78,32 @@ impl ModuleItems {
self.owners().map(|id| id.def_id)
}

pub fn par_items(&self, f: impl Fn(ItemId) + DynSend + DynSync) {
par_for_each_in(&self.items[..], |&id| f(id))
pub fn par_items(
&self,
f: impl Fn(ItemId) -> Result<(), ErrorGuaranteed> + DynSend + DynSync,
) -> Result<(), ErrorGuaranteed> {
try_par_for_each_in(&self.items[..], |&id| f(id))
}

pub fn par_trait_items(&self, f: impl Fn(TraitItemId) + DynSend + DynSync) {
par_for_each_in(&self.trait_items[..], |&id| f(id))
pub fn par_trait_items(
&self,
f: impl Fn(TraitItemId) -> Result<(), ErrorGuaranteed> + DynSend + DynSync,
) -> Result<(), ErrorGuaranteed> {
try_par_for_each_in(&self.trait_items[..], |&id| f(id))
}

pub fn par_impl_items(&self, f: impl Fn(ImplItemId) + DynSend + DynSync) {
par_for_each_in(&self.impl_items[..], |&id| f(id))
pub fn par_impl_items(
&self,
f: impl Fn(ImplItemId) -> Result<(), ErrorGuaranteed> + DynSend + DynSync,
) -> Result<(), ErrorGuaranteed> {
try_par_for_each_in(&self.impl_items[..], |&id| f(id))
}

pub fn par_foreign_items(&self, f: impl Fn(ForeignItemId) + DynSend + DynSync) {
par_for_each_in(&self.foreign_items[..], |&id| f(id))
pub fn par_foreign_items(
&self,
f: impl Fn(ForeignItemId) -> Result<(), ErrorGuaranteed> + DynSend + DynSync,
) -> Result<(), ErrorGuaranteed> {
try_par_for_each_in(&self.foreign_items[..], |&id| f(id))
}
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -975,8 +975,9 @@ rustc_queries! {
desc { |tcx| "checking that impls are well-formed in {}", describe_as_module(key, tcx) }
}

query check_mod_type_wf(key: LocalModDefId) -> () {
query check_mod_type_wf(key: LocalModDefId) -> Result<(), ErrorGuaranteed> {
desc { |tcx| "checking that types are well-formed in {}", describe_as_module(key, tcx) }
cache_on_disk_if { true }
}

query collect_mod_item_types(key: LocalModDefId) -> () {
Expand Down Expand Up @@ -1509,8 +1510,9 @@ rustc_queries! {
feedable
}

query check_well_formed(key: hir::OwnerId) -> () {
query check_well_formed(key: hir::OwnerId) -> Result<(), ErrorGuaranteed> {
desc { |tcx| "checking that `{}` is well-formed", tcx.def_path_str(key) }
cache_on_disk_if { true }
}

// The `DefId`s of all non-generic functions and statics in the given crate
Expand Down
14 changes: 2 additions & 12 deletions src/tools/clippy/tests/ui/crashes/ice-6252.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ help: you might be missing a type parameter
LL | impl<N, M, VAL> TypeVal<usize> for Multiply<N, M> where N: TypeVal<VAL> {}
| +++++

error[E0046]: not all trait items implemented, missing: `VAL`
--> $DIR/ice-6252.rs:11:1
|
LL | const VAL: T;
| ------------ `VAL` from trait
...
LL | impl<N, M> TypeVal<usize> for Multiply<N, M> where N: TypeVal<VAL> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `VAL` in implementation

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

Some errors have detailed explanations: E0046, E0412.
For more information about an error, try `rustc --explain E0046`.
For more information about this error, try `rustc --explain E0412`.
9 changes: 3 additions & 6 deletions tests/ui/associated-consts/issue-105330.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,10 @@ impl TraitWAssocConst for impl Demo { //~ ERROR E0404
}

fn foo<A: TraitWAssocConst<A=32>>() { //~ ERROR E0658
foo::<Demo>()(); //~ ERROR E0271
//~^ ERROR E0618
//~| ERROR E0277
foo::<Demo>()();
}

fn main<A: TraitWAssocConst<A=32>>() { //~ ERROR E0131
fn main<A: TraitWAssocConst<A=32>>() {
//~^ ERROR E0658
foo::<Demo>(); //~ ERROR E0277
//~^ ERROR E0271
foo::<Demo>();
}
86 changes: 4 additions & 82 deletions tests/ui/associated-consts/issue-105330.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ LL | fn foo<A: TraitWAssocConst<A=32>>() {
= help: add `#![feature(associated_const_equality)]` to the crate attributes to enable

error[E0658]: associated const equality is incomplete
--> $DIR/issue-105330.rs:17:29
--> $DIR/issue-105330.rs:15:29
|
LL | fn main<A: TraitWAssocConst<A=32>>() {
| ^^^^
Expand All @@ -39,85 +39,7 @@ error[E0562]: `impl Trait` only allowed in function and inherent method argument
LL | impl TraitWAssocConst for impl Demo {
| ^^^^^^^^^

error[E0131]: `main` function is not allowed to have generic parameters
--> $DIR/issue-105330.rs:17:8
|
LL | fn main<A: TraitWAssocConst<A=32>>() {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `main` cannot have generic parameters

error[E0277]: the trait bound `Demo: TraitWAssocConst` is not satisfied
--> $DIR/issue-105330.rs:12:11
|
LL | foo::<Demo>()();
| ^^^^ the trait `TraitWAssocConst` is not implemented for `Demo`
|
help: this trait has no implementations, consider adding one
--> $DIR/issue-105330.rs:1:1
|
LL | pub trait TraitWAssocConst {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
note: required by a bound in `foo`
--> $DIR/issue-105330.rs:11:11
|
LL | fn foo<A: TraitWAssocConst<A=32>>() {
| ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `foo`

error[E0271]: type mismatch resolving `<Demo as TraitWAssocConst>::A == 32`
--> $DIR/issue-105330.rs:12:11
|
LL | foo::<Demo>()();
| ^^^^ expected `32`, found `<Demo as TraitWAssocConst>::A`
|
= note: expected constant `32`
found constant `<Demo as TraitWAssocConst>::A`
note: required by a bound in `foo`
--> $DIR/issue-105330.rs:11:28
|
LL | fn foo<A: TraitWAssocConst<A=32>>() {
| ^^^^ required by this bound in `foo`

error[E0618]: expected function, found `()`
--> $DIR/issue-105330.rs:12:5
|
LL | fn foo<A: TraitWAssocConst<A=32>>() {
| ----------------------------------- `foo::<Demo>` defined here returns `()`
LL | foo::<Demo>()();
| ^^^^^^^^^^^^^--
| |
| call expression requires function

error[E0277]: the trait bound `Demo: TraitWAssocConst` is not satisfied
--> $DIR/issue-105330.rs:19:11
|
LL | foo::<Demo>();
| ^^^^ the trait `TraitWAssocConst` is not implemented for `Demo`
|
help: this trait has no implementations, consider adding one
--> $DIR/issue-105330.rs:1:1
|
LL | pub trait TraitWAssocConst {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
note: required by a bound in `foo`
--> $DIR/issue-105330.rs:11:11
|
LL | fn foo<A: TraitWAssocConst<A=32>>() {
| ^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `foo`

error[E0271]: type mismatch resolving `<Demo as TraitWAssocConst>::A == 32`
--> $DIR/issue-105330.rs:19:11
|
LL | foo::<Demo>();
| ^^^^ expected `32`, found `<Demo as TraitWAssocConst>::A`
|
= note: expected constant `32`
found constant `<Demo as TraitWAssocConst>::A`
note: required by a bound in `foo`
--> $DIR/issue-105330.rs:11:28
|
LL | fn foo<A: TraitWAssocConst<A=32>>() {
| ^^^^ required by this bound in `foo`

error: aborting due to 11 previous errors
error: aborting due to 5 previous errors

Some errors have detailed explanations: E0131, E0271, E0277, E0404, E0562, E0618, E0658.
For more information about an error, try `rustc --explain E0131`.
Some errors have detailed explanations: E0404, E0562, E0658.
For more information about an error, try `rustc --explain E0404`.
1 change: 1 addition & 0 deletions tests/ui/associated-consts/issue-58022.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ impl Bar<[u8]> {
const SIZE: usize = 32;

fn new(slice: &[u8; Self::SIZE]) -> Self {
//~^ ERROR: the size for values of type `[u8]` cannot be known at compilation time
Foo(Box::new(*slice))
//~^ ERROR: expected function, tuple struct or tuple variant, found trait `Foo`
}
Expand Down
22 changes: 18 additions & 4 deletions tests/ui/associated-consts/issue-58022.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,27 @@ LL |
LL | fn new(slice: &[u8; Foo::SIZE]) -> Self;
| ^^^^^^^^^ cannot refer to the associated constant of trait

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
--> $DIR/issue-58022.rs:13:41
|
LL | fn new(slice: &[u8; Self::SIZE]) -> Self {
| ^^^^ doesn't have a size known at compile-time
|
= help: within `Bar<[u8]>`, the trait `Sized` is not implemented for `[u8]`
note: required because it appears within the type `Bar<[u8]>`
--> $DIR/issue-58022.rs:8:12
|
LL | pub struct Bar<T: ?Sized>(T);
| ^^^
= note: the return type of a function must have a statically known size

error[E0423]: expected function, tuple struct or tuple variant, found trait `Foo`
--> $DIR/issue-58022.rs:14:9
--> $DIR/issue-58022.rs:15:9
|
LL | Foo(Box::new(*slice))
| ^^^ not a function, tuple struct or tuple variant

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

Some errors have detailed explanations: E0423, E0790.
For more information about an error, try `rustc --explain E0423`.
Some errors have detailed explanations: E0277, E0423, E0790.
For more information about an error, try `rustc --explain E0277`.
2 changes: 1 addition & 1 deletion tests/ui/async-await/issue-66312.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ trait Test<T> {

async fn f() {
let x = Some(2);
if x.is_some() {
if x.is_some() { //~ ERROR mismatched types
println!("Some");
}
}
Expand Down
11 changes: 9 additions & 2 deletions tests/ui/async-await/issue-66312.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ LL | fn is_some(self: T);
= note: type of `self` must be `Self` or a type that dereferences to it
= help: consider changing to `self`, `&self`, `&mut self`, `self: Box<Self>`, `self: Rc<Self>`, `self: Arc<Self>`, or `self: Pin<P>` (where P is one of the previous types except `Self`)

error: aborting due to previous error
error[E0308]: mismatched types
--> $DIR/issue-66312.rs:9:8
|
LL | if x.is_some() {
| ^^^^^^^^^^^ expected `bool`, found `()`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0307`.
Some errors have detailed explanations: E0307, E0308.
For more information about an error, try `rustc --explain E0307`.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn bad_infer_fn<_>() {}


fn main() {
let a: All<_, _, _>;
let a: All<_, _, _>; //~ ERROR struct takes 2 generic arguments but 3
all_fn();
let v: [u8; _];
let v: [u8; 10] = [0; _];
Expand Down
Loading

0 comments on commit c5dfd1e

Please sign in to comment.