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

Don't print newlines in APITs #111929

Merged
merged 1 commit into from
May 26, 2023
Merged

Don't print newlines in APITs #111929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1425,7 +1425,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
DefPathData::ImplTrait,
span,
);
let ident = Ident::from_str_and_span(&pprust::ty_to_string(t), span);

// HACK: pprust breaks strings with newlines when the type
// gets too long. We don't want these to show up in compiler
// output or built artifacts, so replace them here...
// Perhaps we should instead format APITs more robustly.
Copy link
Member

Choose a reason for hiding this comment

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

Can we change this in pprust instead? I don't know why we'd want it to add the newline ... if it's just for -Zunpretty=expanded that doesn't seem worth adding hacks for.

Copy link
Member Author

Choose a reason for hiding this comment

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

This newline is not impl Trait specifically. This is just the regular line breaking that pprust does for code that is too long:

rust/compiler/rustc_ast_pretty/src/pp.rs

Lines 417 to 435 in 096309e

fn print_break(&mut self, token: BreakToken, size: isize) {
let fits = match self.get_top() {
PrintFrame::Fits => true,
PrintFrame::Broken { breaks: Breaks::Consistent, .. } => false,
PrintFrame::Broken { breaks: Breaks::Inconsistent, .. } => size <= self.space,
};
if fits {
self.pending_indentation += token.blank_space;
self.space -= token.blank_space;
} else {
if let Some(pre_break) = token.pre_break {
self.out.push(pre_break);
}
self.out.push('\n');
let indent = self.indent as isize + token.offset;
self.pending_indentation = indent;
self.space = cmp::max(MARGIN - indent, MIN_SPACE);
}
}

I don't think we should remove pprust's ability to break lines, and I don't think it's worth it to introduce extra configurables (like breaking only with spaces). I'd rather have the hack be as localized as possible, or else rewrite this formatting code to no longer rely on pprust at all.

let ident = Ident::from_str_and_span(
&pprust::ty_to_string(t).replace('\n', " "),
span,
);

let (param, bounds, path) = self.lower_universal_param_and_bounds(
*def_node_id,
span,
Expand Down
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/arg-position-impl-trait-too-long.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
struct Header;
struct EntryMetadata;
struct Entry<A, B>(A, B);

trait Tr {
type EncodedKey;
type EncodedValue;
}

fn test<C: Tr, R>(
// This APIT is long, however we shouldn't render the type name with a newline in it.
y: impl FnOnce(
&mut Header,
&mut [EntryMetadata],
&mut [Entry<C::EncodedKey, C::EncodedValue>]
) -> R,
) {
let () = y;
//~^ ERROR mismatched types
}

fn main() {}
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/arg-position-impl-trait-too-long.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0308]: mismatched types
--> $DIR/arg-position-impl-trait-too-long.rs:18:9
|
LL | y: impl FnOnce(
| ________-
LL | | &mut Header,
LL | | &mut [EntryMetadata],
LL | | &mut [Entry<C::EncodedKey, C::EncodedValue>]
LL | | ) -> R,
| |__________- this type parameter
LL | ) {
LL | let () = y;
| ^^ - this expression has type `impl FnOnce(&mut Header, &mut [EntryMetadata], &mut [Entry<C::EncodedKey, C::EncodedValue>]) -> R`
| |
| expected type parameter `impl FnOnce(&mut Header, &mut [EntryMetadata], &mut [Entry<C::EncodedKey, C::EncodedValue>]) -> R`, found `()`
|
= note: expected type parameter `impl FnOnce(&mut Header, &mut [EntryMetadata], &mut [Entry<C::EncodedKey, C::EncodedValue>]) -> R`
found unit type `()`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.