Skip to content

Commit ab6b50a

Browse files
Rollup merge of rust-lang#138063 - compiler-errors:improve-attr-unpretty, r=jdonszelmann
Improve `-Zunpretty=hir` for parsed attrs 0. Rename `print_something` to `should_render` to make it distinct from `print_attribute` in that it doesn't print anything, it's just a way to probe if a type renders anything. 1. Fixes a few bugs in the `PrintAttribute` derive. Namely, the `__printed_anything` variable was entangled with the `should_render` call, leading us to always render field names but never render commas. 2. Remove the outermost `""` from the attr. 3. Debug print `Symbol`s. I know that this is redundant for some parsed attributes, but there's no good way to distinguish symbols that are ident-like and symbols which are cooked string literals. We could perhaps *conditionally* to fall back to a debug printing if the symbol doesn't match an ident? But seems like overkill. Based on rust-lang#138060, only review the commits not in that one.
2 parents 29e15ef + 9ae9453 commit ab6b50a

File tree

5 files changed

+57
-49
lines changed

5 files changed

+57
-49
lines changed

compiler/rustc_attr_data_structures/src/lib.rs

+30-22
Original file line numberDiff line numberDiff line change
@@ -35,33 +35,39 @@ pub trait HashStableContext: rustc_ast::HashStableContext + rustc_abi::HashStabl
3535
/// like [`Span`]s and empty tuples, are gracefully skipped so they don't clutter the
3636
/// representation much.
3737
pub trait PrintAttribute {
38-
fn print_something(&self) -> bool;
38+
/// Whether or not this will render as something meaningful, or if it's skipped
39+
/// (which will force the containing struct to also skip printing a comma
40+
/// and the field name).
41+
fn should_render(&self) -> bool;
42+
3943
fn print_attribute(&self, p: &mut Printer);
4044
}
4145

