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

Consider block impls in lookup_impl_assoc_item_for_trait_ref #14855

Merged
merged 1 commit into from May 20, 2023

Conversation

HKalbasi
Copy link
Member

fix #14782

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 19, 2023
@HKalbasi
Copy link
Member Author

This is different from what said in #14782 (comment) it extracts the block from the self type instead of passing it by caller. But I think in the const eval test added in this PR we have no way to find that in the caller of lookup_impl_method (other than looking the self ty which has no difference) so I think this is in the right path.

@bors r+

@bors
Copy link
Collaborator

bors commented May 20, 2023

📌 Commit 92d6670 has been approved by HKalbasi

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented May 20, 2023

⌛ Testing commit 92d6670 with merge a04d845...

@bors
Copy link
Collaborator

bors commented May 20, 2023

☀️ Test successful - checks-actions
Approved by: HKalbasi
Pushing a04d845 to master...

@bors bors merged commit a04d845 into rust-lang:master May 20, 2023
10 checks passed
@Veykril
Copy link
Member

Veykril commented May 20, 2023

This should ideally mimic the same logic for fetching impls that we use in trait solving, so this is in part on the right track. There we pick the trait impls of the block (and ancestors) that the self ty is defined in, the block (and ancestors) of the trait def, and then the block (and ancestors) of the "current location if applicable". The former two are simple to do, the latter obviously isn't but usually the former two cover almost all cases anyways.

let mut def_blocks =
[trait_module.containing_block(), type_module.and_then(|it| it.containing_block())];
// Note: Since we're using impls_for_trait, only impls where the trait
// can be resolved should ever reach Chalk. impl_datum relies on that
// and will panic if the trait can't be resolved.
let in_deps = self.db.trait_impls_in_deps(self.krate);
let in_self = self.db.trait_impls_in_crate(self.krate);
let block_impls = iter::successors(self.block, |&block_id| {
cov_mark::hit!(block_local_impls);
self.db.block_def_map(block_id).parent().and_then(|module| module.containing_block())
})
.inspect(|&block_id| {
// make sure we don't search the same block twice
def_blocks.iter_mut().for_each(|block| {
if *block == Some(block_id) {
*block = None;
}
});
})
.map(|block_id| self.db.trait_impls_in_block(block_id));

def_blocks is the former two, block_impls the latter one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go to definition doesn't work for trait implementations for structs inside function bodies
4 participants