simplify all optional<str> getters

nyurik · Feb 4, 2025 · 94b0fb0 · 94b0fb0
1 parent 157f6aa
commit 94b0fb0
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 60 deletions.
diff --git a/platform/rust/include/wrapper.h b/platform/rust/include/wrapper.h
@@ -11,18 +11,38 @@ uint32_t get_42();
 using namespace mbgl;
 
 // TileServerOptions constructor helpers
-std::unique_ptr<TileServerOptions> TileServerOptions_new();
-std::unique_ptr<TileServerOptions> TileServerOptions_default();
-std::unique_ptr<TileServerOptions> TileServerOptions_mapbox();
-std::unique_ptr<TileServerOptions> TileServerOptions_maplibre();
-std::unique_ptr<TileServerOptions> TileServerOptions_maptiler();
+inline std::unique_ptr<TileServerOptions> TileServerOptions_new() {
+  return std::make_unique<TileServerOptions>();
+}
+inline std::unique_ptr<TileServerOptions> TileServerOptions_mapbox() {
+  return std::make_unique<TileServerOptions>(TileServerOptions::MapLibreConfiguration());
+}
+inline std::unique_ptr<TileServerOptions> TileServerOptions_maplibre() {
+  return std::make_unique<TileServerOptions>(TileServerOptions::MapboxConfiguration());
+}
+inline std::unique_ptr<TileServerOptions> TileServerOptions_maptiler() {
+  return std::make_unique<TileServerOptions>(TileServerOptions::MapTilerConfiguration());
+}
 
-// TileServerOptions Optional<string> helpers (not supported yet by cxx)
-const int8_t* TileServerOptions_sourceVersionPrefix(const TileServerOptions& self);
-const int8_t* TileServerOptions_styleVersionPrefix(const TileServerOptions& self);
-const int8_t* TileServerOptions_spritesVersionPrefix(const TileServerOptions& self);
-const int8_t* TileServerOptions_glyphsVersionPrefix(const TileServerOptions& self);
-const int8_t* TileServerOptions_tileVersionPrefix(const TileServerOptions& self);
+// TileServerOptions getters for optional string fields
+inline const int8_t * get_raw_str_ptr(const std::optional<std::string>& v) {
+    return (const int8_t *)(v.has_value() ? v->c_str() : nullptr);
+}
+inline const int8_t * TileServerOptions_sourceVersionPrefix(const TileServerOptions& self) {
+    return get_raw_str_ptr(self.sourceVersionPrefix());
+}
+inline const int8_t * TileServerOptions_styleVersionPrefix(const TileServerOptions& self) {
+    return get_raw_str_ptr(self.styleVersionPrefix());
+}
+inline const int8_t * TileServerOptions_spritesVersionPrefix(const TileServerOptions& self) {
+    return get_raw_str_ptr(self.spritesVersionPrefix());
+}
+inline const int8_t * TileServerOptions_glyphsVersionPrefix(const TileServerOptions& self) {
+    return get_raw_str_ptr(self.glyphsVersionPrefix());
+}
+inline const int8_t * TileServerOptions_tileVersionPrefix(const TileServerOptions& self) {
+    return get_raw_str_ptr(self.tileVersionPrefix());
+}
 
 } // namespace bridge
 } // namespace ml
diff --git a/platform/rust/src/lib.rs b/platform/rust/src/lib.rs
@@ -172,55 +172,61 @@ mod ffi {
 // Re-export native functions that do not need safety wrappers.
 pub use ffi::{ceil_log2, get_42, TileServerOptions};
 
-unsafe fn to_opt<'a>(value: *const c_char) -> Option<&'a CStr> {
-    value.as_ref().map(|v| CStr::from_ptr(v))
-}
-
 // Add more methods to make usage more ergonomic
 impl TileServerOptions {
+    /// Convert a pointer to a C string to an optional Rust &CStr with lifetime of &self.
+    ///
+    /// # Safety
+    /// The &CStr will remain valid as long as the TileServerOptions `&self` remains valid.
+    /// If some code tries to modify self, it will need to use &mut self,
+    /// which will not compile unless there are no &CStr references, as they use the same lifetime.
+    unsafe fn to_opt(&self, value: *const c_char) -> Option<&CStr> {
+        value.as_ref().map(|v| CStr::from_ptr(v))
+    }
+
     /// Create a new default configuration
     pub fn new() -> UniquePtr<TileServerOptions> {
-        crate::ffi::TileServerOptions_new()
+        ffi::TileServerOptions_new()
     }
 
     /// Get the tile server options configured for MapLibre.
     pub fn default_mapbox() -> UniquePtr<TileServerOptions> {
-        crate::ffi::TileServerOptions_mapbox()
+        ffi::TileServerOptions_mapbox()
     }
 
     /// Get the tile server options configured for Mapbox.
     pub fn default_maplibre() -> UniquePtr<TileServerOptions> {
-        crate::ffi::TileServerOptions_maplibre()
+        ffi::TileServerOptions_maplibre()
     }
 
     /// Get the tile server options configured for MapTiler.
     pub fn default_maptiler() -> UniquePtr<TileServerOptions> {
-        crate::ffi::TileServerOptions_maptiler()
+        ffi::TileServerOptions_maptiler()
     }
 
     /// Gets the source version prefix.
     pub fn source_version_prefix(&self) -> Option<&CStr> {
-        unsafe { to_opt(crate::ffi::TileServerOptions_sourceVersionPrefix(self)) }
+        unsafe { self.to_opt(ffi::TileServerOptions_sourceVersionPrefix(self)) }
     }
 
     /// Gets the style version prefix.
     pub fn style_version_prefix(&self) -> Option<&CStr> {
-        unsafe { to_opt(crate::ffi::TileServerOptions_styleVersionPrefix(self)) }
+        unsafe { self.to_opt(ffi::TileServerOptions_styleVersionPrefix(self)) }
     }
 
     /// Gets the sprites version prefix.
     pub fn sprites_version_prefix(&self) -> Option<&CStr> {
-        unsafe { to_opt(crate::ffi::TileServerOptions_spritesVersionPrefix(self)) }
+        unsafe { self.to_opt(ffi::TileServerOptions_spritesVersionPrefix(self)) }
     }
 
     /// Gets the glyphs version prefix.
     pub fn glyphs_version_prefix(&self) -> Option<&CStr> {
-        unsafe { to_opt(crate::ffi::TileServerOptions_glyphsVersionPrefix(self)) }
+        unsafe { self.to_opt(ffi::TileServerOptions_glyphsVersionPrefix(self)) }
     }
 
     /// Gets the tile version prefix.
     pub fn tile_version_prefix(&self) -> Option<&CStr> {
-        unsafe { to_opt(crate::ffi::TileServerOptions_tileVersionPrefix(self)) }
+        unsafe { self.to_opt(ffi::TileServerOptions_tileVersionPrefix(self)) }
     }
 }
 

diff --git a/platform/rust/src/wrapper.cc b/platform/rust/src/wrapper.cc
@@ -8,41 +8,5 @@ uint32_t get_42() {
     return 42;
 }
 
-std::unique_ptr<TileServerOptions> TileServerOptions_new() {
-  return std::make_unique<TileServerOptions>();
-}
-std::unique_ptr<TileServerOptions> TileServerOptions_mapbox() {
-  return std::make_unique<TileServerOptions>(TileServerOptions::MapLibreConfiguration());
-}
-std::unique_ptr<TileServerOptions> TileServerOptions_maplibre() {
-  return std::make_unique<TileServerOptions>(TileServerOptions::MapboxConfiguration());
-}
-std::unique_ptr<TileServerOptions> TileServerOptions_maptiler() {
-  return std::make_unique<TileServerOptions>(TileServerOptions::MapTilerConfiguration());
-}
-
-// FIXME: there should be a helper function to convert std::optional<std::string> to const int8_t *
-
-const int8_t * TileServerOptions_sourceVersionPrefix(const TileServerOptions& self) {
-    auto const &v = self.sourceVersionPrefix();
-    return (const int8_t *)(v.has_value() ? v->c_str() : nullptr);
-}
-const int8_t * TileServerOptions_styleVersionPrefix(const TileServerOptions& self) {
-    auto const &v = self.styleVersionPrefix();
-    return (const int8_t *)(v.has_value() ? v->c_str() : nullptr);
-}
-const int8_t * TileServerOptions_spritesVersionPrefix(const TileServerOptions& self) {
-    auto const &v = self.spritesVersionPrefix();
-    return (const int8_t *)(v.has_value() ? v->c_str() : nullptr);
-}
-const int8_t * TileServerOptions_glyphsVersionPrefix(const TileServerOptions& self) {
-    auto const &v = self.glyphsVersionPrefix();
-    return (const int8_t *)(v.has_value() ? v->c_str() : nullptr);
-}
-const int8_t * TileServerOptions_tileVersionPrefix(const TileServerOptions& self) {
-    auto const &v = self.tileVersionPrefix();
-    return (const int8_t *)(v.has_value() ? v->c_str() : nullptr);
-}
-
 } // namespace bridge
 } // namespace ml