Skip to content

Commit

Permalink
Add #[non_exhaustive] return types to DataExporter::close and `Ex…
Browse files Browse the repository at this point in the history
…portDriver::export` (#5629)

#4716, #3746
  • Loading branch information
robertbastian authored Oct 1, 2024
1 parent f8534c9 commit efac644
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 38 deletions.
4 changes: 2 additions & 2 deletions provider/baked/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ impl DataExporter for BakedExporter {
}
}

fn close(&mut self) -> Result<(), DataError> {
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
log::info!("Writing macros module...");

let data = core::mem::take(&mut self.impl_data)
Expand Down Expand Up @@ -771,7 +771,7 @@ impl DataExporter for BakedExporter {

self.print_deps();

Ok(())
Ok(Default::default())
}
}

Expand Down
6 changes: 3 additions & 3 deletions provider/blob/src/export/blob_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl DataExporter for BlobExporter<'_> {
Ok(())
}

fn close(&mut self) -> Result<(), DataError> {
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
self.close_internal()
}
}
Expand Down Expand Up @@ -144,7 +144,7 @@ impl BlobExporter<'_> {
FinalizedBuffers { vzv, remap }
}

fn close_internal(&mut self) -> Result<(), DataError> {
fn close_internal(&mut self) -> Result<ExporterCloseMetadata, DataError> {
let FinalizedBuffers { vzv, remap } = self.finalize_buffers();

let all_markers = self.all_markers.lock().expect("poison");
Expand Down Expand Up @@ -200,6 +200,6 @@ impl BlobExporter<'_> {
}
}

Ok(())
Ok(Default::default())
}
}
18 changes: 13 additions & 5 deletions provider/core/src/export/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,16 @@ pub trait DataExporter: Sync {
/// This function has to be called before the object is dropped (after all
/// markers have been fully dumped). This conceptually takes ownership, so
/// clients *may not* interact with this object after close has been called.
fn close(&mut self) -> Result<(), DataError> {
Ok(())
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
Ok(ExporterCloseMetadata)
}
}

#[non_exhaustive]
#[derive(Debug, Clone, Default)]
/// Contains information about a successful export.
pub struct ExporterCloseMetadata;

/// Metadata for [`DataExporter::flush`]
#[non_exhaustive]
#[derive(Debug, Copy, Clone, Default)]
Expand Down Expand Up @@ -86,7 +91,7 @@ impl DataExporter for Box<dyn DataExporter> {
(**self).flush(marker, metadata)
}

fn close(&mut self) -> Result<(), DataError> {
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
(**self).close()
}
}
Expand Down Expand Up @@ -193,7 +198,10 @@ impl DataExporter for MultiExporter {
self.0.iter().try_for_each(|e| e.flush(marker, metadata))
}

fn close(&mut self) -> Result<(), DataError> {
self.0.iter_mut().try_for_each(|e| e.close())
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
self.0.iter_mut().try_fold(ExporterCloseMetadata, |m, e| {
e.close()?;
Ok(m)
})
}
}
26 changes: 6 additions & 20 deletions provider/export/src/export_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// called LICENSE at the top level of the ICU4X source tree
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

use crate::{DataLocaleFamilyAnnotations, DeduplicationStrategy, ExportDriver};
use crate::{DataLocaleFamilyAnnotations, DeduplicationStrategy, ExportDriver, ExportMetadata};
use icu_locale::fallback::LocaleFallbackIterator;
use icu_locale::LocaleFallbacker;
use icu_provider::export::*;
Expand All @@ -27,27 +27,11 @@ impl<T: IntoIterator> IntoParallelIterator for T {}
use rayon::prelude::*;

impl ExportDriver {
/// Exports data from the given provider to the given exporter.
///
/// See
/// [`make_exportable_provider!`](icu_provider::export::make_exportable_provider),
/// [`BlobExporter`](icu_provider_blob::export),
/// [`FileSystemExporter`](icu_provider_fs::export),
/// and [`BakedExporter`](icu_provider_baked::export).
pub fn export(
self,
provider: &impl ExportableProvider,
mut sink: impl DataExporter,
) -> Result<(), DataError> {
self.export_dyn(provider, &mut sink)
}

// Avoids multiple monomorphizations
fn export_dyn(
pub(crate) fn export_dyn(
self,
provider: &dyn ExportableProvider,
sink: &mut dyn DataExporter,
) -> Result<(), DataError> {
) -> Result<ExportMetadata, DataError> {
let Self {
markers,
requested_families,
Expand Down Expand Up @@ -263,7 +247,9 @@ impl ExportDriver {
Ok(())
})?;

sink.close()
let exporter = sink.close()?;

Ok(ExportMetadata { exporter })
}
}

Expand Down
27 changes: 27 additions & 0 deletions provider/export/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@

mod export_impl;
mod locale_family;
use icu_provider::export::ExporterCloseMetadata;
pub use locale_family::*;

#[cfg(feature = "baked_exporter")]
Expand All @@ -80,6 +81,8 @@ pub mod prelude {
}

use icu_locale::LocaleFallbacker;
use icu_provider::export::DataExporter;
use icu_provider::export::ExportableProvider;
use icu_provider::prelude::*;
use std::collections::HashMap;
use std::collections::HashSet;
Expand Down Expand Up @@ -263,6 +266,30 @@ impl ExportDriver {
let set = models.into_iter().collect::<HashSet<_>>();
self.with_marker_attributes_filter("segmenter", move |attrs| set.contains(attrs.as_str()))
}

/// Exports data from the given provider to the given exporter.
///
/// See
/// [`make_exportable_provider!`](icu_provider::export::make_exportable_provider),
/// [`BlobExporter`](icu_provider_blob::export),
/// [`FileSystemExporter`](icu_provider_fs::export),
/// and [`BakedExporter`](icu_provider_baked::export).
pub fn export(
self,
provider: &impl ExportableProvider,
mut sink: impl DataExporter,
) -> Result<ExportMetadata, DataError> {
// Avoids multiple monomorphizations
self.export_dyn(provider, &mut sink)
}
}

#[non_exhaustive]
#[derive(Debug, Clone)]
/// Contains information about a successful export.
pub struct ExportMetadata {
/// The metadata coming from the [`DataExporter`].
pub exporter: ExporterCloseMetadata,
}

/// Options bag configuring locale inclusion and behavior when runtime fallback is disabled.
Expand Down
4 changes: 2 additions & 2 deletions provider/icu4x-datagen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ fn main() -> eyre::Result<()> {
driver.with_segmenter_models(cli.segmenter_models.clone())
};

match cli.format {
let _metadata = match cli.format {
#[cfg(not(feature = "fs_exporter"))]
Format::Fs => {
eyre::bail!("Exporting to an FsProvider requires the `fs_exporter` Cargo feature")
Expand Down Expand Up @@ -598,7 +598,7 @@ fn main() -> eyre::Result<()> {
},
)?
})?,
}
};

Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions provider/source/src/tests/make_testdata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn make_testdata() {
matches!(attrs.as_str(), "CAD" | "EGP" | "EUR" | "GBP" | "USD")
})
.export(&provider, exporter)
.unwrap()
.unwrap();
}

struct ZeroCopyCheckExporter {
Expand Down Expand Up @@ -194,7 +194,7 @@ impl DataExporter for ZeroCopyCheckExporter {
Ok(())
}

fn close(&mut self) -> Result<(), DataError> {
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
assert_eq!(
self.rountrip_errors.get_mut().expect("poison"),
&mut BTreeSet::default()
Expand Down Expand Up @@ -225,7 +225,7 @@ impl DataExporter for ZeroCopyCheckExporter {
Expected:\n{EXPECTED_VIOLATIONS:?}\nFound:\n{violations:?}\nExpected (transient):\n{EXPECTED_TRANSIENT_VIOLATIONS:?}\nFound (transient):\n{transient_violations:?}"
);

Ok(())
Ok(Default::default())
}
}

Expand Down
6 changes: 3 additions & 3 deletions tools/make/bakeddata/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl<E: DataExporter> DataExporter for StubExporter<E> {
self.0.flush(marker, metadata)
}

fn close(&mut self) -> Result<(), DataError> {
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
self.0.close()
}
}
Expand Down Expand Up @@ -278,7 +278,7 @@ impl<F: Write + Send + Sync> DataExporter for StatisticsExporter<F> {
Ok(())
}

fn close(&mut self) -> Result<(), DataError> {
fn close(&mut self) -> Result<ExporterCloseMetadata, DataError> {
let data = core::mem::take(self.data.get_mut().expect("poison"));

let mut lines = Vec::new();
Expand Down Expand Up @@ -333,6 +333,6 @@ impl<F: Write + Send + Sync> DataExporter for StatisticsExporter<F> {
writeln!(&mut self.fingerprints, "{line}")?;
}

Ok(())
Ok(Default::default())
}
}

0 comments on commit efac644

Please sign in to comment.