Skip to content

Remove redundant is_dyn_thread_safe checks#153776

Merged
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
zetanumbers:curry-howard-dyn-thread-safe
Mar 20, 2026
Merged

Remove redundant is_dyn_thread_safe checks#153776
rust-bors[bot] merged 2 commits intorust-lang:mainfrom
zetanumbers:curry-howard-dyn-thread-safe

Conversation

@zetanumbers
Copy link
Contributor

@zetanumbers zetanumbers commented Mar 12, 2026

Refactor uses of FromDyn to reduce number of redundant is_dyn_thread_safe checks by replacing FromDyn::from with check_dyn_thread_safe in tandem with existing FromDyn::derive so that the users would avoid redundancy in the future.

PR is split up into multiple commits for an easier review.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 12, 2026
@rustbot
Copy link
Collaborator

rustbot commented Mar 12, 2026

r? @JonathanBrouwer

rustbot has assigned @JonathanBrouwer.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 69 candidates
  • Random selection from 16 candidates

@zetanumbers zetanumbers changed the title Reduce is_dyn_thread_safe checks Remove redundant is_dyn_thread_safe checks Mar 12, 2026
@Zoxc
Copy link
Contributor

Zoxc commented Mar 12, 2026

The change to reduce the number of checks is good here.

I'm not sure about adding a separate type for it. The new type DynThreadSafe is equivalent to FromDyn<()> and the same change could be done by adding a FromDyn::try_from method. The naming of DynThreadSafe seem quite trait-like to me too. It seems like it could be more something like DynThreadSafetyEnabled.

@zetanumbers
Copy link
Contributor Author

zetanumbers commented Mar 12, 2026

The naming of DynThreadSafe seem quite trait-like to me too.

Yes. That's because traits are predicates over types. Analogously DynThreadSafe is also a predicate but over a static variable. These are the same in dependent type systems.

@zetanumbers
Copy link
Contributor Author

I'm not sure about adding a separate type for it. The new type DynThreadSafe is equivalent to FromDyn<()> and the same change could be done by adding a FromDyn::try_from method.

FromDyn is simply a newtype over a pair (T, DynThreadSafe) to implement Send and Sync. If those weren't rust traits which require a unique implementation then having a newtype wouldn't be necessary.

@Zoxc
Copy link
Contributor

Zoxc commented Mar 14, 2026

FromDyn is simply a newtype over a pair (T, DynThreadSafe) to implement Send and Sync. If those weren't rust traits which require a unique implementation then having a newtype wouldn't be necessary.

Not quite sure what your point is here. The addition of DynThreadSafe isn't necessary since it's the same type as FromDyn<()> (both before and after the PR). What I'm unsure about is if its addition is a good idea.

@zetanumbers zetanumbers force-pushed the curry-howard-dyn-thread-safe branch from bd83bd6 to 3973d9f Compare March 17, 2026 11:47
@rustbot
Copy link
Collaborator

rustbot commented Mar 17, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@zetanumbers
Copy link
Contributor Author

Well fine, no renames - no renames.

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Executing "/scripts/stage_2_test_set1.sh"
+ /scripts/stage_2_test_set1.sh
PR_CI_JOB set; skipping tidy
+ '[' 1 == 1 ']'
+ echo 'PR_CI_JOB set; skipping tidy'
+ SKIP_TIDY='--skip tidy'
+ ../x.py --stage 2 test --skip tidy --skip compiler --skip src
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
downloading https://static.rust-lang.org/dist/2026-03-05/rustfmt-nightly-aarch64-unknown-linux-gnu.tar.xz
---
   Compiling rustc_parse v0.0.0 (/checkout/compiler/rustc_parse)
[RUSTC-TIMING] rustc_parse test:false 15.791
   Compiling rustc_attr_parsing v0.0.0 (/checkout/compiler/rustc_attr_parsing)

Session terminated, killing shell...::group::Clock drift check
##[error]The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.
  local time: Tue Mar 17 11:53:28 UTC 2026
  network time: Tue, 17 Mar 2026 11:53:29 GMT
##[endgroup]
 ...killed.
##[error]The operation was canceled.

@zetanumbers
Copy link
Contributor Author

What

@JonathanBrouwer
Copy link
Contributor

GitHub moment... I've restarted PR ci

Copy link
Contributor

@JonathanBrouwer JonathanBrouwer left a comment

Choose a reason for hiding this comment

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

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 20, 2026

📌 Commit 3973d9f has been approved by JonathanBrouwer

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2026
@JonathanBrouwer
Copy link
Contributor

Always love some extra static checks, thanks for the work!
(I'm also a big fan of dependent types btw :3)

JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 20, 2026
…d-safe, r=JonathanBrouwer

Remove redundant `is_dyn_thread_safe` checks

Refactor uses of `FromDyn` to reduce number of redundant `is_dyn_thread_safe` checks by replacing `FromDyn::from` with `check_dyn_thread_safe` in tandem with existing `FromDyn::derive` so that the users would avoid redundancy in the future.

PR is split up into multiple commits for an easier review.
rust-bors bot pushed a commit that referenced this pull request Mar 20, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #154103 (coretests: Expand ieee754 parsing and printing tests to f16)
 - #152669 (rustc_public: add `vtable_entries()` to `TraitRef`)
 - #153776 (Remove redundant `is_dyn_thread_safe` checks)
 - #154121 (Fix typos and markdown errors)
 - #154126 (refactor(attribute parser): move check_custom_mir to attribute parser)
@rust-bors rust-bors bot merged commit 8a30071 into rust-lang:main Mar 20, 2026
13 of 22 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 20, 2026
@zetanumbers zetanumbers deleted the curry-howard-dyn-thread-safe branch March 21, 2026 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants