Skip to content

Conversation

@matklad
Copy link
Contributor

@matklad matklad commented May 4, 2021

No description provided.

assert_eq!(module_data.scope.resolutions().count(), 4);
});
let n_recalculated_item_trees = events.iter().filter(|it| it.contains("item_tree")).count();
assert_eq!(n_recalculated_item_trees, 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonas-schievink currently this returns 4 rather than 1. That is, we recalculate item_tree for file itself, as well as for each of the macro calls. Am I correct that we don't expect this happening?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah that shouldn't happen

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
@matklad
Copy link
Contributor Author

matklad commented May 4, 2021

Current guess: it's to_fragment_kind

@jonas-schievink
Copy link
Contributor

ItemTrees for macro files seem to have a lot of unexpected query dependencies:

[2021-05-05T13:31:26Z DEBUG salsa::derived::slot] read_upgrade(FileItemTreeQuery(HirFileId(MacroFile(MacroFile { macro_call_id: LazyMacro(LazyMacroId(0)) })))): inputs=Tracked {
        inputs: [
            parse_macro_expansion(MacroFile { macro_call_id: LazyMacro(LazyMacroId(0)) }),
            intern_macro(MacroCallLoc { def: MacroDefId { krate: CrateId(0), kind: Declarative(InFile { file_id: HirFileId(FileId(FileId(0))), value: FileAstId::<syntax::ast::node_ext::Macro>(0) }), local_inner: false }, krate: CrateId(0), kind: FnLike { ast_id: InFile { file_id: HirFileId(FileId(FileId(2))), value: FileAstId::<syntax::ast::generated::nodes::MacroCall>(0) } } }),
            parse(FileId(2)),
            ast_id_map(HirFileId(FileId(FileId(2)))),
            parse(FileId(0)),
            ast_id_map(HirFileId(FileId(FileId(0)))),
            macro_def(MacroDefId { krate: CrateId(0), kind: Declarative(InFile { file_id: HirFileId(FileId(FileId(0))), value: FileAstId::<syntax::ast::node_ext::Macro>(0) }), local_inner: false }),
            macro_arg_text(LazyMacro(LazyMacroId(0))),
            hygiene_frame(HirFileId(FileId(FileId(0)))),
            hygiene_frame(HirFileId(FileId(FileId(2)))),
            ast_id_map(HirFileId(MacroFile(MacroFile { macro_call_id: LazyMacro(LazyMacroId(0)) }))),
        ],
    }

I would expect just 2: parse_macro_expansion and ast_id_map

@jonas-schievink
Copy link
Contributor

Seems like these are all from Hygiene::new, which calls HygieneFrame::new

@jonas-schievink
Copy link
Contributor

Making Hygiene::new go through another query does not seem to fix the bug (but costs us 100 MB of RAM)

@jonas-schievink
Copy link
Contributor

Ah, that's probably just because HygieneInfo contains TextSizes, so its value always changes when some text is added or deleted in the file containing the macro invocation. Not really sure what the best way to fix this would be, maybe only storing AstIds in the hygiene data works? cc @edwin0cheng

@edwin0cheng
Copy link
Contributor

Ah, that's probably just because HygieneInfo contains TextSizes, so its value always changes when some text is added or deleted in the file containing the macro invocation. Not really sure what the best way to fix this would be, maybe only storing AstIds in the hygiene data works? cc @edwin0cheng

In theory yes, but HygieneInfo mainly used inside lower_path and it is db independent such that we can't simply convent that back to offset of the HirFile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants