Remove DataMarkerInfo's Display implementation (#5118)

I want to be explicit about code that actually requires the
`DataKeyPath`. Most code just needs a `Debug` implementation for
logging.

provider/baked/src/export.rs

@@ -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()
@@ -691,7 +691,7 @@ macro_rules! cb {
             )+
 
             $(
-                if *marker.path == *$epath {
+                if marker.path.as_str() == $epath {
                     return stringify!($emarker)
                         .replace("icu :: ", "icu_")
                         .parse()

provider/bikeshed/src/lib.rs

@@ -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
     }
 

provider/bikeshed/src/tests/make_testdata.rs

@@ -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()

provider/core/macros/src/lib.rs

@@ -84,14 +84,14 @@ mod tests;
 /// // Note: FooV1Marker implements `DynamicDataMarker` but not `DataMarker`.
 /// // The other two implement `DataMarker`.
 ///
-/// assert_eq!(&*BarV1Marker::INFO.path, "demo/bar@1");
+/// assert_eq!(BarV1Marker::INFO.path.as_str(), "demo/bar@1");
 /// assert_eq!(
 ///     BarV1Marker::INFO.fallback_config.priority,
 ///     LocaleFallbackPriority::Language
 /// );
 /// assert_eq!(BarV1Marker::INFO.fallback_config.extension_key, None);
 ///
-/// assert_eq!(&*BazV1Marker::INFO.path, "demo/baz@1");
+/// assert_eq!(BazV1Marker::INFO.path.as_str(), "demo/baz@1");
 /// assert_eq!(
 ///     BazV1Marker::INFO.fallback_config.priority,
 ///     LocaleFallbackPriority::Region

provider/core/src/error.rs

@@ -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>,
@@ -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}")?;
@@ -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,
         }
@@ -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,
         }
@@ -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,
         }
@@ -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,
         }
@@ -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)
     }
@@ -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,
         }

provider/core/src/marker.rs

@@ -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::*;
 
@@ -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(
@@ -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.
@@ -603,7 +591,7 @@ impl DataMarkerInfo {
     /// ));
     ///
     /// // The error context contains the argument:
-    /// assert_eq!(HelloWorldV1Marker::INFO.match_marker(DummyMarker::INFO).unwrap_err().marker, Some(DummyMarker::INFO.path));
+    /// assert_eq!(HelloWorldV1Marker::INFO.match_marker(DummyMarker::INFO).unwrap_err().marker_path, Some(DummyMarker::INFO.path));
     /// ```
     pub fn match_marker(self, marker: Self) -> Result<(), DataError> {
         if self == marker {
@@ -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:
@@ -774,7 +743,7 @@ fn test_path_to_string() {
             expected: "core/cardinal@65535",
         },
     ] {
-        assert_eq!(cas.expected, &*cas.path);
+        assert_eq!(cas.expected, cas.path.as_str());
     }
 }
 
@@ -818,6 +787,6 @@ fn test_path_hash() {
             hash: DataMarkerPathHash([176, 131, 182, 223]),
         },
     ] {
-        assert_eq!(cas.hash, cas.path.hashed(), "{}", &cas.path as &str);
+        assert_eq!(cas.hash, cas.path.hashed(), "{}", cas.path.as_str());
     }
 }

provider/datagen/src/driver.rs

@@ -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
@@ -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));
@@ -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();
@@ -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 {
@@ -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(());
             }
@@ -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(())
         })?;
@@ -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
@@ -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
@@ -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);
                     }
@@ -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(());

provider/datagen/tests/testutil.rs

@@ -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(())