fix: Resolve `$crate` in derive paths by lowr · Pull Request #14610 · rust-lang/... - JOYK Joy of Geek, Geek News, Link all geek

Conversation

Contributor

commented

Apr 19, 2023

•

edited

Paths in derive meta item list may contain any kind of paths, including those that start with $crate generated by macros. We need to take hygiene into account when we lower paths in the list.

This issue was identified while investigating #14607, though this patch doesn't fix the broken trait resolution.

lnicola reacted with heart emoji

rustbot

added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label

Apr 19, 2023

crates/hir-def/src/nameres/collector.rs

Outdated

		@@ -1223,8 +1225,9 @@ impl DefCollector<'_> {
		}
		};
		let ast_id = ast_id.with_value(ast_adt_id);
		let hygiene = Hygiene::new(self.db.upcast(), file_id);

(Haven't checked) I feel like we should have a hygiene lying around here somewhere already no? (Building hygiene isn't the cheapest from what I remember)

Contributor

Author

All paths but those in attributes seem to be lowered during ItemTree construction, which is decoupled from DefCollector, so there's no Hygiene around here. Moreover, a single DefCollector operates on possibly multiple HirFileIds, so we can't just cache Hygiene in it.

One thing we could try is having something like HashMap<HirFileId, Hygiene> in DefCollector and building Hygiene on demand. How does that sound?

Ye I think adding that to reuse hygienes sounds good for now

Member

@bors r+

Collaborator

Commit 85e7654 has been approved by Veykril