From 1dd56aa3047851891e6a1923e8fec3569d0fc89f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 19 Aug 2019 23:10:07 +0300 Subject: [PATCH 1/5] Add a test for an opaque macro eagerly expanding its arguments --- src/test/ui/hygiene/eager-from-opaque.rs | 18 ++++++++++++++++++ src/test/ui/hygiene/eager-from-opaque.stderr | 8 ++++++++ 2 files changed, 26 insertions(+) create mode 100644 src/test/ui/hygiene/eager-from-opaque.rs create mode 100644 src/test/ui/hygiene/eager-from-opaque.stderr diff --git a/src/test/ui/hygiene/eager-from-opaque.rs b/src/test/ui/hygiene/eager-from-opaque.rs new file mode 100644 index 0000000000000..57925d626b9d1 --- /dev/null +++ b/src/test/ui/hygiene/eager-from-opaque.rs @@ -0,0 +1,18 @@ +// Opaque macro can eagerly expand its input without breaking its resolution. +// Regression test for issue #63685. + +macro_rules! foo { + () => { + "foo" + }; +} + +macro_rules! bar { + () => { + foo!() //~ ERROR cannot find macro `foo!` in this scope + }; +} + +fn main() { + format_args!(bar!()); +} diff --git a/src/test/ui/hygiene/eager-from-opaque.stderr b/src/test/ui/hygiene/eager-from-opaque.stderr new file mode 100644 index 0000000000000..8db96e6ac95e5 --- /dev/null +++ b/src/test/ui/hygiene/eager-from-opaque.stderr @@ -0,0 +1,8 @@ +error: cannot find macro `foo!` in this scope + --> $DIR/eager-from-opaque.rs:12:9 + | +LL | foo!() + | ^^^ + +error: aborting due to previous error + From 96032aa5efd82c3cddc485332162614b9b8dd3dd Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 19 Aug 2019 23:35:25 +0300 Subject: [PATCH 2/5] expand: Keep the correct current expansion ID for eager expansions Solve the problem of `ParentScope` entries for eager expansions not exising in the resolver map by creating them on demand. --- src/librustc_resolve/macros.rs | 4 +++- src/libsyntax/ext/expand.rs | 1 - src/test/ui/hygiene/eager-from-opaque.stderr | 3 +++ src/test/ui/macros/derive-in-eager-expansion-hang.stderr | 3 +++ 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 01ad67252a387..8a7a30813c1e0 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -142,7 +142,9 @@ impl<'a> base::Resolver for Resolver<'a> { fn resolve_macro_invocation(&mut self, invoc: &Invocation, invoc_id: ExpnId, force: bool) -> Result>, Indeterminate> { - let parent_scope = self.invocation_parent_scopes[&invoc_id]; + let inherited_parent_scope = self.invocation_parent_scopes[&invoc_id]; + let parent_scope = *self.invocation_parent_scopes.entry(invoc.expansion_data.id) + .or_insert(inherited_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), diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index c1d52c9745529..5ac234b78d0fe 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -318,7 +318,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 { diff --git a/src/test/ui/hygiene/eager-from-opaque.stderr b/src/test/ui/hygiene/eager-from-opaque.stderr index 8db96e6ac95e5..f696e6caff7ca 100644 --- a/src/test/ui/hygiene/eager-from-opaque.stderr +++ b/src/test/ui/hygiene/eager-from-opaque.stderr @@ -3,6 +3,9 @@ error: cannot find macro `foo!` in this scope | LL | foo!() | ^^^ +... +LL | format_args!(bar!()); + | ------ in this macro invocation error: aborting due to previous error diff --git a/src/test/ui/macros/derive-in-eager-expansion-hang.stderr b/src/test/ui/macros/derive-in-eager-expansion-hang.stderr index 1ef9427666bc5..5ca4088e585db 100644 --- a/src/test/ui/macros/derive-in-eager-expansion-hang.stderr +++ b/src/test/ui/macros/derive-in-eager-expansion-hang.stderr @@ -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!()); From a83c35692fa5fc65ec9860599501f1a5a5e98214 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 19 Aug 2019 23:44:57 +0300 Subject: [PATCH 3/5] expand: Do not do questionable span adjustment before eagerly expanding an expression Maybe it made sense when it was introduced, but now it's doing something incorrect. --- src/libsyntax/ext/base.rs | 5 +---- src/test/ui/hygiene/eager-from-opaque.rs | 4 +++- src/test/ui/hygiene/eager-from-opaque.stderr | 11 ----------- 3 files changed, 4 insertions(+), 16 deletions(-) delete mode 100644 src/test/ui/hygiene/eager-from-opaque.stderr diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index b0a4a6af9839c..376df4062b116 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -908,12 +908,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, + expr: P, err_msg: &str, ) -> Result<(Symbol, ast::StrStyle, Span), Option>> { - // 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(); diff --git a/src/test/ui/hygiene/eager-from-opaque.rs b/src/test/ui/hygiene/eager-from-opaque.rs index 57925d626b9d1..6f3215dd697f3 100644 --- a/src/test/ui/hygiene/eager-from-opaque.rs +++ b/src/test/ui/hygiene/eager-from-opaque.rs @@ -1,6 +1,8 @@ // Opaque macro can eagerly expand its input without breaking its resolution. // Regression test for issue #63685. +// check-pass + macro_rules! foo { () => { "foo" @@ -9,7 +11,7 @@ macro_rules! foo { macro_rules! bar { () => { - foo!() //~ ERROR cannot find macro `foo!` in this scope + foo!() }; } diff --git a/src/test/ui/hygiene/eager-from-opaque.stderr b/src/test/ui/hygiene/eager-from-opaque.stderr deleted file mode 100644 index f696e6caff7ca..0000000000000 --- a/src/test/ui/hygiene/eager-from-opaque.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: cannot find macro `foo!` in this scope - --> $DIR/eager-from-opaque.rs:12:9 - | -LL | foo!() - | ^^^ -... -LL | format_args!(bar!()); - | ------ in this macro invocation - -error: aborting due to previous error - From 93d369bc2b65e822d001a8d08f99c6bbaf105ee5 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 20 Aug 2019 00:24:28 +0300 Subject: [PATCH 4/5] resolve/expand: Rename some things for clarity and add comments --- src/librustc_resolve/macros.rs | 36 +++++++++++++++++++++------------- src/libsyntax/ext/base.rs | 5 +++-- src/libsyntax/ext/expand.rs | 6 ++++-- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 8a7a30813c1e0..719167eb057b2 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -140,11 +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>, Indeterminate> { - let inherited_parent_scope = self.invocation_parent_scopes[&invoc_id]; - let parent_scope = *self.invocation_parent_scopes.entry(invoc.expansion_data.id) - .or_insert(inherited_parent_scope); + fn resolve_macro_invocation( + &mut self, invoc: &Invocation, eager_expansion_root: ExpnId, force: bool + ) -> Result>, 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), @@ -163,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), @@ -180,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)) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 376df4062b116..075e6a8001336 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -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>, Indeterminate>; + fn resolve_macro_invocation( + &mut self, invoc: &Invocation, eager_expansion_root: ExpnId, force: bool + ) -> Result>, Indeterminate>; fn check_unused_macros(&self); diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 5ac234b78d0fe..72f2c1375e7a2 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -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); From fe2dc919726d17dbe3568f1cb9de34c73b7f1dff Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 21 Aug 2019 12:53:11 +0300 Subject: [PATCH 5/5] Add a regression test for issue #63460 --- src/test/ui/hygiene/eager-from-opaque-2.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 src/test/ui/hygiene/eager-from-opaque-2.rs diff --git a/src/test/ui/hygiene/eager-from-opaque-2.rs b/src/test/ui/hygiene/eager-from-opaque-2.rs new file mode 100644 index 0000000000000..220e5526745c3 --- /dev/null +++ b/src/test/ui/hygiene/eager-from-opaque-2.rs @@ -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)) +}