4246
impl<T: PrintAttribute> PrintAttribute for &T {
43-
fn print_something(&self) -> bool {
44-
T::print_something(self)
47+
fn should_render(&self) -> bool {
48+
T::should_render(self)
4549
}
4650

4751
fn print_attribute(&self, p: &mut Printer) {
4852
T::print_attribute(self, p)
4953
}
5054
}
5155
impl<T: PrintAttribute> PrintAttribute for Option<T> {
52-
fn print_something(&self) -> bool {
53-
self.as_ref().is_some_and(|x| x.print_something())
56+
fn should_render(&self) -> bool {
57+
self.as_ref().is_some_and(|x| x.should_render())
5458
}
59+
5560
fn print_attribute(&self, p: &mut Printer) {
5661
if let Some(i) = self {
5762
T::print_attribute(i, p)
5863
}
5964
}
6065
}
6166
impl<T: PrintAttribute> PrintAttribute for ThinVec<T> {
62-
fn print_something(&self) -> bool {
63-
self.is_empty() || self[0].print_something()
67+
fn should_render(&self) -> bool {
68+
self.is_empty() || self[0].should_render()
6469
}
70+
6571
fn print_attribute(&self, p: &mut Printer) {
6672
let mut last_printed = false;
6773
p.word("[");
@@ -70,15 +76,15 @@ impl<T: PrintAttribute> PrintAttribute for ThinVec<T> {
7076
p.word_space(",");
7177
}
7278
i.print_attribute(p);
73-
last_printed = i.print_something();
79+
last_printed = i.should_render();
7480
}
7581
p.word("]");
7682
}
7783
}
7884
macro_rules! print_skip {
7985
($($t: ty),* $(,)?) => {$(
8086
impl PrintAttribute for $t {
81-
fn print_something(&self) -> bool { false }
87+
fn should_render(&self) -> bool { false }
8288
fn print_attribute(&self, _: &mut Printer) { }
8389
})*
8490
};
@@ -87,7 +93,7 @@ macro_rules! print_skip {
8793
macro_rules! print_disp {
8894
($($t: ty),* $(,)?) => {$(
8995
impl PrintAttribute for $t {
90-
fn print_something(&self) -> bool { true }
96+
fn should_render(&self) -> bool { true }
9197
fn print_attribute(&self, p: &mut Printer) {
9298
p.word(format!("{}", self));
9399
}
@@ -97,7 +103,7 @@ macro_rules! print_disp {
97103
macro_rules! print_debug {
98104
($($t: ty),* $(,)?) => {$(
99105
impl PrintAttribute for $t {
100-
fn print_something(&self) -> bool { true }
106+
fn should_render(&self) -> bool { true }
101107
fn print_attribute(&self, p: &mut Printer) {
102108
p.word(format!("{:?}", self));
103109
}
@@ -106,37 +112,39 @@ macro_rules! print_debug {
106112
}
107113

108114
macro_rules! print_tup {
109-
(num_print_something $($ts: ident)*) => { 0 $(+ $ts.print_something() as usize)* };
115+
(num_should_render $($ts: ident)*) => { 0 $(+ $ts.should_render() as usize)* };
110116
() => {};
111117
($t: ident $($ts: ident)*) => {
112118
#[allow(non_snake_case, unused)]
113119
impl<$t: PrintAttribute, $($ts: PrintAttribute),*> PrintAttribute for ($t, $($ts),*) {
114-
fn print_something(&self) -> bool {
120+
fn should_render(&self) -> bool {
115121
let ($t, $($ts),*) = self;
116-
print_tup!(num_print_something $t $($ts)*) != 0
122+
print_tup!(num_should_render $t $($ts)*) != 0
117123
}
118124

119125
fn print_attribute(&self, p: &mut Printer) {
120126
let ($t, $($ts),*) = self;
121-
let parens = print_tup!(num_print_something $t $($ts)*) > 1;
127+
let parens = print_tup!(num_should_render $t $($ts)*) > 1;
122128
if parens {
123-
p.word("(");
129+
p.popen();
124130
}
125131

126-
let mut printed_anything = $t.print_something();
132+
let mut printed_anything = $t.should_render();
127133

128134
$t.print_attribute(p);
129135

130136
$(
131-
if printed_anything && $ts.print_something() {
132-
p.word_space(",");
137+
if $ts.should_render() {
138+
if printed_anything {
139+
p.word_space(",");
140+
}
133141
printed_anything = true;
134142
}
135143
$ts.print_attribute(p);
136144
)*
137145

138146
if parens {
139-
p.word(")");
147+
p.pclose();
140148
}
141149
}
142150
}
@@ -147,5 +155,5 @@ macro_rules! print_tup {
147155

148156
print_tup!(A B C D E F G H);
149157
print_skip!(Span, ());
150-
print_disp!(Symbol, u16, bool, NonZero<u32>);
151-
print_debug!(UintTy, IntTy, Align, AttrStyle, CommentKind, Transparency);
158+
print_disp!(u16, bool, NonZero<u32>);
159+
print_debug!(Symbol, UintTy, IntTy, Align, AttrStyle, CommentKind, Transparency);

compiler/rustc_hir_pretty/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,9 @@ impl<'a> State<'a> {
118118
self.hardbreak()
119119
}
120120
hir::Attribute::Parsed(pa) => {
121-
self.word("#[attr=\"");
121+
self.word("#[attr = ");
122122
pa.print_attribute(self);
123-
self.word("\")]");
123+
self.word("]");
124124
self.hardbreak()
125125
}
126126
}

compiler/rustc_macros/src/print_attribute.rs

+16-11
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ fn print_fields(name: &Ident, fields: &Fields) -> (TokenStream, TokenStream, Tok
1616
let name = field.ident.as_ref().unwrap();
1717
let string_name = name.to_string();
1818
disps.push(quote! {
19-
if __printed_anything && #name.print_something() {
20-
__p.word_space(",");
19+
if #name.should_render() {
20+
if __printed_anything {
21+
__p.word_space(",");
22+
}
23+
__p.word(#string_name);
24+
__p.word_space(":");
2125
__printed_anything = true;
2226
}
23-
__p.word(#string_name);
24-
__p.word_space(":");
2527
#name.print_attribute(__p);
2628
});
2729
field_names.push(name);
@@ -31,10 +33,11 @@ fn print_fields(name: &Ident, fields: &Fields) -> (TokenStream, TokenStream, Tok
3133
quote! { {#(#field_names),*} },
3234
quote! {
3335
__p.word(#string_name);
34-
if true #(&& !#field_names.print_something())* {
36+
if true #(&& !#field_names.should_render())* {
3537
return;
3638
}
3739

40+
__p.nbsp();
3841
__p.word("{");
3942
#(#disps)*
4043
__p.word("}");
@@ -48,8 +51,10 @@ fn print_fields(name: &Ident, fields: &Fields) -> (TokenStream, TokenStream, Tok
4851
for idx in 0..fields_unnamed.unnamed.len() {
4952
let name = format_ident!("f{idx}");
5053
disps.push(quote! {
51-
if __printed_anything && #name.print_something() {
52-
__p.word_space(",");
54+
if #name.should_render() {
55+
if __printed_anything {
56+
__p.word_space(",");
57+
}
5358
__printed_anything = true;
5459
}
5560
#name.print_attribute(__p);
@@ -62,13 +67,13 @@ fn print_fields(name: &Ident, fields: &Fields) -> (TokenStream, TokenStream, Tok
6267
quote! {
6368
__p.word(#string_name);
6469

65-
if true #(&& !#field_names.print_something())* {
70+
if true #(&& !#field_names.should_render())* {
6671
return;
6772
}
6873

69-
__p.word("(");
74+
__p.popen();
7075
#(#disps)*
71-
__p.word(")");
76+
__p.pclose();
7277
},
7378
quote! { true },
7479
)
@@ -138,7 +143,7 @@ pub(crate) fn print_attribute(input: Structure<'_>) -> TokenStream {
138143
input.gen_impl(quote! {
139144
#[allow(unused)]
140145
gen impl PrintAttribute for @Self {
141-
fn print_something(&self) -> bool { #printed }
146+
fn should_render(&self) -> bool { #printed }
142147
fn print_attribute(&self, __p: &mut rustc_ast_pretty::pp::Printer) { #code }
143148
}
144149
})

tests/ui/unpretty/deprecated-attr.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
//@ compile-flags: -Zunpretty=hir
22
//@ check-pass
33

4-
// FIXME(jdonszelmann): the pretty printing output for deprecated (and possibly more attrs) is
5-
// slightly broken.
64
#[deprecated]
75
pub struct PlainDeprecated;
86

tests/ui/unpretty/deprecated-attr.stdout

+9-12
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,21 @@ extern crate std;
55
//@ compile-flags: -Zunpretty=hir
66
//@ check-pass
77

8-
// FIXME(jdonszelmann): the pretty printing output for deprecated (and possibly more attrs) is
9-
// slightly broken.
10-
#[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote:
11-
suggestion: }span: }")]
8+
#[attr = Deprecation {deprecation: Deprecation {since: Unspecified}}]
129
struct PlainDeprecated;
1310

14-
#[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote:
15-
here's why this is deprecatedsuggestion: }span: }")]
11+
#[attr = Deprecation {deprecation: Deprecation {since: Unspecified, note:
12+
"here's why this is deprecated"}}]
1613
struct DirectNote;
1714

18-
#[attr="Deprecation{deprecation: Deprecation{since: Unspecifiednote:
19-
here's why this is deprecatedsuggestion: }span: }")]
15+
#[attr = Deprecation {deprecation: Deprecation {since: Unspecified, note:
16+
"here's why this is deprecated"}}]
2017
struct ExplicitNote;
2118

22-
#[attr="Deprecation{deprecation: Deprecation{since: NonStandard(1.2.3)note:
23-
here's why this is deprecatedsuggestion: }span: }")]
19+
#[attr = Deprecation {deprecation: Deprecation {since: NonStandard("1.2.3"),
20+
note: "here's why this is deprecated"}}]
2421
struct SinceAndNote;
2522

26-
#[attr="Deprecation{deprecation: Deprecation{since: NonStandard(1.2.3)note:
27-
here's why this is deprecatedsuggestion: }span: }")]
23+
#[attr = Deprecation {deprecation: Deprecation {since: NonStandard("1.2.3"),
24+
note: "here's why this is deprecated"}}]
2825
struct FlippedOrder;

0 commit comments

Comments
 (0)