Skip to content
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.

Sign up for GitHub

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

Jump to bottom

expand: Preserve order of inert attributes during expansion #82419

Merged
merged 2 commits into from
Feb 27, 2021
Merged

expand: Preserve order of inert attributes during expansion #82419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c02d210
Add tests
petrochenkov Feb 22, 2021
fc9d578
expand: Preserve order of inert attributes during expansion
petrochenkov Feb 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions compiler/rustc_expand/src/expand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,8 @@ pub enum InvocationKind {
},
Attr {
attr: ast::Attribute,
// Re-insertion position for inert attributes.
pos: usize,
item: Annotatable,
// Required for resolving derive helper attributes.
derives: Vec<Path>,
Expand Down Expand Up @@ -690,7 +692,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
_ => unreachable!(),
},
InvocationKind::Attr { attr, mut item, derives } => match ext {
InvocationKind::Attr { attr, pos, mut item, derives } => match ext {
SyntaxExtensionKind::Attr(expander) => {
self.gate_proc_macro_input(&item);
self.gate_proc_macro_attr_item(span, &item);
Expand Down Expand Up @@ -721,7 +723,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
ExpandResult::Retry(item) => {
// Reassemble the original invocation for retrying.
return ExpandResult::Retry(Invocation {
kind: InvocationKind::Attr { attr, item, derives },
kind: InvocationKind::Attr { attr, pos, item, derives },
..invoc
});
}
Expand All @@ -739,7 +741,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
if *mark_used {
self.cx.sess.mark_attr_used(&attr);
}
item.visit_attrs(|attrs| attrs.push(attr));
item.visit_attrs(|attrs| attrs.insert(pos, attr));
fragment_kind.expect_from_annotatables(iter::once(item))
}
_ => unreachable!(),
Expand Down Expand Up @@ -1000,17 +1002,20 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {

fn collect_attr(
&mut self,
(attr, derives): (ast::Attribute, Vec<Path>),
(attr, pos, derives): (ast::Attribute, usize, Vec<Path>),
item: Annotatable,
kind: AstFragmentKind,
) -> AstFragment {
self.collect(kind, InvocationKind::Attr { attr, item, derives })
self.collect(kind, InvocationKind::Attr { attr, pos, item, derives })
}

/// If `item` is an attribute invocation, remove the attribute and return it together with
/// derives following it. We have to collect the derives in order to resolve legacy derive
/// helpers (helpers written before derives that introduce them).
fn take_first_attr(&mut self, item: &mut impl HasAttrs) -> Option<(ast::Attribute, Vec<Path>)> {
/// its position and derives following it. We have to collect the derives in order to resolve
/// legacy derive helpers (helpers written before derives that introduce them).
fn take_first_attr(
&mut self,
item: &mut impl HasAttrs,
) -> Option<(ast::Attribute, usize, Vec<Path>)> {
let mut attr = None;

item.visit_attrs(|attrs| {
Expand All @@ -1033,7 +1038,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
})
.collect();

(attr, following_derives)
(attr, attr_pos, following_derives)
})
});

Expand Down
22 changes: 16 additions & 6 deletions src/test/ui/proc-macro/auxiliary/test-macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#![crate_type = "proc-macro"]

extern crate proc_macro;
use proc_macro::TokenStream;
use proc_macro::{TokenStream, TokenTree};

// Macro that return empty token stream.

Expand Down Expand Up @@ -80,6 +80,10 @@ pub fn recollect_derive(input: TokenStream) -> TokenStream {
// Macros that print their input in the original and re-collected forms (if they differ).

fn print_helper(input: TokenStream, kind: &str) -> TokenStream {
print_helper_ext(input, kind, true)
}

fn print_helper_ext(input: TokenStream, kind: &str, debug: bool) -> TokenStream {
let input_display = format!("{}", input);
let input_debug = format!("{:#?}", input);
let recollected = input.into_iter().collect();
Expand All @@ -89,9 +93,11 @@ fn print_helper(input: TokenStream, kind: &str) -> TokenStream {
if recollected_display != input_display {
println!("PRINT-{} RE-COLLECTED (DISPLAY): {}", kind, recollected_display);
}
println!("PRINT-{} INPUT (DEBUG): {}", kind, input_debug);
if recollected_debug != input_debug {
println!("PRINT-{} RE-COLLECTED (DEBUG): {}", kind, recollected_debug);
if debug {
println!("PRINT-{} INPUT (DEBUG): {}", kind, input_debug);
if recollected_debug != input_debug {
println!("PRINT-{} RE-COLLECTED (DEBUG): {}", kind, recollected_debug);
}
}
recollected
}
Expand All @@ -108,8 +114,12 @@ pub fn print_bang_consume(input: TokenStream) -> TokenStream {
}

#[proc_macro_attribute]
pub fn print_attr(_: TokenStream, input: TokenStream) -> TokenStream {
print_helper(input, "ATTR")
pub fn print_attr(args: TokenStream, input: TokenStream) -> TokenStream {
let debug = match &args.into_iter().collect::<Vec<_>>()[..] {
[TokenTree::Ident(ident)] if ident.to_string() == "nodebug" => false,
_ => true,
};
print_helper_ext(input, "ATTR", debug)
}

#[proc_macro_attribute]
Expand Down
12 changes: 12 additions & 0 deletions src/test/ui/proc-macro/derive-helper-legacy-spurious.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// aux-build:test-macros.rs

#![dummy] //~ ERROR cannot find attribute `dummy` in this scope

#[macro_use]
extern crate test_macros;

#[derive(Empty)] //~ ERROR cannot determine resolution for the attribute macro `derive`
#[empty_helper] //~ ERROR cannot find attribute `empty_helper` in this scope
struct Foo {}

fn main() {}
22 changes: 22 additions & 0 deletions src/test/ui/proc-macro/derive-helper-legacy-spurious.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: cannot find attribute `dummy` in this scope
--> $DIR/derive-helper-legacy-spurious.rs:3:4
|
LL | #![dummy]
| ^^^^^

error: cannot determine resolution for the attribute macro `derive`
--> $DIR/derive-helper-legacy-spurious.rs:8:3
|
LL | #[derive(Empty)]
| ^^^^^^
|
= note: import resolution is stuck, try simplifying macro imports

error: cannot find attribute `empty_helper` in this scope
--> $DIR/derive-helper-legacy-spurious.rs:9:3
|
LL | #[empty_helper]
| ^^^^^^^^^^^^

error: aborting due to 3 previous errors

23 changes: 23 additions & 0 deletions src/test/ui/proc-macro/inert-attribute-order.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Order of inert attributes, both built-in and custom is preserved during expansion.

// check-pass
// compile-flags: -Z span-debug
// aux-build:test-macros.rs

#![no_std] // Don't load unnecessary hygiene information from std
extern crate std;

#[macro_use]
extern crate test_macros;

/// 1
#[rustfmt::attr2]
#[doc = "3"]
#[print_attr(nodebug)]
#[doc = "4"]
#[rustfmt::attr5]
/// 6
#[print_attr(nodebug)]
struct S;

fn main() {}
7 changes: 7 additions & 0 deletions src/test/ui/proc-macro/inert-attribute-order.stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
PRINT-ATTR INPUT (DISPLAY): /// 1
#[rustfmt :: attr2] #[doc = "3"] #[doc = "4"] #[rustfmt :: attr5] /// 6
#[print_attr(nodebug)] struct S ;
PRINT-ATTR RE-COLLECTED (DISPLAY): #[doc = " 1"] #[rustfmt :: attr2] #[doc = "3"] #[doc = "4"]
#[rustfmt :: attr5] #[doc = " 6"] #[print_attr(nodebug)] struct S ;
PRINT-ATTR INPUT (DISPLAY): #[doc = " 1"] #[rustfmt :: attr2] #[doc = "3"] #[doc = "4"]
#[rustfmt :: attr5] #[doc = " 6"] struct S ;
64 changes: 32 additions & 32 deletions src/test/ui/proc-macro/issue-75930-derive-cfg.stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
PRINT-ATTR INPUT (DISPLAY): #[allow(dead_code)] #[derive(Print)] #[print_helper(b)] #[print_helper(a)]
PRINT-ATTR INPUT (DISPLAY): #[print_helper(a)] #[allow(dead_code)] #[derive(Print)] #[print_helper(b)]
Copy link
Member

@Aaron1011 Aaron1011 Feb 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is a subtle detail that I just noticed - since we perform cfg-stripping before invoking custom attributes (e.g. #[my_attr] #[cfg(FALSE)] struct Foo {} does nothing), we also expand #[cfg_attr]. So, an attribute macro sees expanded #[cfg_attr] attributes (e.g. the #[allow(dead_code)]) at the 'top level' of the attribute target, but not anywhere else. For example, you can see #[cfg_attr(not(FALSE), allow(warnings))] in the input below.

Copy link
Contributor Author

@petrochenkov petrochenkov Feb 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

since we perform cfg-stripping before invoking custom attributes

This is something I want to change, btw (and make the expansion order fully left-to-right).
It will certainly needs a crater run, but I think we'll be able to do this in practice, similarly to #79078.

struct Foo < #[cfg(FALSE)] A, B >
{
#[cfg(FALSE)] first : String, #[cfg_attr(FALSE, deny(warnings))] second :
Expand All @@ -23,6 +23,31 @@ struct Foo < #[cfg(FALSE)] A, B >
}], #[print_helper(d)] fourth : B
}
PRINT-ATTR INPUT (DEBUG): TokenStream [
Punct {
ch: '#',
spacing: Alone,
span: $DIR/issue-75930-derive-cfg.rs:16:1: 16:2 (#0),
},
Group {
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "print_helper",
span: $DIR/issue-75930-derive-cfg.rs:16:3: 16:15 (#0),
},
Group {
delimiter: Parenthesis,
stream: TokenStream [
Ident {
ident: "a",
span: $DIR/issue-75930-derive-cfg.rs:16:16: 16:17 (#0),
},
],
span: $DIR/issue-75930-derive-cfg.rs:16:15: 16:18 (#0),
},
],
span: $DIR/issue-75930-derive-cfg.rs:16:2: 16:19 (#0),
},
Punct {
ch: '#',
spacing: Alone,
Expand Down Expand Up @@ -98,31 +123,6 @@ PRINT-ATTR INPUT (DEBUG): TokenStream [
],
span: $DIR/issue-75930-derive-cfg.rs:21:2: 21:19 (#0),
},
Punct {
ch: '#',
spacing: Alone,
span: $DIR/issue-75930-derive-cfg.rs:16:1: 16:2 (#0),
},
Group {
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "print_helper",
span: $DIR/issue-75930-derive-cfg.rs:16:3: 16:15 (#0),
},
Group {
delimiter: Parenthesis,
stream: TokenStream [
Ident {
ident: "a",
span: $DIR/issue-75930-derive-cfg.rs:16:16: 16:17 (#0),
},
],
span: $DIR/issue-75930-derive-cfg.rs:16:15: 16:18 (#0),
},
],
span: $DIR/issue-75930-derive-cfg.rs:16:2: 16:19 (#0),
},
Ident {
ident: "struct",
span: $DIR/issue-75930-derive-cfg.rs:22:1: 22:7 (#0),
Expand Down Expand Up @@ -1194,7 +1194,7 @@ PRINT-ATTR INPUT (DEBUG): TokenStream [
span: $DIR/issue-75930-derive-cfg.rs:22:32: 65:2 (#0),
},
]
PRINT-DERIVE INPUT (DISPLAY): #[allow(dead_code)] #[print_helper(b)] #[print_helper(a)] struct Foo < B >
PRINT-DERIVE INPUT (DISPLAY): #[print_helper(a)] #[allow(dead_code)] #[print_helper(b)] struct Foo < B >
{
second : bool, third :
[u8 ;
Expand All @@ -1217,14 +1217,14 @@ PRINT-DERIVE INPUT (DEBUG): TokenStream [
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "allow",
ident: "print_helper",
span: $DIR/issue-75930-derive-cfg.rs:22:1: 65:2 (#0),
},
Group {
delimiter: Parenthesis,
stream: TokenStream [
Ident {
ident: "dead_code",
ident: "a",
span: $DIR/issue-75930-derive-cfg.rs:22:1: 65:2 (#0),
},
],
Expand All @@ -1242,14 +1242,14 @@ PRINT-DERIVE INPUT (DEBUG): TokenStream [
delimiter: Bracket,
stream: TokenStream [
Ident {
ident: "print_helper",
ident: "allow",
span: $DIR/issue-75930-derive-cfg.rs:22:1: 65:2 (#0),
},
Group {
delimiter: Parenthesis,
stream: TokenStream [
Ident {
ident: "b",
ident: "dead_code",
span: $DIR/issue-75930-derive-cfg.rs:22:1: 65:2 (#0),
},
],
Expand All @@ -1274,7 +1274,7 @@ PRINT-DERIVE INPUT (DEBUG): TokenStream [
delimiter: Parenthesis,
stream: TokenStream [
Ident {
ident: "a",
ident: "b",
span: $DIR/issue-75930-derive-cfg.rs:22:1: 65:2 (#0),
},
],
Expand Down