-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Driver cleanups #133198
Driver cleanups #133198
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 |
---|---|---|
|
@@ -424,13 +424,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
} | ||
|
||
/// Recursively expand all macro invocations in this AST fragment. | ||
pub fn fully_expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment { | ||
pub fn fully_expand_fragment(&mut self, mut fragment: AstFragment) -> AstFragment { | ||
let orig_expansion_data = self.cx.current_expansion.clone(); | ||
let orig_force_mode = self.cx.force_mode; | ||
|
||
// Collect all macro invocations and replace them with placeholders. | ||
let (mut fragment_with_placeholders, mut invocations) = | ||
self.collect_invocations(input_fragment, &[]); | ||
let mut invocations = self.collect_invocations(&mut fragment, &[]); | ||
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. The naming ( 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. I guess we should also leave a comment explaining the difference between these fragment phases? |
||
|
||
// Optimization: if we resolve all imports now, | ||
// we'll be able to immediately resolve most of imported macros. | ||
|
@@ -492,7 +491,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
|
||
let fragment_kind = invoc.fragment_kind; | ||
match self.expand_invoc(invoc, &ext.kind) { | ||
ExpandResult::Ready(fragment) => { | ||
ExpandResult::Ready(mut fragment) => { | ||
let mut derive_invocations = Vec::new(); | ||
let derive_placeholders = self | ||
.cx | ||
|
@@ -523,8 +522,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
}) | ||
.unwrap_or_default(); | ||
|
||
let (expanded_fragment, collected_invocations) = | ||
self.collect_invocations(fragment, &derive_placeholders); | ||
let collected_invocations = | ||
self.collect_invocations(&mut fragment, &derive_placeholders); | ||
// We choose to expand any derive invocations associated with this macro | ||
// invocation *before* any macro invocations collected from the output | ||
// fragment. | ||
|
@@ -534,7 +533,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
if expanded_fragments.len() < depth { | ||
expanded_fragments.push(Vec::new()); | ||
} | ||
expanded_fragments[depth - 1].push((expn_id, expanded_fragment)); | ||
expanded_fragments[depth - 1].push((expn_id, fragment)); | ||
invocations.extend(derive_invocations.into_iter().rev()); | ||
} | ||
ExpandResult::Retry(invoc) => { | ||
|
@@ -562,8 +561,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
.add(NodeId::placeholder_from_expn_id(expn_id), expanded_fragment); | ||
} | ||
} | ||
fragment_with_placeholders.mut_visit_with(&mut placeholder_expander); | ||
fragment_with_placeholders | ||
fragment.mut_visit_with(&mut placeholder_expander); | ||
fragment | ||
} | ||
|
||
fn resolve_imports(&mut self) { | ||
|
@@ -578,9 +577,9 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
/// prepares data for resolving paths of macro invocations. | ||
fn collect_invocations( | ||
&mut self, | ||
mut fragment: AstFragment, | ||
fragment: &mut AstFragment, | ||
extra_placeholders: &[NodeId], | ||
) -> (AstFragment, Vec<(Invocation, Option<Lrc<SyntaxExtension>>)>) { | ||
) -> Vec<(Invocation, Option<Lrc<SyntaxExtension>>)> { | ||
// Resolve `$crate`s in the fragment for pretty-printing. | ||
self.cx.resolver.resolve_dollar_crates(); | ||
|
||
|
@@ -615,7 +614,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { | |
} | ||
} | ||
|
||
(fragment, invocations) | ||
invocations | ||
} | ||
|
||
fn error_recursion_limit_reached(&mut self) -> ErrorGuaranteed { | ||
|
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 change was attempted before and reverted - #107740.
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.
Huh, we should add a comment to point this out linking to #107740 and why.
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.
FWIW #132410 changes after_analysis to pass in TyCtxt directly, avoiding the double locking issue.