about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNicholas Nethercote <n.nethercote@gmail.com>2024-05-13 08:57:42 +1000
committerNicholas Nethercote <n.nethercote@gmail.com>2024-05-13 10:11:29 +1000
commit5e7a80b2d2462060e81295ea6a044f7012bba8cb (patch)
tree5e84e612d5933795a9f083fc7d53b37736ec0284
parent852a78ea8de3aa24c50457340d9560547bc67008 (diff)
downloadrust-5e7a80b2d2462060e81295ea6a044f7012bba8cb.tar.gz
rust-5e7a80b2d2462060e81295ea6a044f7012bba8cb.zip
Make handling of `Comments` more iterator-like.
The current way of stepping through each comment in `Comments` is a bit
weird. There is a `Vec<Comments>` and a `current` index, which is fine.
The `Comments::next` method clones the current comment but doesn't
advance `current`; the advancing instead happens in `print_comment`,
which is where each cloned comment is actually finally used (or not, in
some cases, if the comment fails to satisfy a predicate).

This commit makes things more iterator-like:
- `Comments::next` now advances `current` instead of `print_comment`.
- `Comments::peek` is added so you can inspect a comment and check a
  predicate without consuming it.
- This requires splitting `PrintState::comments` into immutable and
  mutable versions. The commit also moves the ref inside the `Option` of
  the return type, to save callers from having to use `as_ref`/`as_mut`.
- It also requires adding `PrintState::peek_comment` alongside the
  existing `PrintState::next_comment`. (The lifetimes in the signature
  of `peek_comment` ended up more complex than I expected.)

We now have a neat separation between consuming (`next`) and
non-consuming (`peek`) uses of each comment. As well as being clearer,
this will facilitate the next commit that avoids unnecessary cloning.
-rw-r--r--compiler/rustc_ast_pretty/src/pprust/state.rs56
-rw-r--r--compiler/rustc_hir_pretty/src/lib.rs8
2 files changed, 40 insertions, 24 deletions
diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs
index 2c176828c84..9eb552cdcdf 100644
--- a/compiler/rustc_ast_pretty/src/pprust/state.rs
+++ b/compiler/rustc_ast_pretty/src/pprust/state.rs
@@ -186,17 +186,23 @@ impl<'a> Comments<'a> {
         Comments { sm, comments, current: 0 }
     }
 
+    fn peek(&self) -> Option<&Comment> {
+        self.comments.get(self.current)
+    }
+
     // FIXME: This shouldn't probably clone lmao
-    fn next(&self) -> Option<Comment> {
-        self.comments.get(self.current).cloned()
+    fn next(&mut self) -> Option<Comment> {
+        let cmnt = self.comments.get(self.current).cloned();
+        self.current += 1;
+        cmnt
     }
 
     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;
             }
@@ -204,7 +210,7 @@ impl<'a> Comments<'a> {
             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());
             }
         }
 
@@ -400,7 +406,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);
 
@@ -442,18 +449,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() {
@@ -517,19 +524,20 @@ 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);
             }
         }
     }
@@ -537,11 +545,11 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
     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)
         }
     }
 
@@ -994,8 +1002,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) {
diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs
index 4f5fbd024a9..a47e6af0bf2 100644
--- a/compiler/rustc_hir_pretty/src/lib.rs
+++ b/compiler/rustc_hir_pretty/src/lib.rs
@@ -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) {