Skip to content

Commit

Permalink
reporting: use ThinVec for storing diagnostics
Browse files Browse the repository at this point in the history
Swapping to `ThinVec` reduces the size of diagnostic stores
in order to avoid bloating things like the `SemanticAnalyser`,
`AstGen`, `Lexer`. This reduces store size from `48bytes`
to `16bytes` per diagnostic store.

Additionally, remove manual implementations of diagnostic access
for `Lexer` and `SemanticAnalyser`.
  • Loading branch information
feds01 committed Aug 31, 2023
1 parent bd4f661 commit 78f173c
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 117 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

62 changes: 10 additions & 52 deletions compiler/hash-lexer/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Hash Compiler lexer error data types.
use std::{cell::Cell, convert::Infallible, fmt::Display, mem::take};
use std::{cell::Cell, fmt::Display};

use hash_reporting::{
diagnostic::{AccessToDiagnosticsMut, DiagnosticsMut},
diagnostic::{AccessToDiagnosticsMut, DiagnosticStore},
report::{Report, ReportElement, ReportNote, ReportNoteKind},
reporter::{Reporter, Reports},
};
Expand Down Expand Up @@ -184,65 +184,23 @@ impl From<LexerError> for Reports {
#[derive(Default)]
pub struct LexerDiagnostics {
/// Inner stored diagnostics from the lexer.
errors: Vec<LexerError>,
store: DiagnosticStore<LexerError, ()>,

/// Whether the [Lexer] encountered a fatal error and
/// must abort on the next token advance
pub(crate) has_fatal_error: Cell<bool>,
}

impl LexerDiagnostics {
pub fn into_reports(&mut self) -> Vec<Report> {
self.errors.drain(..).flat_map(Reports::from).collect()
}
}

impl DiagnosticsMut for LexerDiagnostics {
type Error = LexerError;
type Warning = Infallible;

/// Add an error into the store
fn add_error(&mut self, error: LexerError) {
self.errors.push(error);
}

/// The lexer does not currently emit any warnings and so if this
/// is called, it should panic.
fn add_warning(&mut self, warning: Infallible) {
match warning {}
}

fn has_errors(&self) -> bool {
!self.errors.is_empty()
}

/// Lexer never emits any warnings so this always false
fn has_warnings(&self) -> bool {
false
}

fn into_diagnostics(&mut self) -> (Vec<LexerError>, Vec<Infallible>) {
(take(&mut self.errors), vec![])
}

fn merge_diagnostics(
&mut self,
mut other: impl DiagnosticsMut<Error = LexerError, Warning = Infallible>,
) {
let (errors, _) = other.into_diagnostics();
self.errors.extend(errors)
}
impl AccessToDiagnosticsMut for Lexer<'_> {
type Diagnostics = DiagnosticStore<LexerError, ()>;

fn clear_diagnostics(&mut self) {
self.errors.clear();
self.has_fatal_error.set(false);
fn diagnostics(&mut self) -> &mut Self::Diagnostics {
&mut self.diagnostics.store
}
}

impl AccessToDiagnosticsMut for Lexer<'_> {
type Diagnostics = LexerDiagnostics;

fn diagnostics(&mut self) -> &mut Self::Diagnostics {
&mut self.diagnostics
impl Lexer<'_> {
pub fn into_reports(&mut self) -> Vec<Report> {
self.diagnostics.store.errors.drain(..).flat_map(Reports::from).collect()
}
}
20 changes: 11 additions & 9 deletions compiler/hash-reporting/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use std::{
mem::take,
};

use hash_utils::thin_vec::ThinVec;

use crate::reporter::Reports;

/// This macro creates `Diagnostics{,Mut}` trait definitions, which provide
Expand Down Expand Up @@ -67,7 +69,7 @@ macro_rules! make_diagnostic_traits {
/// immediately converting the diagnostics into [Report]s.
///
/// This will modify self.
fn into_diagnostics(self: $self_ref) -> (Vec<Self::Error>, Vec<Self::Warning>);
fn into_diagnostics(self: $self_ref) -> (ThinVec<Self::Error>, ThinVec<Self::Warning>);

/// Merge another diagnostic store with this one.
fn merge_diagnostics(self: $self_ref, other: impl $name<Error=Self::Error, Warning=Self::Warning>);
Expand All @@ -84,13 +86,13 @@ make_diagnostic_traits!(Diagnostics with &Self, DiagnosticsMut with &mut Self);
/// A standard implementation of [Diagnostics] that uses a [RefCell] to store
/// errors and warnings immutably.
pub struct DiagnosticCellStore<E, W> {
pub errors: RefCell<Vec<E>>,
pub warnings: RefCell<Vec<W>>,
pub errors: RefCell<ThinVec<E>>,
pub warnings: RefCell<ThinVec<W>>,
}

impl<E, W> DiagnosticCellStore<E, W> {
pub fn new() -> Self {
Self { errors: RefCell::new(Vec::new()), warnings: RefCell::new(Vec::new()) }
Self { errors: RefCell::new(ThinVec::new()), warnings: RefCell::new(ThinVec::new()) }
}
}

Expand Down Expand Up @@ -144,7 +146,7 @@ impl<E, W> Diagnostics for DiagnosticCellStore<E, W> {
!self.warnings.borrow().is_empty()
}

fn into_diagnostics(&self) -> (Vec<E>, Vec<W>) {
fn into_diagnostics(&self) -> (ThinVec<E>, ThinVec<W>) {
// This drains all the errors and warnings from the diagnostics store.
let mut errors = self.errors.borrow_mut();
let mut warnings = self.warnings.borrow_mut();
Expand All @@ -161,13 +163,13 @@ impl<E, W> Diagnostics for DiagnosticCellStore<E, W> {
/// A standard implementation of [DiagnosticsMut] that stores errors and
/// warnings directly, and thus is mutable.
pub struct DiagnosticStore<E, W> {
pub errors: Vec<E>,
pub warnings: Vec<W>,
pub errors: ThinVec<E>,
pub warnings: ThinVec<W>,
}

impl<E, W> DiagnosticStore<E, W> {
pub fn new() -> Self {
Self { errors: Vec::new(), warnings: Vec::new() }
Self { errors: ThinVec::new(), warnings: ThinVec::new() }
}
}

Expand Down Expand Up @@ -203,7 +205,7 @@ impl<E, W> DiagnosticsMut for DiagnosticStore<E, W> {
!self.warnings.is_empty()
}

fn into_diagnostics(&mut self) -> (Vec<E>, Vec<W>) {
fn into_diagnostics(&mut self) -> (ThinVec<E>, ThinVec<W>) {
(take(&mut self.errors), take(&mut self.warnings))
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/hash-semantics/src/diagnostics/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use hash_reporting::{
use hash_source::location::Span;
use hash_tir::{symbols::Symbol, terms::TermId, utils::common::get_location};
use hash_typecheck::errors::{TcError, TcErrorReporter};
use hash_utils::thin_vec::ThinVec;

use crate::{
environment::sem_env::{AccessToSemEnv, WithSemEnv},
Expand All @@ -17,7 +18,7 @@ use crate::{
#[derive(Clone, Debug)]
pub enum SemanticError {
/// A series of errors.
Compound { errors: Vec<SemanticError> },
Compound { errors: ThinVec<SemanticError> },

/// An error exists, this is just a signal to stop typechecking.
Signal,
Expand Down
3 changes: 2 additions & 1 deletion compiler/hash-semantics/src/diagnostics/warning.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use hash_exhaustiveness::diagnostics::ExhaustivenessWarning;
use hash_reporting::reporter::{Reporter, Reports};
use hash_utils::thin_vec::ThinVec;

use crate::environment::sem_env::{AccessToSemEnv, WithSemEnv};

/// Warnings that can originate from the semantic analysis phase.
#[derive(Clone, Debug)]
pub enum SemanticWarning {
/// Compounded warnings.
Compound { warnings: Vec<SemanticWarning> },
Compound { warnings: ThinVec<SemanticWarning> },

/// A warning that comes from exhaustive pattern checking and
/// analysis.
Expand Down
3 changes: 2 additions & 1 deletion compiler/hash-untyped-semantics/src/analysis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl<'s> SemanticAnalyser<'s> {
/// Given a [Sender], send all of the generated warnings and messaged into
/// the sender.
pub(crate) fn send_generated_messages(self, sender: &Sender<AnalysisDiagnostic>) {
self.diagnostics.items.into_iter().for_each(|t| sender.send(t).unwrap())
self.diagnostics.store.errors.into_iter().for_each(|t| sender.send(t.into()).unwrap());
self.diagnostics.store.warnings.into_iter().for_each(|t| sender.send(t.into()).unwrap());
}
}
61 changes: 8 additions & 53 deletions compiler/hash-untyped-semantics/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
use hash_ast::ast::AstNodeId;
use hash_reporting::{
diagnostic::{AccessToDiagnosticsMut, DiagnosticsMut},
diagnostic::{AccessToDiagnosticsMut, DiagnosticStore},
reporter::Reports,
};
use hash_utils::derive_more::From;

use self::{error::AnalysisError, warning::AnalysisWarning};
use crate::analysis::SemanticAnalyser;
Expand All @@ -15,10 +16,13 @@ pub(crate) mod warning;

/// A representation of either a [AnalysisWarning] or [AnalysisError]. This
/// is used when they are accumulated and converted into reports at the end.
#[derive(From)]
pub enum AnalysisDiagnostic {
/// Warnings that are emitted by the analysis pass.
#[from]
Warning(AnalysisWarning),
/// Errors that are emitted by the analysis pass.
#[from]
Error(AnalysisError),
}

Expand All @@ -45,62 +49,13 @@ impl From<AnalysisDiagnostic> for Reports {
#[derive(Default)]
pub struct AnalyserDiagnostics {
/// Any diagnostics that the [SemanticAnalyser] produces.
pub(crate) items: Vec<AnalysisDiagnostic>,
}

impl DiagnosticsMut for AnalyserDiagnostics {
type Error = AnalysisError;
type Warning = AnalysisWarning;

fn add_error(&mut self, error: AnalysisError) {
self.items.push(AnalysisDiagnostic::Error(error));
}

fn add_warning(&mut self, warning: AnalysisWarning) {
self.items.push(AnalysisDiagnostic::Warning(warning));
}

fn has_errors(&self) -> bool {
!self.items.iter().any(|d| matches!(d, AnalysisDiagnostic::Error(_)))
}

fn has_warnings(&self) -> bool {
!self.items.iter().any(|d| matches!(d, AnalysisDiagnostic::Warning(_)))
}

fn into_diagnostics(&mut self) -> (Vec<AnalysisError>, Vec<AnalysisWarning>) {
let mut errors = vec![];
let mut warnings = vec![];

for item in self.items.drain(..) {
match item {
AnalysisDiagnostic::Warning(w) => warnings.push(w),
AnalysisDiagnostic::Error(e) => errors.push(e),
}
}

(errors, warnings)
}

fn merge_diagnostics(
&mut self,
mut other: impl DiagnosticsMut<Error = AnalysisError, Warning = AnalysisWarning>,
) {
let (errors, warnings) = other.into_diagnostics();

self.items.extend(errors.into_iter().map(AnalysisDiagnostic::Error));
self.items.extend(warnings.into_iter().map(AnalysisDiagnostic::Warning));
}

fn clear_diagnostics(&mut self) {
self.items.clear();
}
pub(crate) store: DiagnosticStore<AnalysisError, AnalysisWarning>,
}

impl AccessToDiagnosticsMut for SemanticAnalyser<'_> {
type Diagnostics = AnalyserDiagnostics;
type Diagnostics = DiagnosticStore<AnalysisError, AnalysisWarning>;

fn diagnostics(&mut self) -> &mut Self::Diagnostics {
&mut self.diagnostics
&mut self.diagnostics.store
}
}
1 change: 1 addition & 0 deletions compiler/hash-utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ backtrace = "0.3"
bitflags = "2.4.0"
crossbeam-channel = "0.5.1"
dashmap = "5.1"
derive_more = "0.99"
fixedbitset = "0.4.2"
fxhash = "0.2.1"
index_vec = "0.1.3"
Expand Down
1 change: 1 addition & 0 deletions compiler/hash-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub use backtrace;
pub use bitflags;
pub use crossbeam_channel;
pub use dashmap;
pub use derive_more;
pub use fxhash;
pub use index_vec;
pub use indexmap;
Expand Down

0 comments on commit 78f173c

Please sign in to comment.