-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: Name resolution failures inside include!
d files
#18325
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,7 +104,7 @@ use hir_expand::{ | |
}; | ||
use rustc_hash::FxHashMap; | ||
use smallvec::SmallVec; | ||
use span::{FileId, MacroFileId}; | ||
use span::{EditionedFileId, FileId, MacroFileId}; | ||
use stdx::impl_from; | ||
use syntax::{ | ||
ast::{self, HasName}, | ||
|
@@ -118,6 +118,7 @@ pub(super) struct SourceToDefCache { | |
pub(super) dynmap_cache: FxHashMap<(ChildContainer, HirFileId), DynMap>, | ||
expansion_info_cache: FxHashMap<MacroFileId, ExpansionInfo>, | ||
pub(super) file_to_def_cache: FxHashMap<FileId, SmallVec<[ModuleId; 1]>>, | ||
included_file_cache: FxHashMap<EditionedFileId, MacroFileId>, | ||
} | ||
|
||
impl SourceToDefCache { | ||
|
@@ -163,7 +164,11 @@ impl SourceToDefCtx<'_, '_> { | |
.include_macro_invoc(crate_id) | ||
.iter() | ||
.filter(|&&(_, file_id)| file_id == file) | ||
.flat_map(|(call, _)| { | ||
.flat_map(|(call, file_id)| { | ||
self.cache | ||
.included_file_cache | ||
.insert(*file_id, MacroFileId { macro_call_id: *call }); | ||
|
||
modules( | ||
call.lookup(self.db.upcast()) | ||
.kind | ||
|
@@ -499,7 +504,12 @@ impl SourceToDefCtx<'_, '_> { | |
let parent = |this: &mut Self, node: InFile<&SyntaxNode>| match node.value.parent() { | ||
Some(parent) => Some(node.with_value(parent)), | ||
None => { | ||
let macro_file = node.file_id.macro_file()?; | ||
let macro_file = if let Some(macro_file) = node.file_id.macro_file() { | ||
macro_file | ||
} else { | ||
let file_id = node.file_id.file_id()?; | ||
*this.cache.included_file_cache.get(&file_id)? | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the current node is inside the |
||
|
||
let expansion_info = this | ||
.cache | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This id mismatch was blocking us from inserting item for usage search
rust-analyzer/crates/hir-def/src/child_by_source.rs
Line 260 in 8dd53a3
LHS: MacroFileId != RHS: EditionFileId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, does it mean we have to make the
include!()
d file a macro file instead of anEditionedFileId
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about both directions; making included file into
MacroFile
as you said and current way and I think that as the ide things triggered from the opened included file are anchored with nodes fromEditionedFile
, this way would be more intuitive. But I'm not 100% sure about this so I'll rethink about thisThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I'm afraid that making it a macro file would require us to map between
InFile<T> { file_id: Editioned, .. }
andInFile<T> { file_id: Macro, .. }
back and forth quite often and missing it in one place might cause another error. This PR seems simpler