Introduce BoxMarker to pretty-printing.

The pretty-printers open and close "boxes" of text a lot. The open and
close operations must be matched. The matching is currently all implicit
and very easy to get wrong. (rust-lang#140280 and rust-lang#140246 are two recent
pretty-printing fixes that both involved unclosed boxes.)

This commit introduces `BoxMarker`, a marker type that represents an
open box. It makes box opening/closing explicit, which makes it much
easier to understand and harder to get wrong.

The commit also removes many comments are on `end` calls saying things
like "end outer head-block", "Close the outer-box". These demonstrate
how confusing the implicit approach was, but aren't necessary any more.

Author: nnethercote

compiler/rustc_ast_pretty/src/lib.rs

@@ -2,6 +2,7 @@
 #![allow(internal_features)]
 #![doc(rust_logo)]
 #![feature(box_patterns)]
+#![feature(negative_impls)]
 #![feature(rustdoc_internals)]
 // tidy-alphabetical-end
 

compiler/rustc_ast_pretty/src/pp.rs

@@ -234,6 +234,43 @@ struct BufEntry {
     size: isize,
 }
 
+// Boxes opened with methods like `Printer::{cbox,ibox}` must be closed with
+// `Printer::end`. Failure to do so can result in bad indenting, or in extreme
+// cases, cause no output to be produced at all.
+//
+// Box opening and closing used to be entirely implicit, which was hard to
+// understand and easy to get wrong. This marker type is now returned from the
+// box opening methods and consumed by `Printer::end`. Any marker that isn't
+// closed, or closed more than once, will trigger a panic if debug assertions
+// are enabled.
+//
+// FIXME(nnethercote): the "isn't closed" assertion is currently disabled
+// because a few places fail to close their boxes. It can be enabled once they
+// are fixed.
+//
+// Note: it would be better to make open/close mismatching impossible and avoid
+// the need for this marker type altogether by having functions like
+// `with_ibox` that open a box, call a closure, and then close the box. That
+// would work for simple cases, but box lifetimes sometimes interact with
+// complex control flow and across function boundaries in ways that are
+// difficult to handle with such a technique.
+#[must_use]
+pub struct BoxMarker {
+    pub ended: bool,
+}
+
+impl !Clone for BoxMarker {}
+impl !Copy for BoxMarker {}
+
+impl Drop for BoxMarker {
+    fn drop(&mut self) {
+        if cfg!(debug_assertions) && !self.ended {
+            // FIXME(nnethercote): enable once the bad cases are fixed
+            //panic!("BoxMarker not ended");
+        }
+    }
+}
+
 impl Printer {
     pub fn new() -> Self {
         Printer {
@@ -270,23 +307,30 @@ impl Printer {
         }
     }
 
-    fn scan_begin(&mut self, token: BeginToken) {
+    // This is is where `BoxMarker`s are produced.
+    fn scan_begin(&mut self, token: BeginToken) -> BoxMarker {
         if self.scan_stack.is_empty() {
             self.left_total = 1;
             self.right_total = 1;
             self.buf.clear();
         }
         let right = self.buf.push(BufEntry { token: Token::Begin(token), size: -self.right_total });
         self.scan_stack.push_back(right);
+        BoxMarker { ended: false }
     }
 
-    fn scan_end(&mut self) {
+    // This is is where `BoxMarker`s are consumed.
+    fn scan_end(&mut self, mut b: BoxMarker) {
         if self.scan_stack.is_empty() {
             self.print_end();
         } else {
             let right = self.buf.push(BufEntry { token: Token::End, size: -1 });
             self.scan_stack.push_back(right);
         }
+        if cfg!(debug_assertions) && b.ended {
+            panic!("BoxMarker ended more than once");
+        }
+        b.ended = true;
     }
 
     fn scan_break(&mut self, token: BreakToken) {

compiler/rustc_ast_pretty/src/pp/convenience.rs

@@ -1,25 +1,27 @@
 use std::borrow::Cow;
 
-use crate::pp::{BeginToken, BreakToken, Breaks, IndentStyle, Printer, SIZE_INFINITY, Token};
+use crate::pp::{
+    BeginToken, BoxMarker, BreakToken, Breaks, IndentStyle, Printer, SIZE_INFINITY, Token,
+};
 
 impl Printer {
     /// "raw box"
-    pub fn rbox(&mut self, indent: isize, breaks: Breaks) {
+    pub fn rbox(&mut self, indent: isize, breaks: Breaks) -> BoxMarker {
         self.scan_begin(BeginToken { indent: IndentStyle::Block { offset: indent }, breaks })
     }
 
     /// Inconsistent breaking box
-    pub fn ibox(&mut self, indent: isize) {
+    pub fn ibox(&mut self, indent: isize) -> BoxMarker {
         self.rbox(indent, Breaks::Inconsistent)
     }
 
     /// Consistent breaking box
-    pub fn cbox(&mut self, indent: isize) {
+    pub fn cbox(&mut self, indent: isize) -> BoxMarker {
         self.rbox(indent, Breaks::Consistent)
     }
 
-    pub fn visual_align(&mut self) {
-        self.scan_begin(BeginToken { indent: IndentStyle::Visual, breaks: Breaks::Consistent });
+    pub fn visual_align(&mut self) -> BoxMarker {
+        self.scan_begin(BeginToken { indent: IndentStyle::Visual, breaks: Breaks::Consistent })
     }
 
     pub fn break_offset(&mut self, n: usize, off: isize) {
@@ -30,8 +32,8 @@ impl Printer {
         });
     }
 
-    pub fn end(&mut self) {
-        self.scan_end()
+    pub fn end(&mut self, b: BoxMarker) {
+        self.scan_end(b)
     }
 
     pub fn eof(mut self) -> String {