Skip to content

Commit

Permalink
Auto merge of rust-lang#125055 - nnethercote:Comment-FIXME, r=compile…
Browse files Browse the repository at this point in the history
…r-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`
  • Loading branch information
bors committed May 13, 2024
2 parents ba956ef + 74e1b46 commit 982c9c1
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 29 deletions.
64 changes: 37 additions & 27 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ impl PpAnn for NoAnn {}

pub struct Comments<'a> {
sm: &'a SourceMap,
comments: Vec<Comment>,
current: usize,
// Stored in reverse order so we can consume them by popping.
reversed_comments: Vec<Comment>,
}

/// Returns `None` if the first `col` chars of `s` contain a non-whitespace char.
Expand Down Expand Up @@ -182,29 +182,33 @@ fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec<Comment>

impl<'a> Comments<'a> {
pub fn new(sm: &'a SourceMap, filename: FileName, input: String) -> Comments<'a> {
let comments = gather_comments(sm, filename, input);
Comments { sm, comments, current: 0 }
let mut comments = gather_comments(sm, filename, input);
comments.reverse();
Comments { sm, reversed_comments: comments }
}

// FIXME: This shouldn't probably clone lmao
fn next(&self) -> Option<Comment> {
self.comments.get(self.current).cloned()
fn peek(&self) -> Option<&Comment> {
self.reversed_comments.last()
}

fn next(&mut self) -> Option<Comment> {
self.reversed_comments.pop()
}

fn trailing_comment(
&self,
&mut self,
span: rustc_span::Span,
next_pos: Option<BytePos>,
) -> Option<Comment> {
if let Some(cmnt) = self.next() {
if let Some(cmnt) = self.peek() {
if cmnt.style != CommentStyle::Trailing {
return None;
}
let span_line = self.sm.lookup_char_pos(span.hi());
let comment_line = self.sm.lookup_char_pos(cmnt.pos);
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 {
return Some(cmnt);
return Some(self.next().unwrap());
}
}

Expand Down Expand Up @@ -400,7 +404,8 @@ impl std::ops::DerefMut for State<'_> {

/// This trait is used for both AST and HIR pretty-printing.
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 print_generic_args(&mut self, args: &ast::GenericArgs, colons_before_params: bool);

Expand Down Expand Up @@ -442,18 +447,18 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere

fn maybe_print_comment(&mut self, pos: BytePos) -> bool {
let mut has_comment = false;
while let Some(cmnt) = self.next_comment() {
if cmnt.pos < pos {
has_comment = true;
self.print_comment(&cmnt);
} else {
while let Some(cmnt) = self.peek_comment() {
if cmnt.pos >= pos {
break;
}
has_comment = true;
let cmnt = self.next_comment().unwrap();
self.print_comment(cmnt);
}
has_comment
}

fn print_comment(&mut self, cmnt: &Comment) {
fn print_comment(&mut self, cmnt: Comment) {
match cmnt.style {
CommentStyle::Mixed => {
if !self.is_beginning_of_line() {
Expand Down Expand Up @@ -517,31 +522,32 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
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> {
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>) {
if let Some(cmnts) = self.comments() {
if let Some(cmnts) = self.comments_mut() {
if let Some(cmnt) = cmnts.trailing_comment(span, next_pos) {
self.print_comment(&cmnt);
self.print_comment(cmnt);
}
}
}

fn print_remaining_comments(&mut self) {
// If there aren't any remaining comments, then we need to manually
// make sure there is a line break at the end.
if self.next_comment().is_none() {
if self.peek_comment().is_none() {
self.hardbreak();
}
while let Some(cmnt) = self.next_comment() {
self.print_comment(&cmnt)
self.print_comment(cmnt)
}
}

Expand Down Expand Up @@ -994,8 +1000,12 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
}

impl<'a> PrintState<'a> for State<'a> {
fn comments(&mut self) -> &mut Option<Comments<'a>> {
&mut self.comments
fn comments(&self) -> Option<&Comments<'a>> {
self.comments.as_ref()
}

fn comments_mut(&mut self) -> Option<&mut Comments<'a>> {
self.comments.as_mut()
}

fn ann_post(&mut self, ident: Ident) {
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,12 @@ impl std::ops::DerefMut for State<'_> {
}

impl<'a> PrintState<'a> for State<'a> {
fn comments(&mut self) -> &mut Option<Comments<'a>> {
&mut self.comments
fn comments(&self) -> Option<&Comments<'a>> {
self.comments.as_ref()
}

fn comments_mut(&mut self) -> Option<&mut Comments<'a>> {
self.comments.as_mut()
}

fn ann_post(&mut self, ident: Ident) {
Expand Down

0 comments on commit 982c9c1

Please sign in to comment.