From 7e62276907e0680fc5741e7eba0d6cec1a877b4b Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 25 Oct 2022 23:44:53 -0400 Subject: [PATCH] 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 | 13 ++++---- tamer/src/bin/tameld.rs | 2 +- tamer/src/diagnose/panic.rs | 2 +- tamer/src/diagnose/report.rs | 33 ++++++++----------- tamer/src/diagnose/report/test/integration.rs | 14 ++++---- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/tamer/src/bin/tamec.rs b/tamer/src/bin/tamec.rs index 95fbd8c0..db13e10b 100644 --- a/tamer/src/bin/tamec.rs +++ b/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( 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( e: &TamecError, - reporter: &R, + reporter: &mut R, ebuf: &mut String, ) -> Result<(), TamecError> { // See below note about buffering. @@ -149,7 +148,7 @@ fn compile( 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. diff --git a/tamer/src/bin/tameld.rs b/tamer/src/bin/tameld.rs index d11f25ec..e7b70a50 100644 --- a/tamer/src/bin/tameld.rs +++ b/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 { diff --git a/tamer/src/diagnose/panic.rs b/tamer/src/diagnose/panic.rs index 5025c47b..6405e590 100644 --- a/tamer/src/diagnose/panic.rs +++ b/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() ); diff --git a/tamer/src/diagnose/report.rs b/tamer/src/diagnose/report.rs index 649a050a..e54fc87a 100644 --- a/tamer/src/diagnose/report.rs +++ b/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 { /// /// 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, + resolver: R, /// Number of reports with a severity level of error or higher. - err_n: RefCell, + err_n: usize, } impl VisualReporter { pub fn new(resolver: R) -> Self { - Self { - resolver: RefCell::new(resolver), - err_n: RefCell::new(0), - } + Self { resolver, err_n: 0 } } } impl Reporter for VisualReporter { - 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 Reporter for VisualReporter { 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 Reporter for VisualReporter { } fn error_count(&self) -> usize { - *self.err_n.borrow() + self.err_n } } diff --git a/tamer/src/diagnose/report/test/integration.rs b/tamer/src/diagnose/report/test/integration.rs index 1af88a15..7be5a276 100644 --- a/tamer/src/diagnose/report/test/integration.rs +++ b/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()); }