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.

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

Be less confident when dyn suggestion is not checked for object safety #120530

Merged
merged 1 commit into from
Feb 14, 2024
Merged

Be less confident when dyn suggestion is not checked for object safety #120530

merged 1 commit into from
Feb 14, 2024

Conversation

trevyn
Copy link
Contributor

@trevyn trevyn commented Jan 31, 2024

#120275 no longer checks bare traits for object safety when making a dyn suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in #116434

r? @fmease

@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 Jan 31, 2024
@compiler-errors
Copy link
Member

I don't see a problem with this. If fmease has issues, follow-up later xD

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 31, 2024

📌 Commit 8d31a53 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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 Jan 31, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 31, 2024
@matthiaskrgr
Rollup merge of rust-lang#120530 - trevyn:issue-116434, r=compiler-er…
958d6f8 
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? `@fmease`
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 31, 2024
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 31, 2024
@matthiaskrgr
Rollup merge of rust-lang#120530 - trevyn:issue-116434, r=compiler-er…
b06e799 
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ``@fmease``
@matthiaskrgr matthiaskrgr mentioned this pull request Jan 31, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 1, 2024
@bors
Auto merge of rust-lang#120536 - matthiaskrgr:rollup-wllj7tb, r=matth…
11d66cb 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#120484 (Avoid ICE when is_val_statically_known is not of a supported type)
 - rust-lang#120516 (pattern_analysis: cleanup manual impls)
 - rust-lang#120517 (never patterns: It is correct to lower `!` to `_`.)
 - rust-lang#120523 (Improve `io::Read::read_buf_exact` error case)
 - rust-lang#120528 (Store SHOULD_CAPTURE as AtomicU8)
 - rust-lang#120529 (Update data layouts in custom target tests for LLVM 18)
 - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety)
 - rust-lang#120531 (Remove a bunch of `has_errors` checks that have no meaningful or the wrong effect)
 - rust-lang#120533 (Correct paths for hexagon-unknown-none-elf platform doc)

r? `@ghost`
`@rustbot` modify labels: rollup
@trevyn trevyn mentioned this pull request Feb 2, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2024
@bors
Auto merge of rust-lang#120536 - matthiaskrgr:rollup-wllj7tb, r=matth…
5bc7414 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#120484 (Avoid ICE when is_val_statically_known is not of a supported type)
 - rust-lang#120516 (pattern_analysis: cleanup manual impls)
 - rust-lang#120517 (never patterns: It is correct to lower `!` to `_`.)
 - rust-lang#120523 (Improve `io::Read::read_buf_exact` error case)
 - rust-lang#120528 (Store SHOULD_CAPTURE as AtomicU8)
 - rust-lang#120529 (Update data layouts in custom target tests for LLVM 18)
 - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety)
 - rust-lang#120531 (Remove a bunch of `has_errors` checks that have no meaningful or the wrong effect)
 - rust-lang#120533 (Correct paths for hexagon-unknown-none-elf platform doc)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2024
@bors
Auto merge of rust-lang#120536 - matthiaskrgr:rollup-wllj7tb, r=matth…
5976d95 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#120484 (Avoid ICE when is_val_statically_known is not of a supported type)
 - rust-lang#120516 (pattern_analysis: cleanup manual impls)
 - rust-lang#120517 (never patterns: It is correct to lower `!` to `_`.)
 - rust-lang#120523 (Improve `io::Read::read_buf_exact` error case)
 - rust-lang#120528 (Store SHOULD_CAPTURE as AtomicU8)
 - rust-lang#120529 (Update data layouts in custom target tests for LLVM 18)
 - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety)
 - rust-lang#120531 (Remove a bunch of `has_errors` checks that have no meaningful or the wrong effect)
 - rust-lang#120533 (Correct paths for hexagon-unknown-none-elf platform doc)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

matthiaskrgr commented Feb 3, 2024 •
edited
Loading

@bors r-
failed in https://github.com/rust-lang-ci/rust/actions/runs/7769229110/job/21188065200#step:26:10319 (cargo tests I assume)
#120536

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 3, 2024
@trevyn
Copy link
Contributor Author

trevyn commented Feb 4, 2024

@fmease @compiler-errors @estebank So it looks like changing the "use dyn" suggestion from MachineApplicable to MaybeIncorrect breaks the cargo fix tests because it's no longer automatically applied.

