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

Remove DataMarkerInfo's Display implementation #5118

Merged
merged 6 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 3 additions & 3 deletions provider/baked/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,12 +677,12 @@ impl DataExporter for BakedExporter {
macro_rules! cb {
($($marker:path = $path:literal,)+ #[experimental] $($emarker:path = $epath:literal,)+) => {
fn bake_marker(marker: DataMarkerInfo) -> databake::TokenStream {
if *marker.path == *icu_provider::hello_world::HelloWorldV1Marker::INFO.path {
if marker.path.as_str() == icu_provider::hello_world::HelloWorldV1Marker::INFO.path.as_str() {
return databake::quote!(icu_provider::hello_world::HelloWorldV1Marker);
}

$(
if *marker.path == *$path {
if marker.path.as_str() == $path {
return stringify!($marker)
.replace("icu :: ", "icu_")
.parse()
Expand All @@ -691,7 +691,7 @@ macro_rules! cb {
)+

$(
if *marker.path == *$epath {
if marker.path.as_str() == $epath {
return stringify!($emarker)
.replace("icu :: ", "icu_")
.parse()
Expand Down
6 changes: 3 additions & 3 deletions provider/bikeshed/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,19 +253,19 @@ impl DatagenProvider {

/// Identifies errors that are due to missing CLDR data.
pub fn is_missing_cldr_error(mut e: DataError) -> bool {
e.marker = None;
e.marker_path = None;
e == Self::MISSING_CLDR_ERROR
}

/// Identifies errors that are due to missing ICU export data.
pub fn is_missing_icuexport_error(mut e: DataError) -> bool {
e.marker = None;
e.marker_path = None;
e == Self::MISSING_ICUEXPORT_ERROR
}

/// Identifies errors that are due to missing segmenter LSTM data.
pub fn is_missing_segmenter_lstm_error(mut e: DataError) -> bool {
e.marker = None;
e.marker_path = None;
e == Self::MISSING_SEGMENTER_LSTM_ERROR
}

Expand Down
4 changes: 2 additions & 2 deletions provider/bikeshed/src/tests/make_testdata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@ impl DataExporter for ZeroCopyCheckExporter {

if deallocated != allocated {
if !EXPECTED_VIOLATIONS.contains(&marker) {
eprintln!("Zerocopy violation {marker} {locale}: {allocated}B allocated, {deallocated}B deallocated");
eprintln!("Zerocopy violation {marker:?} {locale}: {allocated}B allocated, {deallocated}B deallocated");
}
self.zero_copy_violations
.lock()
.expect("poison")
.insert(marker);
} else if allocated > 0 {
if !EXPECTED_TRANSIENT_VIOLATIONS.contains(&marker) {
eprintln!("Transient zerocopy violation {marker} {locale}: {allocated}B allocated/deallocated");
eprintln!("Transient zerocopy violation {marker:?} {locale}: {allocated}B allocated/deallocated");
}
self.zero_copy_transient_violations
.lock()
Expand Down
18 changes: 9 additions & 9 deletions provider/core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub struct DataError {
pub kind: DataErrorKind,

/// The data marker of the request, if available.
pub marker: Option<DataMarkerPath>,
pub marker_path: Option<DataMarkerPath>,

/// Additional context, if available.
pub str_context: Option<&'static str>,
Expand All @@ -109,8 +109,8 @@ impl fmt::Display for DataError {
if self.kind != DataErrorKind::Custom {
write!(f, ": {}", self.kind)?;
}
if let Some(marker) = self.marker {
write!(f, " (marker: {})", &marker as &str)?;
if let Some(marker) = self.marker_path {
write!(f, " (marker: {})", marker.as_str())?;
}
if let Some(str_context) = self.str_context {
write!(f, ": {str_context}")?;
Expand All @@ -127,7 +127,7 @@ impl DataErrorKind {
pub const fn into_error(self) -> DataError {
DataError {
kind: self,
marker: None,
marker_path: None,
str_context: None,
silent: false,
}
Expand Down Expand Up @@ -164,7 +164,7 @@ impl DataError {
pub const fn custom(str_context: &'static str) -> Self {
Self {
kind: DataErrorKind::Custom,
marker: None,
marker_path: None,
str_context: Some(str_context),
silent: false,
}
Expand All @@ -175,7 +175,7 @@ impl DataError {
pub const fn with_marker(self, marker: DataMarkerInfo) -> Self {
Self {
kind: self.kind,
marker: Some(marker.path),
marker_path: Some(marker.path),
str_context: self.str_context,
silent: self.silent,
}
Expand All @@ -186,7 +186,7 @@ impl DataError {
pub const fn with_str_context(self, context: &'static str) -> Self {
Self {
kind: self.kind,
marker: self.marker,
marker_path: self.marker_path,
str_context: Some(context),
silent: self.silent,
}
Expand All @@ -211,7 +211,7 @@ impl DataError {
}
// Don't write out a log for MissingDataMarker since there is no context to add
if !self.silent && self.kind != DataErrorKind::MissingDataMarker {
log::warn!("{} (marker: {}, request: {})", self, marker, req);
log::warn!("{self} (marker: {marker:?}, request: {req})");
}
self.with_marker(marker)
}
Expand Down Expand Up @@ -258,7 +258,7 @@ impl DataError {
pub(crate) fn for_type<T>() -> DataError {
DataError {
kind: DataErrorKind::MismatchedType(core::any::type_name::<T>()),
marker: None,
marker_path: None,
str_context: None,
silent: false,
}
Expand Down
35 changes: 2 additions & 33 deletions provider/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,8 @@

use crate::fallback::LocaleFallbackConfig;
use crate::{DataError, DataErrorKind, DataProvider, DataProviderWithMarker};
use alloc::borrow::Cow;
use core::fmt;
use core::fmt::Write;
use core::marker::PhantomData;
use core::ops::Deref;
use writeable::{LengthHint, Writeable};
use yoke::Yokeable;
use zerovec::ule::*;

Expand Down Expand Up @@ -489,7 +485,7 @@ impl DataMarkerPath {

/// Gets the path as a static string slice.
#[inline]
pub const fn get(self) -> &'static str {
pub const fn as_str(self) -> &'static str {
unsafe {
// Safe due to invariant that self.path is tagged correctly
core::str::from_utf8_unchecked(core::slice::from_raw_parts(
Expand Down Expand Up @@ -520,14 +516,6 @@ impl DataMarkerPath {
}
}

impl Deref for DataMarkerPath {
type Target = str;
#[inline]
fn deref(&self) -> &Self::Target {
self.get()
}
}

/// Used for loading data from a dynamic ICU4X data provider.
///
/// A data marker is tightly coupled with the code that uses it to load data at runtime.
Expand Down Expand Up @@ -641,29 +629,10 @@ pub use __data_marker_path as data_marker_path;

impl fmt::Debug for DataMarkerInfo {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str("DataMarkerInfo{")?;
fmt::Display::fmt(self, f)?;
f.write_char('}')?;
Ok(())
}
}

impl Writeable for DataMarkerInfo {
fn write_to<W: core::fmt::Write + ?Sized>(&self, sink: &mut W) -> core::fmt::Result {
self.path.write_to(sink)
}

fn writeable_length_hint(&self) -> LengthHint {
self.path.writeable_length_hint()
}

fn write_to_string(&self) -> Cow<str> {
Cow::Borrowed(self.path.get())
f.write_str(self.path.as_str())
}
}

writeable::impl_display_with_writeable!(DataMarkerInfo);

#[test]
fn test_path_syntax() {
// Valid paths:
Expand Down
32 changes: 16 additions & 16 deletions provider/datagen/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,7 @@ impl DatagenDriver {
);

let load_with_fallback = |marker, locale: &_, marker_attributes: &_| {
log::trace!("Generating marker/locale: {marker}/{locale:}");
log::trace!("Generating marker/locale: {marker:?}/{locale}");
let mut metadata = DataRequestMetadata::default();
metadata.silent = true;
// Lazy-compute the fallback iterator so that we don't always require CLDR data
Expand All @@ -565,7 +565,7 @@ impl DatagenDriver {
Ok(data_response) => {
if let Some(iter) = locale_iter.as_ref() {
if iter.get().is_und() && !locale.is_und() {
log::debug!("Falling back to und: {marker}/{locale}");
log::debug!("Falling back to und: {marker:?}/{locale}");
}
}
return Some(Ok(data_response.payload));
Expand All @@ -576,7 +576,7 @@ impl DatagenDriver {
}) => {
if let Some(iter) = locale_iter.as_mut() {
if iter.get().is_und() {
log::debug!("Could not find data for: {marker}/{locale}");
log::debug!("Could not find data for: {marker:?}/{locale}");
return None;
}
iter.step();
Expand All @@ -594,7 +594,7 @@ impl DatagenDriver {
};

markers.clone().into_par_iter().try_for_each(|marker| {
log::trace!("Generating marker {marker}");
log::trace!("Generating marker {marker:?}");
let instant1 = Instant::now();

if marker.is_singleton {
Expand Down Expand Up @@ -623,12 +623,12 @@ impl DatagenDriver {
if final_duration > Duration::new(0, 500_000_000) {
// Print durations if the marker took longer than 500 ms
log::info!(
"Generated marker {marker} ({}, flushed in {})",
"Generated marker {marker:?} ({}, flushed in {})",
DisplayDuration(final_duration),
DisplayDuration(flush_duration)
);
} else {
log::info!("Generated marker {marker}");
log::info!("Generated marker {marker:?}");
}
return Ok(());
}
Expand Down Expand Up @@ -712,13 +712,13 @@ impl DatagenDriver {
if final_duration > Duration::new(0, 500_000_000) {
// Print durations if the marker took longer than 500 ms
log::info!(
"Generated marker {marker} ({}, '{slowest_locale}' in {}, flushed in {})",
"Generated marker {marker:?} ({}, '{slowest_locale}' in {}, flushed in {})",
DisplayDuration(final_duration),
DisplayDuration(slowest_duration),
DisplayDuration(flush_duration)
);
} else {
log::info!("Generated marker {marker}");
log::info!("Generated marker {marker:?}");
}
Ok(())
})?;
Expand Down Expand Up @@ -752,21 +752,21 @@ fn select_locales_for_marker(
.insert((locale, marker_attributes));
}

if marker.path.get().starts_with("segmenter/dictionary/") {
if marker.path.as_str().starts_with("segmenter/dictionary/") {
supported_map.retain(|_, locales| {
locales.retain(|(_, attrs)| segmenter_models.iter().any(|m| **m == **attrs));
!locales.is_empty()
});
// Don't perform additional locale filtering
return Ok(supported_map.into_values().flatten().collect());
} else if marker.path.get().starts_with("segmenter/lstm/") {
} else if marker.path.as_str().starts_with("segmenter/lstm/") {
supported_map.retain(|_, locales| {
locales.retain(|(_, attrs)| segmenter_models.iter().any(|m| **m == **attrs));
!locales.is_empty()
});
// Don't perform additional locale filtering
return Ok(supported_map.into_values().flatten().collect());
} else if marker.path.get().starts_with("collator/") {
} else if marker.path.as_str().starts_with("collator/") {
supported_map.retain(|_, locales| {
locales.retain(|(locale, _)| {
let Some(collation) = locale
Expand Down Expand Up @@ -810,12 +810,12 @@ fn select_locales_for_marker(
.cloned()
.unwrap_or_default();
if include_full && !selected_langids.contains(current_langid) {
log::trace!("Including {current_langid}: full locale family: {marker}");
log::trace!("Including {current_langid}: full locale family: {marker:?}");
selected_langids.insert(current_langid.clone());
}
if current_langid.language.is_empty() && current_langid != &LanguageIdentifier::UND
{
log::trace!("Including {current_langid}: und variant: {marker}");
log::trace!("Including {current_langid}: und variant: {marker:?}");
selected_langids.insert(current_langid.clone());
}
let include_ancestors = requested_families
Expand All @@ -837,13 +837,13 @@ fn select_locales_for_marker(
.unwrap_or(false);
if include_descendants && !selected_langids.contains(current_langid) {
log::trace!(
"Including {current_langid}: descendant of {parent_langid}: {marker}"
"Including {current_langid}: descendant of {parent_langid}: {marker:?}"
);
selected_langids.insert(current_langid.clone());
}
if include_ancestors && !selected_langids.contains(&parent_langid) {
log::trace!(
"Including {parent_langid}: ancestor of {current_langid}: {marker}"
"Including {parent_langid}: ancestor of {current_langid}: {marker:?}"
);
selected_langids.insert(parent_langid);
}
Expand Down Expand Up @@ -922,7 +922,7 @@ fn deduplicate_payloads<const MAXIMAL: bool>(
if inherited_payload == payload {
// Found a match: don't need to write anything
log::trace!(
"Deduplicating {marker}/{locale} (inherits from {})",
"Deduplicating {marker:?}/{locale} (inherits from {})",
iter.get()
);
return Ok(());
Expand Down
2 changes: 1 addition & 1 deletion provider/datagen/tests/testutil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl DataExporter for &mut TestingExporter {
.output
.finalize()
.expect("Failed to finalize serializer output");
println!("Putting: {marker}/{}/{locale}", marker_attributes as &str);
println!("Putting: {marker:?}/{}/{locale}", marker_attributes as &str);
self.0
.insert((locale.clone(), marker_attributes.clone()), output);
Ok(())
Expand Down
4 changes: 2 additions & 2 deletions provider/fs/src/export/fs_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl DataExporter for FilesystemExporter {
obj: &DataPayload<ExportMarker>,
) -> Result<(), DataError> {
let mut path_buf = self.root.clone().into_os_string();
write!(&mut path_buf, "/{marker}").expect("infallible");
write!(&mut path_buf, "/{}", marker.path.as_str()).expect("infallible");
if !marker_attributes.is_empty() {
write!(&mut path_buf, "/{}", marker_attributes as &str).expect("infallible");
}
Expand Down Expand Up @@ -140,7 +140,7 @@ impl DataExporter for FilesystemExporter {

fn flush(&self, marker: DataMarkerInfo) -> Result<(), DataError> {
let mut path_buf = self.root.clone().into_os_string();
write!(&mut path_buf, "/{marker}").expect("infallible");
write!(&mut path_buf, "/{}", marker.path.as_str()).expect("infallible");

if !Path::new(&path_buf).exists() {
fs::create_dir_all(&path_buf)
Expand Down
2 changes: 1 addition & 1 deletion provider/fs/src/fs_data_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl DynamicDataProvider<BufferMarker> for FsDataProvider {
return Err(DataErrorKind::ExtraneousLocale.with_req(marker, req));
}
let mut path = self.root.clone().into_os_string();
write!(&mut path, "/{marker}").expect("infallible");
write!(&mut path, "/{}", marker.path.as_str()).expect("infallible");
if !Path::new(&path).exists() {
return Err(DataErrorKind::MissingDataMarker.with_req(marker, req));
}
Expand Down
4 changes: 2 additions & 2 deletions tools/bakeddata-scripts/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,10 +275,10 @@ impl<F: Write + Send + Sync> DataExporter for PostcardFingerprintExporter<F> {
if let Some(deduped_req) = seen.get(hash) {
writeln!(
&mut self.fingerprints,
"{marker}, {req}, {size}B, -> {deduped_req}",
"{marker:?}, {req}, {size}B, -> {deduped_req}",
)?;
} else {
writeln!(&mut self.fingerprints, "{marker}, {req}, {size}B, {hash:x}",)?;
writeln!(&mut self.fingerprints, "{marker:?}, {req}, {size}B, {hash:x}",)?;
seen.insert(hash, req);
}
}
Expand Down
Loading