Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add #[non_exhaustive] return types to DataExporter::close and ExportDriver::export #5629

Merged
merged 1 commit into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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())
}
}