Skip to content

Merge collect_mod_item_types query into check_well_formed #121500

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.

Sign up for GitHub

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 4 commits into from
Mar 8, 2024
Merged

Merge collect_mod_item_types query into check_well_formed #121500

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de3fb8d
Collect mod item types in parallel, just like wfcheck
oli-obk Feb 23, 2024
ebf1b92
Use the same collection order as check_mod_type_wf
oli-obk Feb 23, 2024
42ab88d
Remove redundant query invocations in rustdoc
oli-obk Feb 23, 2024
ae50e36
Merge collect_mod_item_types query into check_well_formed
oli-obk Feb 23, 2024
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_hir_analysis/src/collect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,18 @@ 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 });
let items = tcx.hir_module_items(module_def_id);
let hir = tcx.hir();
let _ = items.par_items(|item| Ok(CollectItemTypesVisitor { tcx }.visit_item(hir.item(item))));
let _ = items.par_trait_items(|item| {
Ok(CollectItemTypesVisitor { tcx }.visit_trait_item(hir.trait_item(item)))
});
let _ = items.par_impl_items(|item| {
Ok(CollectItemTypesVisitor { tcx }.visit_impl_item(hir.impl_item(item)))
});
let _ = items.par_foreign_items(|item| {
Ok(CollectItemTypesVisitor { tcx }.visit_foreign_item(hir.foreign_item(item)))
});
Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I'd expect having something like par_visit_item_likes_in_module instead of doing things like this here or in wf checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That can't be done nicely, because visit_item_likes_in_module uses a visitor. I guess we could add something that creates the visitor 4 times just like I did here, but considering after this PR we again have only a single use site of such a par_visit_item_likes_in_module method, keeping it inline at the one use site seems best to me?

Copy link
Contributor

@petrochenkov petrochenkov Mar 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

considering after this PR we again have only a single use site of such a par_visit_item_likes_in_module method

If overhead of parallel queries was low enough, then I'd expect pretty much every visit_item_likes_in_modules and visit_all_item_likes_in_crates to benefit from being turned into its par_ version.
(Although maybe we'll need a special Par version of visitor for this.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add it to the list of things I am investigating around the parallel compiler

}

pub fn provide(providers: &mut Providers) {
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/suggestions/suggest-swapping-self-ty-and-trait-edition-2021.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ impl<'a, T> Struct<T> for Trait<'a, T> {}

impl<'a, T> Enum<T> for Trait<'a, T> {}
//~^ ERROR expected trait, found enum `Enum`
//~| ERROR trait objects must include the `dyn` keyword

impl<'a, T> Union<T> for Trait<'a, T> {}
//~^ ERROR expected trait, found union `Union`
//~| ERROR trait objects must include the `dyn` keyword

fn main() {}
26 changes: 24 additions & 2 deletions tests/ui/suggestions/suggest-swapping-self-ty-and-trait-edition-2021.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LL | impl<'a, T> Trait<'a, T> for Enum<T> {}
| ~~~~~~~~~~~~ ~~~~~~~

error[E0404]: expected trait, found union `Union`
--> $DIR/suggest-swapping-self-ty-and-trait-edition-2021.rs:19:13
--> $DIR/suggest-swapping-self-ty-and-trait-edition-2021.rs:20:13
|
LL | impl<'a, T> Union<T> for Trait<'a, T> {}
| ^^^^^^^^ not a trait
Expand All @@ -42,7 +42,29 @@ help: add `dyn` keyword before this trait
LL | impl<'a, T> Struct<T> for dyn Trait<'a, T> {}
| +++

error: aborting due to 4 previous errors
error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/suggest-swapping-self-ty-and-trait-edition-2021.rs:16:25
|
LL | impl<'a, T> Enum<T> for Trait<'a, T> {}
| ^^^^^^^^^^^^
|
help: add `dyn` keyword before this trait
|
LL | impl<'a, T> Enum<T> for dyn Trait<'a, T> {}
| +++

error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/suggest-swapping-self-ty-and-trait-edition-2021.rs:20:26
|
LL | impl<'a, T> Union<T> for Trait<'a, T> {}
| ^^^^^^^^^^^^
|
help: add `dyn` keyword before this trait
|
LL | impl<'a, T> Union<T> for dyn Trait<'a, T> {}
| +++

error: aborting due to 6 previous errors

Some errors have detailed explanations: E0404, E0782.
For more information about an error, try `rustc --explain E0404`.
4 changes: 4 additions & 0 deletions tests/ui/suggestions/suggest-swapping-self-ty-and-trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,12 @@ impl<'a, T> Struct<T> for Trait<'a, T> {}

impl<'a, T> Enum<T> for Trait<'a, T> {}
//~^ ERROR expected trait, found enum `Enum`
//~| WARNING trait objects without an explicit `dyn` are deprecated
//~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!

impl<'a, T> Union<T> for Trait<'a, T> {}
//~^ ERROR expected trait, found union `Union`
//~| WARNING trait objects without an explicit `dyn` are deprecated
//~| WARNING this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!

fn main() {}
30 changes: 28 additions & 2 deletions tests/ui/suggestions/suggest-swapping-self-ty-and-trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LL | impl<'a, T> Trait<'a, T> for Enum<T> {}
| ~~~~~~~~~~~~ ~~~~~~~

error[E0404]: expected trait, found union `Union`
--> $DIR/suggest-swapping-self-ty-and-trait.rs:18:13
--> $DIR/suggest-swapping-self-ty-and-trait.rs:20:13
|
LL | impl<'a, T> Union<T> for Trait<'a, T> {}
| ^^^^^^^^ not a trait
Expand All @@ -45,6 +45,32 @@ help: if this is an object-safe trait, use `dyn`
LL | impl<'a, T> Struct<T> for dyn Trait<'a, T> {}
| +++

error: aborting due to 3 previous errors; 1 warning emitted
warning: trait objects without an explicit `dyn` are deprecated
--> $DIR/suggest-swapping-self-ty-and-trait.rs:15:25
|
LL | impl<'a, T> Enum<T> for Trait<'a, T> {}
| ^^^^^^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
help: if this is an object-safe trait, use `dyn`
|
LL | impl<'a, T> Enum<T> for dyn Trait<'a, T> {}
| +++

warning: trait objects without an explicit `dyn` are deprecated
--> $DIR/suggest-swapping-self-ty-and-trait.rs:20:26
|
LL | impl<'a, T> Union<T> for Trait<'a, T> {}
| ^^^^^^^^^^^^
|
= warning: this is accepted in the current edition (Rust 2015) but is a hard error in Rust 2021!
= note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/warnings-promoted-to-error.html>
help: if this is an object-safe trait, use `dyn`
|
LL | impl<'a, T> Union<T> for dyn Trait<'a, T> {}
| +++

error: aborting due to 3 previous errors; 3 warnings emitted

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