Auto merge of #125055 - nnethercote:Comment-FIXME, r=compiler-errors

Avoid clone in `Comments::next`

`Comments::next`, in `rustc_ast_pretty`, has this comment:
```
// FIXME: This shouldn't probably clone lmao
```
The obvious thing to try is to return `Option<&Comment>` instead of `Option<Comment>`. But that leads to multiple borrows all over the place, because `Comments` must be borrowed from `PrintState` and then processed by `&mut self` methods within `PrintState`.

This PR instead rearranges things so that comments are consumed as they are used, preserving the `Option<Comment>` return type without requiring any cloning.

r? `@compiler-errors`
This commit is contained in:
bors 2024-05-13 06:30:25 +00:00
commit 982c9c1e81
2 changed files with 43 additions and 29 deletions

View File

@ -55,8 +55,8 @@ impl PpAnn for NoAnn {}
pub struct Comments<'a> { pub struct Comments<'a> {
sm: &'a SourceMap, sm: &'a SourceMap,
comments: Vec<Comment>, // Stored in reverse order so we can consume them by popping.
current: usize, reversed_comments: Vec<Comment>,
} }
/// Returns `None` if the first `col` chars of `s` contain a non-whitespace char. /// Returns `None` if the first `col` chars of `s` contain a non-whitespace char.
@ -182,21 +182,25 @@ fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec<Comment>
impl<'a> Comments<'a> { impl<'a> Comments<'a> {
pub fn new(sm: &'a SourceMap, filename: FileName, input: String) -> Comments<'a> { pub fn new(sm: &'a SourceMap, filename: FileName, input: String) -> Comments<'a> {
let comments = gather_comments(sm, filename, input); let mut comments = gather_comments(sm, filename, input);
Comments { sm, comments, current: 0 } comments.reverse();
Comments { sm, reversed_comments: comments }
} }
// FIXME: This shouldn't probably clone lmao fn peek(&self) -> Option<&Comment> {
fn next(&self) -> Option<Comment> { self.reversed_comments.last()
self.comments.get(self.current).cloned() }
fn next(&mut self) -> Option<Comment> {
self.reversed_comments.pop()
} }
fn trailing_comment( fn trailing_comment(
&self, &mut self,
span: rustc_span::Span, span: rustc_span::Span,
next_pos: Option<BytePos>, next_pos: Option<BytePos>,
) -> Option<Comment> { ) -> Option<Comment> {
if let Some(cmnt) = self.next() { if let Some(cmnt) = self.peek() {
if cmnt.style != CommentStyle::Trailing { if cmnt.style != CommentStyle::Trailing {
return None; return None;
} }
@ -204,7 +208,7 @@ fn trailing_comment(
let comment_line = self.sm.lookup_char_pos(cmnt.pos); let comment_line = self.sm.lookup_char_pos(cmnt.pos);
let next = next_pos.unwrap_or_else(|| cmnt.pos + BytePos(1)); let next = next_pos.unwrap_or_else(|| cmnt.pos + BytePos(1));
if span.hi() < cmnt.pos && cmnt.pos < next && span_line.line == comment_line.line { if span.hi() < cmnt.pos && cmnt.pos < next && span_line.line == comment_line.line {
return Some(cmnt); return Some(self.next().unwrap());
} }
} }
@ -400,7 +404,8 @@ fn deref_mut(&mut self) -> &mut Self::Target {
/// This trait is used for both AST and HIR pretty-printing. /// This trait is used for both AST and HIR pretty-printing.
pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::DerefMut { pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::DerefMut {
fn comments(&mut self) -> &mut Option<Comments<'a>>; fn comments(&self) -> Option<&Comments<'a>>;
fn comments_mut(&mut self) -> Option<&mut Comments<'a>>;
fn ann_post(&mut self, ident: Ident); fn ann_post(&mut self, ident: Ident);
fn print_generic_args(&mut self, args: &ast::GenericArgs, colons_before_params: bool); fn print_generic_args(&mut self, args: &ast::GenericArgs, colons_before_params: bool);
@ -442,18 +447,18 @@ fn commasep<T, F>(&mut self, b: Breaks, elts: &[T], op: F)
fn maybe_print_comment(&mut self, pos: BytePos) -> bool { fn maybe_print_comment(&mut self, pos: BytePos) -> bool {
let mut has_comment = false; let mut has_comment = false;
while let Some(cmnt) = self.next_comment() { while let Some(cmnt) = self.peek_comment() {
if cmnt.pos < pos { if cmnt.pos >= pos {
has_comment = true;
self.print_comment(&cmnt);
} else {
break; break;
} }
has_comment = true;
let cmnt = self.next_comment().unwrap();
self.print_comment(cmnt);
} }
has_comment has_comment
} }
fn print_comment(&mut self, cmnt: &Comment) { fn print_comment(&mut self, cmnt: Comment) {
match cmnt.style { match cmnt.style {
CommentStyle::Mixed => { CommentStyle::Mixed => {
if !self.is_beginning_of_line() { if !self.is_beginning_of_line() {
@ -517,19 +522,20 @@ fn print_comment(&mut self, cmnt: &Comment) {
self.hardbreak(); self.hardbreak();
} }
} }
if let Some(cmnts) = self.comments() { }
cmnts.current += 1;
} fn peek_comment<'b>(&'b self) -> Option<&'b Comment> where 'a: 'b {
self.comments().and_then(|c| c.peek())
} }
fn next_comment(&mut self) -> Option<Comment> { fn next_comment(&mut self) -> Option<Comment> {
self.comments().as_mut().and_then(|c| c.next()) self.comments_mut().and_then(|c| c.next())
} }
fn maybe_print_trailing_comment(&mut self, span: rustc_span::Span, next_pos: Option<BytePos>) { fn maybe_print_trailing_comment(&mut self, span: rustc_span::Span, next_pos: Option<BytePos>) {
if let Some(cmnts) = self.comments() { if let Some(cmnts) = self.comments_mut() {
if let Some(cmnt) = cmnts.trailing_comment(span, next_pos) { if let Some(cmnt) = cmnts.trailing_comment(span, next_pos) {
self.print_comment(&cmnt); self.print_comment(cmnt);
} }
} }
} }
@ -537,11 +543,11 @@ fn maybe_print_trailing_comment(&mut self, span: rustc_span::Span, next_pos: Opt
fn print_remaining_comments(&mut self) { fn print_remaining_comments(&mut self) {
// If there aren't any remaining comments, then we need to manually // If there aren't any remaining comments, then we need to manually
// make sure there is a line break at the end. // make sure there is a line break at the end.
if self.next_comment().is_none() { if self.peek_comment().is_none() {
self.hardbreak(); self.hardbreak();
} }
while let Some(cmnt) = self.next_comment() { while let Some(cmnt) = self.next_comment() {
self.print_comment(&cmnt) self.print_comment(cmnt)
} }
} }
@ -994,8 +1000,12 @@ fn to_string(f: impl FnOnce(&mut State<'_>)) -> String {
} }
impl<'a> PrintState<'a> for State<'a> { impl<'a> PrintState<'a> for State<'a> {
fn comments(&mut self) -> &mut Option<Comments<'a>> { fn comments(&self) -> Option<&Comments<'a>> {
&mut self.comments self.comments.as_ref()
}
fn comments_mut(&mut self) -> Option<&mut Comments<'a>> {
self.comments.as_mut()
} }
fn ann_post(&mut self, ident: Ident) { fn ann_post(&mut self, ident: Ident) {

View File

@ -139,8 +139,12 @@ fn deref_mut(&mut self) -> &mut Self::Target {
} }
impl<'a> PrintState<'a> for State<'a> { impl<'a> PrintState<'a> for State<'a> {
fn comments(&mut self) -> &mut Option<Comments<'a>> { fn comments(&self) -> Option<&Comments<'a>> {
&mut self.comments self.comments.as_ref()
}
fn comments_mut(&mut self) -> Option<&mut Comments<'a>> {
self.comments.as_mut()
} }
fn ann_post(&mut self, ident: Ident) { fn ann_post(&mut self, ident: Ident) {