Skip to content

Conversation

@jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented May 8, 2021

This computes a macro's fragment kind eagerly (when the calling file is still available in parsed form) and stores it in the MacroCallLoc. This means that during expansion we no longer have to reparse the file containing the macro call, avoiding the unnecessary salsa dependencies (#8746 (comment)).

Marking as draft until I manage to find a test for this problem, since for some reason typing_inside_a_function_should_not_invalidate_expansions does not catch this (which might indicate that I misunderstand the problem).

I've manually confirmed that this fixes the issue described in #8746 (comment):

    7ms - parse_query @ FileId(179)
   12ms - SourceBinder::to_module_def
       12ms - crate_def_map:wait
            5ms - item_tree_query (1 calls)
            7ms - ???

@matklad
Copy link
Contributor

matklad commented May 9, 2021

Uhu, this is what I have in mind as well, and I’d also expect the test to fire.

But now, I think I see why test passes: to determine fragment kind, we need to reparse parent file. But we have to reparse it anyway, as we typed into it!

@jonas-schievink
Copy link
Contributor Author

Hmm, why does that stop us from recomputing the ItemTrees? Their input still changed after all

@matklad
Copy link
Contributor

matklad commented May 9, 2021

Ok, I think I figured that out. We re-run parse_macro_expansion query, and this query returns a green node of the expansion. The green node is the same, so item tree is not recaltucated.

Why is it recalculated for completion though? Because parse_macro_expansion is an LRU query! We seem to have more that 128 trees, and recalculate all of them, so LRU gets busted.

@matklad
Copy link
Contributor

matklad commented May 9, 2021

I guess we can change the test to look at both item trees and macro-expansion queries.

@matklad
Copy link
Contributor

matklad commented May 9, 2021

@jonas-schievink
Copy link
Contributor Author

Yup, that seems to catch it, nice!

@jonas-schievink jonas-schievink marked this pull request as ready for review May 9, 2021 14:40
@jonas-schievink
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented May 9, 2021

@bors bors bot merged commit 0900bee into rust-lang:master May 9, 2021
@jonas-schievink jonas-schievink deleted the precompute-fragment-kind branch May 12, 2021 16:33
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.

2 participants