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

Rewrite orphan_check_trait_ref to use a TypeVisitor #99552

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Jul 21, 2022

The current impl is far more confusing than it has any right to be ✨

r? rust-lang/types

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 21, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 21, 2022
Comment on lines +714 to +716
// A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so
// the first type we visit is always the self type.
self.in_self_ty = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

would be a bit cleaner to only visit the types that we care about... Tho since we don't implement TypeVisitor for iterators this can be annoying, too.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 21, 2022

The algorithm replicates the existing behaviour, so...

@bors r+

@bors
Copy link
Contributor

bors commented Jul 21, 2022

📌 Commit 84c3fcd has been approved by oli-obk

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 Jul 21, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 21, 2022
Rewrite `orphan_check_trait_ref` to use a `TypeVisitor`

The current impl is far more confusing than it has any right to be ✨

r? rust-lang/types
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 21, 2022
Rewrite `orphan_check_trait_ref` to use a `TypeVisitor`

The current impl is far more confusing than it has any right to be ✨

r? rust-lang/types
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 21, 2022
Rewrite `orphan_check_trait_ref` to use a `TypeVisitor`

The current impl is far more confusing than it has any right to be ✨

r? rust-lang/types
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2022
…askrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#98707 (std: use futex-based locks on Fuchsia)
 - rust-lang#99413 (Add `PhantomData` marker for dropck to `BTreeMap`)
 - rust-lang#99454 (Add map_continue and continue_value combinators to ControlFlow)
 - rust-lang#99523 (Fix the stable version of `AsFd for Arc<T>` and `Box<T>`)
 - rust-lang#99526 (Normalize the arg spans to be within the call span)
 - rust-lang#99528 (couple of clippy::perf fixes)
 - rust-lang#99549 (Add regression test for rust-lang#52304)
 - rust-lang#99552 (Rewrite `orphan_check_trait_ref` to use a `TypeVisitor`)
 - rust-lang#99557 (Edit `rustc_index::vec::IndexVec::pick3_mut` docs)
 - rust-lang#99558 (Fix `remap_constness`)
 - rust-lang#99559 (Remove unused field in ItemKind::KeywordItem)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 31284d2 into rust-lang:master Jul 21, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 21, 2022
@lcnr lcnr deleted the orphan_check-rework branch July 25, 2022 10:51
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