Skip to content

Commit 5e7a80b

Browse files
committed
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.
1 parent 852a78e commit 5e7a80b

File tree

2 files changed

+40
-24
lines changed

2 files changed

+40
-24
lines changed

Diff for: compiler/rustc_ast_pretty/src/pprust/state.rs

+34-22
Original file line numberDiff line numberDiff line change
@@ -186,25 +186,31 @@ impl<'a> Comments<'a> {
186186
Comments { sm, comments, current: 0 }
187187
}
188188

189+
fn peek(&self) -> Option<&Comment> {
190+
self.comments.get(self.current)
191+
}
192+
189193
// FIXME: This shouldn't probably clone lmao
190-
fn next(&self) -> Option<Comment> {
191-
self.comments.get(self.current).cloned()
194+
fn next(&mut self) -> Option<Comment> {
195+
let cmnt = self.comments.get(self.current).cloned();
196+
self.current += 1;
197+
cmnt
192198
}
193199

194200
fn trailing_comment(
195-
&self,
201+
&mut self,
196202
span: rustc_span::Span,
197203
next_pos: Option<BytePos>,
198204
) -> Option<Comment> {
199-
if let Some(cmnt) = self.next() {
205+
if let Some(cmnt) = self.peek() {
200206
if cmnt.style != CommentStyle::Trailing {
201207
return None;
202208
}
203209
let span_line = self.sm.lookup_char_pos(span.hi());
204210
let comment_line = self.sm.lookup_char_pos(cmnt.pos);
205211
let next = next_pos.unwrap_or_else(|| cmnt.pos + BytePos(1));
206212
if span.hi() < cmnt.pos && cmnt.pos < next && span_line.line == comment_line.line {
207-
return Some(cmnt);
213+
return Some(self.next().unwrap());
208214
}
209215
}
210216

@@ -400,7 +406,8 @@ impl std::ops::DerefMut for State<'_> {
400406

401407
/// This trait is used for both AST and HIR pretty-printing.
402408
pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::DerefMut {
403-
fn comments(&mut self) -> &mut Option<Comments<'a>>;
409+
fn comments(&self) -> Option<&Comments<'a>>;
410+
fn comments_mut(&mut self) -> Option<&mut Comments<'a>>;
404411
fn ann_post(&mut self, ident: Ident);
405412
fn print_generic_args(&mut self, args: &ast::GenericArgs, colons_before_params: bool);
406413

@@ -442,18 +449,18 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
442449

443450
fn maybe_print_comment(&mut self, pos: BytePos) -> bool {
444451
let mut has_comment = false;
445-
while let Some(cmnt) = self.next_comment() {
446-
if cmnt.pos < pos {
447-
has_comment = true;
448-
self.print_comment(&cmnt);
449-
} else {
452+
while let Some(cmnt) = self.peek_comment() {
453+
if cmnt.pos >= pos {
450454
break;
451455
}
456+
has_comment = true;
457+
let cmnt = self.next_comment().unwrap();
458+
self.print_comment(cmnt);
452459
}
453460
has_comment
454461
}
455462

456-
fn print_comment(&mut self, cmnt: &Comment) {
463+
fn print_comment(&mut self, cmnt: Comment) {
457464
match cmnt.style {
458465
CommentStyle::Mixed => {
459466
if !self.is_beginning_of_line() {
@@ -517,31 +524,32 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
517524
self.hardbreak();
518525
}
519526
}
520-
if let Some(cmnts) = self.comments() {
521-
cmnts.current += 1;
522-
}
527+
}
528+
529+
fn peek_comment<'b>(&'b self) -> Option<&'b Comment> where 'a: 'b {
530+
self.comments().and_then(|c| c.peek())
523531
}
524532

525533
fn next_comment(&mut self) -> Option<Comment> {
526-
self.comments().as_mut().and_then(|c| c.next())
534+
self.comments_mut().and_then(|c| c.next())
527535
}
528536

529537
fn maybe_print_trailing_comment(&mut self, span: rustc_span::Span, next_pos: Option<BytePos>) {
530-
if let Some(cmnts) = self.comments() {
538+
if let Some(cmnts) = self.comments_mut() {
531539
if let Some(cmnt) = cmnts.trailing_comment(span, next_pos) {
532-
self.print_comment(&cmnt);
540+
self.print_comment(cmnt);
533541
}
534542
}
535543
}
536544

537545
fn print_remaining_comments(&mut self) {
538546
// If there aren't any remaining comments, then we need to manually
539547
// make sure there is a line break at the end.
540-
if self.next_comment().is_none() {
548+
if self.peek_comment().is_none() {
541549
self.hardbreak();
542550
}
543551
while let Some(cmnt) = self.next_comment() {
544-
self.print_comment(&cmnt)
552+
self.print_comment(cmnt)
545553
}
546554
}
547555

@@ -994,8 +1002,12 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
9941002
}
9951003

9961004
impl<'a> PrintState<'a> for State<'a> {
997-
fn comments(&mut self) -> &mut Option<Comments<'a>> {
998-
&mut self.comments
1005+
fn comments(&self) -> Option<&Comments<'a>> {
1006+
self.comments.as_ref()
1007+
}
1008+
1009+
fn comments_mut(&mut self) -> Option<&mut Comments<'a>> {
1010+
self.comments.as_mut()
9991011
}
10001012

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

Diff for: compiler/rustc_hir_pretty/src/lib.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -139,8 +139,12 @@ impl std::ops::DerefMut for State<'_> {
139139
}
140140

141141
impl<'a> PrintState<'a> for State<'a> {
142-
fn comments(&mut self) -> &mut Option<Comments<'a>> {
143-
&mut self.comments
142+
fn comments(&self) -> Option<&Comments<'a>> {
143+
self.comments.as_ref()
144+
}
145+
146+
fn comments_mut(&mut self) -> Option<&mut Comments<'a>> {
147+
self.comments.as_mut()
144148
}
145149

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

0 commit comments

Comments
 (0)