Refactor wasmprinter to rely less on a String output (#1592)

* Refactor how operand immediates are printed

Delegate the space-before-immediate to printing the immediate itself
rather than forcing a space between each immediate. Avoids the need to
`pop()` or otherwise test if space is already present in situations
where immediates are omitted.

* Add a test exercising labels

* Update how operators are printed in `wasmprinter`

Previously operators were printed to a temporary buffer and then
depending on what happened it would conditionally go into the actual
module or instead be discarded (e.g. the last `end`). This commit
refactors to instead avoid a temporary buffer entirely and
unconditionally emit instructions to the real output. This is done by
restructuring the printing slightly by having "before_op" and "after_op"
hooks while printing which are used to manage nesting/newlines instead
of the outer loop in the main printer. This is combined with a few other
minor things such as a `is_end_then_eof` method which is used to avoid
printing the final `end`.

One minor test update happened here with a `delegate` instruction from
the legacy exception-handling proposal which I think is a bugfix given
my read of the older proposal.

* Review comments

Author: alexcrichton

crates/wasmparser/src/binary_reader.rs

@@ -1712,6 +1712,12 @@ impl<'a> BinaryReader<'a> {
         self.visit_operator(&mut OperatorFactory::new())
     }
 
+    /// Returns whether there is an `end` opcode followed by eof remaining in
+    /// this reader.
+    pub fn is_end_then_eof(&self) -> bool {
+        self.remaining_buffer() == &[0x0b]
+    }
+
     fn read_lane_index(&mut self, max: u8) -> Result<u8> {
         let index = self.read_u8()?;
         if index >= max {

crates/wasmparser/src/readers/core/operators.rs

@@ -209,6 +209,12 @@ impl<'a> OperatorsReader<'a> {
     pub fn get_binary_reader(&self) -> BinaryReader<'a> {
         self.reader.clone()
     }
+
+    /// Returns whether there is an `end` opcode followed by eof remaining in
+    /// this reader.
+    pub fn is_end_then_eof(&self) -> bool {
+        self.reader.is_end_then_eof()
+    }
 }
 
 impl<'a> IntoIterator for OperatorsReader<'a> {

crates/wasmprinter/src/lib.rs

@@ -1281,9 +1281,9 @@ impl Printer {
 
         let nesting_start = self.nesting;
 
-        let mut buf = String::new();
-        let mut op_printer = operator::PrintOperator::new(self, state);
-        while !body.eof() {
+        let mut op_printer =
+            operator::PrintOperator::new(self, state, operator::OperatorSeparator::Newline);
+        while !body.is_end_then_eof() {
             // Branch hints are stored in increasing order of their body offset
             // so print them whenever their instruction comes up.
             if let Some(((hint_offset, hint), rest)) = branch_hints.split_first() {
@@ -1299,48 +1299,8 @@ impl Printer {
                 }
             }
 
-            let offset = body.original_position();
-            mem::swap(&mut buf, &mut op_printer.printer.result);
-            let op_kind = body.visit_operator(&mut op_printer)??;
-            mem::swap(&mut buf, &mut op_printer.printer.result);
-
-            match op_kind {
-                // The final `end` in a reader is not printed, it's implied
-                // in the text format.
-                operator::OpKind::End if body.eof() => break,
-
-                // When we start a block we newline to the current
-                // indentation, then we increase the indentation so further
-                // instructions are tabbed over.
-                operator::OpKind::BlockStart => {
-                    op_printer.printer.newline(offset);
-                    op_printer.printer.nesting += 1;
-                }
-
-                // `else`/`catch` are special in that it's printed at
-                // the previous indentation, but it doesn't actually change
-                // our nesting level.
-                operator::OpKind::BlockMid => {
-                    op_printer.printer.nesting -= 1;
-                    op_printer.printer.newline(offset);
-                    op_printer.printer.nesting += 1;
-                }
-
-                // Exiting a block prints `end` at the previous indentation
-                // level.
-                operator::OpKind::End | operator::OpKind::Delegate
-                    if op_printer.printer.nesting > nesting_start =>
-                {
-                    op_printer.printer.nesting -= 1;
-                    op_printer.printer.newline(offset);
-                }
-
-                // .. otherwise everything else just has a normal newline
-                // out in front.
-                _ => op_printer.printer.newline(offset),
-            }
-            op_printer.printer.result.push_str(&buf);
-            buf.truncate(0);
+            op_printer.op_offset = body.original_position();
+            body.visit_operator(&mut op_printer)??;
         }
 
         // If this was an invalid function body then the nesting may not
@@ -1596,40 +1556,14 @@ impl Printer {
         explicit: &str,
     ) -> Result<()> {
         self.start_group("");
-        let mut prev = mem::take(&mut self.result);
         let mut reader = expr.get_operators_reader();
-        let mut op_printer = operator::PrintOperator::new(self, state);
-        let mut first_op = None;
-        for i in 0.. {
-            if reader.eof() {
-                break;
-            }
-            match reader.visit_operator(&mut op_printer)?? {
-                operator::OpKind::End if reader.eof() => {}
 
-                _ if i == 0 => first_op = Some(mem::take(&mut op_printer.printer.result)),
-
-                _ => {
-                    if i == 1 {
-                        prev.push_str(explicit);
-                        prev.push(' ');
-                        prev.push_str(&first_op.take().unwrap());
-                    }
-                    prev.push(' ');
-                    prev.push_str(&op_printer.printer.result);
-                }
-            }
-
-            op_printer.printer.result.truncate(0);
+        if reader.read().is_ok() && !reader.is_end_then_eof() {
+            write!(self.result, "{explicit} ")?;
         }
 
-        // If `first_op` is still set here then it means we don't need to print
-        // an expression with `explicit` as the leading token, instead we can
-        // print the single operator.
-        if let Some(op) = first_op {
-            prev.push_str(&op);
-        }
-        self.result = prev;
+        self.print_const_expr(state, expr)?;
+
         self.end_group();
         Ok(())
     }
@@ -1639,23 +1573,16 @@ impl Printer {
         let mut reader = expr.get_operators_reader();
         let mut first = true;
 
-        let mut result = mem::take(&mut self.result);
-        let mut op_printer = operator::PrintOperator::new(self, state);
-        while !reader.eof() {
+        let mut op_printer =
+            operator::PrintOperator::new(self, state, operator::OperatorSeparator::None);
+        while !reader.is_end_then_eof() {
             if first {
                 first = false;
             } else {
                 op_printer.printer.result.push(' ');
             }
-            match reader.visit_operator(&mut op_printer)?? {
-                operator::OpKind::End if reader.eof() => {}
-                _ => {
-                    result.push_str(&op_printer.printer.result);
-                    op_printer.printer.result.truncate(0);
-                }
-            }
+            reader.visit_operator(&mut op_printer)??;
         }
-        self.result = result;
         Ok(())
     }