tamer: Revert "tamer: diagnose::report::Report: {Mutable=>immutable} self reference"

This reverts commit 85ec626fcd804eb2fac3fd6f0339182554f72cfd.

This revert had to be modified to work alongside other changes.  Interior
mutability is fortunately no longer needed after the previous commit which
allows reporting to occur in a single place in the lowering pipeline (at the
terminal parser).

DEV-13158

tamer/src/bin/tamec.rs

@@ -26,7 +26,6 @@ extern crate tamer;
 
 use getopts::{Fail, Options};
 use std::{
-    cell::RefCell,
     env,
     error::Error,
     fmt::{self, Display, Write},
@@ -103,18 +102,18 @@ fn copy_xml_to<'e, W: io::Write + 'e>(
 fn compile<R: Reporter>(
     src_path: &String,
     dest_path: &String,
-    reporter: &R,
+    reporter: &mut R,
 ) -> Result<(), TamecError> {
     let dest = Path::new(&dest_path);
     let fout = BufWriter::new(fs::File::create(dest)?);
 
     let escaper = DefaultEscaper::default();
 
-    let ebuf = RefCell::new(String::new());
+    let mut ebuf = String::new();
 
     fn report_err<R: Reporter>(
         e: &TamecError,
-        reporter: &R,
+        reporter: &mut R,
         ebuf: &mut String,
     ) -> Result<(), TamecError> {
         // See below note about buffering.
@@ -149,7 +148,7 @@ fn compile<R: Reporter>(
             nir.fold(Ok(()), |x, result| match result {
                 Ok(_) => x,
                 Err(e) => {
-                    report_err(&e, reporter, &mut ebuf.borrow_mut())?;
+                    report_err(&e, reporter, &mut ebuf)?;
                     x
                 }
             })
@@ -178,9 +177,9 @@ pub fn main() -> Result<(), TamecError> {
 
     match parse_options(opts, args) {
         Ok(Command::Compile(src_path, _, dest_path)) => {
-            let reporter = VisualReporter::new(FsSpanResolver);
+            let mut reporter = VisualReporter::new(FsSpanResolver);
 
-            compile(&src_path, &dest_path, &reporter).or_else(
+            compile(&src_path, &dest_path, &mut reporter).or_else(
                 |e: TamecError| {
                     // POC: Rendering to a string ensures buffering so that we don't
                     //   interleave output between processes.

tamer/src/bin/tameld.rs

@@ -59,7 +59,7 @@ pub fn main() -> Result<(), TameldError> {
     let usage =
         opts.usage(&format!("Usage: {} [OPTIONS] -o OUTPUT FILE", program));
 
-    let reporter = VisualReporter::new(FsSpanResolver);
+    let mut reporter = VisualReporter::new(FsSpanResolver);
 
     match parse_options(opts, args) {
         Ok(Command::Link(input, output, emit)) => match emit {

tamer/src/diagnose/panic.rs

@@ -123,7 +123,7 @@ macro_rules! diagnostic_panic {
     ($desc_data:expr, $($panic_args:tt)*) => {{
         use crate::diagnose::Reporter;
 
-        let reporter = crate::diagnose::panic::PanicReporter::new(
+        let mut reporter = crate::diagnose::panic::PanicReporter::new(
             Default::default()
         );
 

tamer/src/diagnose/report.rs

@@ -39,7 +39,6 @@ use super::{
 };
 use crate::span::{Context, Span, UNKNOWN_SPAN};
 use std::{
-    cell::RefCell,
     fmt::{self, Display, Write},
     num::NonZeroU32,
     ops::Add,
@@ -73,7 +72,8 @@ pub trait Reporter {
     ///     ensuring both that the user is made aware of the problem
     ///     and that we're not inadvertently suppressing the actual
     ///       diagnostic messages that were requested.
-    fn render<'d, D: Diagnostic>(&self, diagnostic: &'d D) -> Report<'d, D>;
+    fn render<'d, D: Diagnostic>(&mut self, diagnostic: &'d D)
+        -> Report<'d, D>;
 
     /// Whether any reports have been rendered with an error level or higher.
     fn has_errors(&self) -> bool;
@@ -97,32 +97,25 @@ pub struct VisualReporter<R: SpanResolver> {
     ///
     /// This is responsible for resolving a span to a filename with line and
     ///   column numbers.
-    ///
-    /// This uses interior mutability since it is in our best interest to
-    ///   permit multiple references to a single resolver---the
-    ///     shared resolver caching is far more important than saving on a
-    ///     cell lock check.
-    resolver: RefCell<R>,
+    resolver: R,
 
     /// Number of reports with a severity level of error or higher.
-    err_n: RefCell<usize>,
+    err_n: usize,
 }
 
 impl<R: SpanResolver> VisualReporter<R> {
     pub fn new(resolver: R) -> Self {
-        Self {
-            resolver: RefCell::new(resolver),
-            err_n: RefCell::new(0),
-        }
+        Self { resolver, err_n: 0 }
     }
 }
 
 impl<R: SpanResolver> Reporter for VisualReporter<R> {
-    fn render<'d, D: Diagnostic>(&self, diagnostic: &'d D) -> Report<'d, D> {
-        let mspans = describe_resolved(
-            |span| self.resolver.borrow_mut().resolve(span),
-            diagnostic,
-        );
+    fn render<'d, D: Diagnostic>(
+        &mut self,
+        diagnostic: &'d D,
+    ) -> Report<'d, D> {
+        let mspans =
+            describe_resolved(|span| self.resolver.resolve(span), diagnostic);
 
         let mut report = Report::empty(Message(diagnostic));
         report.extend(mspans.map(Into::into));
@@ -134,7 +127,7 @@ impl<R: SpanResolver> Reporter for VisualReporter<R> {
         if report.level.is_error() {
             // Not worried about overflow panic
             //   (you have bigger problems if there are that many errors).
-            *self.err_n.borrow_mut() += 1;
+            self.err_n += 1;
         }
 
         report
@@ -145,7 +138,7 @@ impl<R: SpanResolver> Reporter for VisualReporter<R> {
     }
 
     fn error_count(&self) -> usize {
-        *self.err_n.borrow()
+        self.err_n
     }
 }
 

tamer/src/diagnose/report/test/integration.rs

@@ -144,7 +144,7 @@ fn new_sut() -> impl Reporter {
 
 macro_rules! assert_report {
     ($msg:expr, $aspans:expr, $expected:expr) => {
-        let sut = new_sut();
+        let mut sut = new_sut();
 
         assert_eq!(
             sut.render(&StubError($msg.into(), $aspans)).to_string(),
@@ -596,10 +596,10 @@ error: wide gutter
 
 #[test]
 fn visual_reporter_tracks_errors() {
-    let sut = new_sut();
+    let sut = &mut new_sut();
     let ctx = Context::from("error/tracking");
 
-    fn feed_aspan(sut: &impl Reporter, aspan: AnnotatedSpan<'static>) {
+    fn feed_aspan(sut: &mut impl Reporter, aspan: AnnotatedSpan<'static>) {
         // We do not care about the report value;
         //   we're only interested in how it tracks errors for this test.
         let _ = sut.render(&StubError("ignored".into(), vec![aspan]));
@@ -610,22 +610,22 @@ fn visual_reporter_tracks_errors() {
     assert!(!sut.has_errors());
 
     // Help must not increment.
-    feed_aspan(&sut, ctx.span(0, 1).help("no increment"));
+    feed_aspan(sut, ctx.span(0, 1).help("no increment"));
     assert_eq!(sut.error_count(), 0);
     assert!(!sut.has_errors());
 
     // Note must not increment.
-    feed_aspan(&sut, ctx.span(0, 1).note("no increment"));
+    feed_aspan(sut, ctx.span(0, 1).note("no increment"));
     assert_eq!(sut.error_count(), 0);
     assert!(!sut.has_errors());
 
     // Error must increment.
-    feed_aspan(&sut, ctx.span(0, 1).error("increment"));
+    feed_aspan(sut, ctx.span(0, 1).error("increment"));
     assert_eq!(sut.error_count(), 1);
     assert!(sut.has_errors());
 
     // Internal error must increment.
-    feed_aspan(&sut, ctx.span(0, 1).error("increment"));
+    feed_aspan(sut, ctx.span(0, 1).error("increment"));
     assert_eq!(sut.error_count(), 2);
     assert!(sut.has_errors());
 }