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

Jump to bottom

Refine error message for thread-safe usage of std::sync::Arc<{Self}> #114842

Closed

reez12g wants to merge 2 commits into rust-lang:master from reez12g:issue-114687

Contributor

reez12g commented Aug 15, 2023

rustbot assigned thomcc

Collaborator

rustbot commented Aug 15, 2023

(rustbot has picked a reviewer for you, use r? to override)

rustbot added S-waiting-on-review T-libs labels

This comment has been minimized.

Sign in to view


          refine error message for thread-safe usage of std::sync::Arc<{Self}>

a46e8c4

reez12g force-pushed the issue-114687 branch from 9d317d0 to a46e8c4 Compare

August 15, 2023 09:01

This comment has been minimized.

Sign in to view

reez12g force-pushed the issue-114687 branch from 6ff0b90 to cde6313 Compare

August 15, 2023 10:13

This comment has been minimized.

Sign in to view

reez12g force-pushed the issue-114687 branch from cde6313 to 5377ef0 Compare

August 16, 2023 04:09

This comment has been minimized.

Sign in to view

reez12g force-pushed the issue-114687 branch from 5377ef0 to 09cc678 Compare

August 16, 2023 07:19

This comment has been minimized.

Sign in to view

reez12g force-pushed the issue-114687 branch from 09cc678 to e34d086 Compare

August 16, 2023 08:23

This comment has been minimized.

Sign in to view


          update tests to reflect refined error message for std::sync::Arc<{Self}>

8e15fc4

reez12g force-pushed the issue-114687 branch from e34d086 to 8e15fc4 Compare

August 16, 2023 10:22

Member

m-ou-se commented Aug 23, 2023 •

edited

Loading

This still points people at Arc as a solution for situations where Arc is useless. Arc doesn't make a !Send things Send, for example. I think the message should just be removed.

Contributor

estebank commented Aug 23, 2023

We can leave only the link to the book and not mention Arc at all in the !Send case.

Member

thomcc commented Aug 28, 2023

I don't have strong feelings here, so perhaps @m-ou-se should take this review, since she seems to care more than I.

rustbot assigned m-ou-se and unassigned thomcc

dtolnay requested changes

View reviewed changes

library/core/src/marker.rs

    
            @@ -79,7 +79,7 @@ macro marker_impls {
          
                  on(_Self = "std::rc::Rc<T, A>", note = "use `std::sync::Arc` instead of `std::rc::Rc`"),

                  message = "`{Self}` cannot be sent between threads safely",

                  label = "`{Self}` cannot be sent between threads safely",

                  note = "consider using `std::sync::Arc<{Self}>`; for more information visit \

                  note = "consider whether `std::sync::Arc<{Self}>` could be incorporated to share this value between threads; for more information visit \

Member

dtolnay Aug 28, 2023

As long as Arc<{Self}> continues to be mentioned, this does not fix #114687. That suggestion is always wrong.

dtolnay added S-waiting-on-author and removed S-waiting-on-review labels

dtolnay mentioned this pull request

Revert "Suggest using Arc on !Send/!Sync types" #115311

Merged

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request


          Rollup merge of rust-lang#115311 - dtolnay:usearcself, r=compiler-errors

9b0abe3

Revert "Suggest using `Arc` on `!Send`/`!Sync` types"

Closes rust-lang#114687. This is a clean revert of rust-lang#88936 + rust-lang#115210. The suggestion to Arc\<{Self}\> when Self does not implement Send is *always* wrong.

rust-lang#114842 is considering a way to make a more refined suggestion.

Contributor

bors commented Aug 28, 2023

☔ The latest upstream changes (presumably #115326) made this pull request unmergeable. Please resolve the merge conflicts.

Contributor Author

reez12g commented Aug 29, 2023

Thank you for your reviews.
Since issue #114687 is now closed, I am going to close this Pull Request as well.

reez12g closed this

dtolnay self-assigned this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author T-libs