Should I change it back to MachineApplicable (even though it's really not), remove the cargo fix tests, or... something else?

@trevyn
Copy link
Contributor Author

trevyn commented Feb 6, 2024

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Feb 6, 2024

@trevyn: 🔑 Insufficient privileges: Not in reviewers

@compiler-errors
Copy link
Member

Can you squash this into one commit?

@trevyn
Copy link
Contributor Author

trevyn commented Feb 6, 2024

@compiler-errors done

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 6, 2024

📌 Commit 915812d has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2024
@bors
Copy link
Contributor

bors commented Feb 9, 2024

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

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 9, 2024
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 9, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 10, 2024

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git rebase -i master
$ # delete any merge commits in the editor that appears
$ git push --force-with-lease

The following commits are merge commits:

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Feb 10, 2024
@rustbot rustbot removed the has-merge-commits PR has merge commits, merge with caution. label Feb 10, 2024
@trevyn
Copy link
Contributor Author

trevyn commented Feb 10, 2024

r? @compiler-errors

@rustbot rustbot assigned compiler-errors and unassigned fmease Feb 10, 2024
@trevyn
Copy link
Contributor Author

trevyn commented Feb 10, 2024

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 10, 2024
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 12, 2024

📌 Commit 29fd82b has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors 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 Feb 12, 2024
oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 13, 2024
@oli-obk
Rollup merge of rust-lang#120530 - trevyn:issue-116434, r=compiler-er…
6e2c002 
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? `@fmease`
@oli-obk oli-obk mentioned this pull request Feb 13, 2024
Closed
oli-obk added a commit to oli-obk/rust that referenced this pull request Feb 13, 2024
@oli-obk
Rollup merge of rust-lang#120530 - trevyn:issue-116434, r=compiler-er…
bd61391 
…rors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ``@fmease``
@oli-obk oli-obk mentioned this pull request Feb 13, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2024
@bors
Auto merge of rust-lang#121013 - oli-obk:rollup-fbzq8qz, r=oli-obk
4272bb8 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#120486 (Use generic `NonZero` internally.)
 - rust-lang#120500 (Implement intrinsics with fallback bodies)
 - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety)
 - rust-lang#120563 (Make `NonZero::get` generic.)
 - rust-lang#120847 (Continue compilation after check_mod_type_wf errors)
 - rust-lang#120959 (Remove good path delayed bugs)

r? `@ghost`
`@rustbot` modify labels: rollup
@oli-obk oli-obk mentioned this pull request Feb 14, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2024
@bors
Auto merge of rust-lang#121078 - oli-obk:rollup-p11zsav, r=oli-obk
340bb19 
Rollup of 13 pull requests

Successful merges:

 - rust-lang#116387 (Additional doc links and explanation of `Wake`.)
 - rust-lang#118738 (Netbsd10 update)
 - rust-lang#118890 (Clarify the lifetimes of allocations returned by the `Allocator` trait)
 - rust-lang#120498 (Uplift `TypeVisitableExt` into `rustc_type_ir`)
 - rust-lang#120530 (Be less confident when `dyn` suggestion is not checked for object safety)
 - rust-lang#120915 (Fix suggestion span for `?Sized` when param type has default)
 - rust-lang#121015 (Optimize `delayed_bug` handling.)
 - rust-lang#121024 (implement `Default` for `AsciiChar`)
 - rust-lang#121039 (Correctly compute adjustment casts in GVN)
 - rust-lang#121045 (Fix two UI tests with incorrect directive / invalid revision)
 - rust-lang#121049 (Do not point at `#[allow(_)]` as the reason for compat lint triggering)
 - rust-lang#121071 (Use fewer delayed bugs.)
 - rust-lang#121073 (Fix typos in `OneLock` doc)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c4371a7 into rust-lang:master Feb 14, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2024
@rust-timer
Unrolled build for rust-lang#120530
733ed82 
Rollup merge of rust-lang#120530 - trevyn:issue-116434, r=compiler-errors

Be less confident when `dyn` suggestion is not checked for object safety

rust-lang#120275 no longer checks bare traits for object safety when making a `dyn` suggestion on Rust < 2021. In this case, qualify the suggestion with a note that the trait must be object safe, to prevent user confusion as seen in rust-lang#116434

r? ```@fmease```
@trevyn trevyn deleted the issue-116434 branch February 15, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@compiler-errors compiler-errors

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
Milestone
1.78.0
Development

Successfully merging this pull request may close these issues.
6 participants
@trevyn @compiler-errors @bors @matthiaskrgr @rustbot @fmease