Skip to content

Commit

Permalink
Rollup merge of rust-lang#63717 - petrochenkov:eager, r=matthewjasper
Browse files Browse the repository at this point in the history
Fix nested eager expansions in arguments of `format_args`

Fixes rust-lang#63460
Fixes rust-lang#63685 (regression from making `format_args` opaque - rust-lang#63114)

r? @matthewjasper
  • Loading branch information
Centril authored Aug 21, 2019
2 parents d034cca + fe2dc91 commit 7043696
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 21 deletions.
34 changes: 22 additions & 12 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,23 @@ impl<'a> base::Resolver for Resolver<'a> {
ImportResolver { r: self }.resolve_imports()
}

fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: ExpnId, force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Indeterminate> {
let parent_scope = self.invocation_parent_scopes[&invoc_id];
fn resolve_macro_invocation(
&mut self, invoc: &Invocation, eager_expansion_root: ExpnId, force: bool
) -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate> {
let invoc_id = invoc.expansion_data.id;
let parent_scope = match self.invocation_parent_scopes.get(&invoc_id) {
Some(parent_scope) => *parent_scope,
None => {
// If there's no entry in the table, then we are resolving an eagerly expanded
// macro, which should inherit its parent scope from its eager expansion root -
// the macro that requested this eager expansion.
let parent_scope = *self.invocation_parent_scopes.get(&eager_expansion_root)
.expect("non-eager expansion without a parent scope");
self.invocation_parent_scopes.insert(invoc_id, parent_scope);
parent_scope
}
};

let (path, kind, derives, after_derive) = match invoc.kind {
InvocationKind::Attr { ref attr, ref derives, after_derive, .. } =>
(&attr.path, MacroKind::Attr, self.arenas.alloc_ast_paths(derives), after_derive),
Expand All @@ -161,7 +175,7 @@ impl<'a> base::Resolver for Resolver<'a> {
match self.resolve_macro_path(path, Some(MacroKind::Derive),
&parent_scope, true, force) {
Ok((Some(ref ext), _)) if ext.is_derive_copy => {
self.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
self.add_derives(invoc_id, SpecialDerives::COPY);
return Ok(None);
}
Err(Determinacy::Undetermined) => result = Err(Indeterminate),
Expand All @@ -178,19 +192,15 @@ impl<'a> base::Resolver for Resolver<'a> {
let (ext, res) = self.smart_resolve_macro_path(path, kind, parent_scope, force)?;

let span = invoc.span();
invoc.expansion_data.id.set_expn_data(
ext.expn_data(parent_scope.expansion, span, fast_print_path(path))
);
invoc_id.set_expn_data(ext.expn_data(parent_scope.expansion, span, fast_print_path(path)));

if let Res::Def(_, def_id) = res {
if after_derive {
self.session.span_err(span, "macro attributes must be placed before `#[derive]`");
}
self.macro_defs.insert(invoc.expansion_data.id, def_id);
let normal_module_def_id =
self.macro_def_scope(invoc.expansion_data.id).normal_ancestor_id;
self.definitions.add_parent_module_of_macro_def(invoc.expansion_data.id,
normal_module_def_id);
self.macro_defs.insert(invoc_id, def_id);
let normal_module_def_id = self.macro_def_scope(invoc_id).normal_ancestor_id;
self.definitions.add_parent_module_of_macro_def(invoc_id, normal_module_def_id);
}

Ok(Some(ext))
Expand Down
10 changes: 4 additions & 6 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,8 +682,9 @@ pub trait Resolver {

fn resolve_imports(&mut self);

fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: ExpnId, force: bool)
-> Result<Option<Lrc<SyntaxExtension>>, Indeterminate>;
fn resolve_macro_invocation(
&mut self, invoc: &Invocation, eager_expansion_root: ExpnId, force: bool
) -> Result<Option<Lrc<SyntaxExtension>>, Indeterminate>;

fn check_unused_macros(&self);

Expand Down Expand Up @@ -908,12 +909,9 @@ impl<'a> ExtCtxt<'a> {
/// compilation on error, merely emits a non-fatal error and returns `None`.
pub fn expr_to_spanned_string<'a>(
cx: &'a mut ExtCtxt<'_>,
mut expr: P<ast::Expr>,
expr: P<ast::Expr>,
err_msg: &str,
) -> Result<(Symbol, ast::StrStyle, Span), Option<DiagnosticBuilder<'a>>> {
// Update `expr.span`'s ctxt now in case expr is an `include!` macro invocation.
expr.span = expr.span.apply_mark(cx.current_expansion.id);

// Perform eager expansion on the expression.
// We want to be able to handle e.g., `concat!("foo", "bar")`.
let expr = cx.expander().fully_expand_fragment(AstFragment::Expr(expr)).make_expr();
Expand Down
7 changes: 4 additions & 3 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,9 +305,11 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
continue
};

let scope =
let eager_expansion_root =
if self.monotonic { invoc.expansion_data.id } else { orig_expansion_data.id };
let ext = match self.cx.resolver.resolve_macro_invocation(&invoc, scope, force) {
let ext = match self.cx.resolver.resolve_macro_invocation(
&invoc, eager_expansion_root, force
) {
Ok(ext) => ext,
Err(Indeterminate) => {
undetermined_invocations.push(invoc);
Expand All @@ -318,7 +320,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
progress = true;
let ExpansionData { depth, id: expn_id, .. } = invoc.expansion_data;
self.cx.current_expansion = invoc.expansion_data.clone();
self.cx.current_expansion.id = scope;

// FIXME(jseyfried): Refactor out the following logic
let (expanded_fragment, new_invocations) = if let Some(ext) = ext {
Expand Down
22 changes: 22 additions & 0 deletions src/test/ui/hygiene/eager-from-opaque-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Regression test for the issue #63460.

// check-pass

#[macro_export]
macro_rules! separator {
() => { "/" };
}

#[macro_export]
macro_rules! concat_separator {
( $e:literal, $($other:literal),+ ) => {
concat!($e, $crate::separator!(), $crate::concat_separator!($($other),+))
};
( $e:literal ) => {
$e
}
}

fn main() {
println!("{}", concat_separator!(2, 3, 4))
}
20 changes: 20 additions & 0 deletions src/test/ui/hygiene/eager-from-opaque.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Opaque macro can eagerly expand its input without breaking its resolution.
// Regression test for issue #63685.

// check-pass

macro_rules! foo {
() => {
"foo"
};
}

macro_rules! bar {
() => {
foo!()
};
}

fn main() {
format_args!(bar!());
}
3 changes: 3 additions & 0 deletions src/test/ui/macros/derive-in-eager-expansion-hang.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ LL | |
LL | | ""
LL | | }
| |_____^
...
LL | format_args!(hang!());
| ------- in this macro invocation
help: you might be missing a string literal to format with
|
LL | format_args!("{}", hang!());
Expand Down

0 comments on commit 7043696

Please sign in to comment.