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

[Merged by Bors] - Miri can set thread names now #5108

Closed
wants to merge 2 commits into from

Conversation

RalfJung
Copy link
Contributor

@RalfJung RalfJung commented Jun 26, 2022

Objective

rust-lang/miri#1717 has been fixed so we can set thread names in Miri now.

Solution

We set thread names in Miri.

@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events A-Build-System Related to build systems or continuous integration labels Jun 26, 2022
@alice-i-cecile alice-i-cecile requested a review from BoxyUwU June 26, 2022 19:05
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Thanks!

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jun 26, 2022
@@ -102,9 +102,6 @@ impl TaskPool {
let ex = Arc::clone(&executor);
let shutdown_rx = shutdown_rx.clone();

// miri does not support setting thread names
// TODO: change back when https://github.com/rust-lang/miri/issues/1717 is fixed
#[cfg(not(miri))]
let mut thread_builder = {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we'd have used a block expression here if it weren't for the cfg

Copy link
Member

@james7132 james7132 left a comment

Choose a reason for hiding this comment

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

I made the same/similar changes in #4740., but that one might require more scrutiny before merging.

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Jun 26, 2022
# Objective

rust-lang/miri#1717 has been fixed so we can set thread names in Miri now.

## Solution

We set thread names in Miri.
@bors bors bot changed the title Miri can set thread names now [Merged by Bors] - Miri can set thread names now Jun 26, 2022
@bors bors bot closed this Jun 26, 2022
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

rust-lang/miri#1717 has been fixed so we can set thread names in Miri now.

## Solution

We set thread names in Miri.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

rust-lang/miri#1717 has been fixed so we can set thread names in Miri now.

## Solution

We set thread names in Miri.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

rust-lang/miri#1717 has been fixed so we can set thread names in Miri now.

## Solution

We set thread names in Miri.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build-System Related to build systems or continuous integration A-ECS Entities, components, systems, and events S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants