Skip to content

Conversation

@redsun82
Copy link
Contributor

@redsun82 redsun82 commented May 27, 2025

This will skip all unexpanded entities in library extraction, where we only really care about expanded things. This means skipping:

  • all token trees
  • the unexpanded AST of attribute macros

In the latter case, in order to replace the single Item with its expansion (which is a MacroItems entity), we wrap the MacroItems in a dummy MacroCall with null path.

This will skip all unexpanded entities in library extraction, where we only really care about expanded things. This means skipping: * the token tree of macro calls * the unexpanded AST of attribute macros In the latter case, in order to replace the single `Item` with its expansion (which is a `MacroItems` entity), we wrap the `MacroItems` in a dummy `MacroCall` with null path.
@github-actions github-actions bot added the Rust Pull requests that update Rust code label May 27, 2025
@redsun82 redsun82 marked this pull request as ready for review May 27, 2025 12:55
Copilot AI review requested due to automatic review settings May 27, 2025 12:55
@redsun82 redsun82 requested a review from a team as a code owner May 27, 2025 12:55
@redsun82
Copy link
Contributor Author

redsun82 commented May 27, 2025

DCA shows a moderate speedup, and improvement in call graph resolution (with one exception being bevy going very slightly worse). I think this is worth taking in.

@redsun82 redsun82 requested a review from aibaars May 27, 2025 12:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances library-mode emission by skipping unexpanded macro entities and only emitting fully expanded items.

  • Updated pre_emit! to use prepare_item_expansion for early returns of attribute macro expansions.
  • Added logic to skip nodes inside unexpanded macro call token trees during library extraction.
  • Introduced prepare_item_expansion and emit_attribute_macro_expansion to wrap attribute macro expansions in a dummy MacroCall and refactored emit_item_expansion.
Comments suppressed due to low confidence (1)

rust/extractor/src/translate/base.rs:692

  • New skip logic for nodes inside unexpanded macro call token trees lacks accompanying tests; consider adding unit tests to verify that these nodes are correctly ignored in library mode.
if syntax 
) -> Option<Label<generated::Item>> {
if self.source_kind == SourceKind::Library {
// if the item expands via an attribute macro, we want to only emit the expansion
if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
Copy link

Copilot AI May 27, 2025

Choose a reason for hiding this comment

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

emit_attribute_macro_expansion is invoked both in prepare_item_expansion and again in emit_item_expansion, which may lead to duplicate work or inconsistent emission. Consider caching the result or ensuring it's only called once.

Suggested change
if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
let node_id = node.syntax().text_range().start().into(); // Use a unique identifier for the node
if let Some(expanded) = self.macro_expansion_cache.get(&node_id).cloned()
.or_else(|| {
let result = self.emit_attribute_macro_expansion(node);
if let Some(ref expansion) = result {
self.macro_expansion_cache.insert(node_id, expansion.clone());
}
result
})
{
Copilot uses AI. Check for mistakes.
{
return true;
}
if syntax
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can simply drop any node of type TokenTree. That includes macro call arguments, as well as macro rule/definition bodies. Like done in #19581

@redsun82 redsun82 merged commit 2561f3c into main Jun 2, 2025
14 checks passed
@redsun82 redsun82 deleted the redsun82/rust-skip-unexpanded-in-libraries branch June 2, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Rust Pull requests that update Rust code

3 participants