From 626812ff966a58fc5f233bbe5d4cdacc32a8db4f Mon Sep 17 00:00:00 2001 From: Owen Avery Date: Tue, 16 Sep 2025 19:31:39 -0400 Subject: [PATCH] gccrs: Improve FFIOpt Also fixes https://github.com/Rust-GCC/gccrs/issues/4171. gcc/rust/ChangeLog: * ast/rust-fmt.h (class FFIOpt): Adjust internal structure to match a repr(C) rust enum. libgrust/ChangeLog: * libformat_parser/src/lib.rs (struct FFIOpt): Likewise and remove some now-redundant methods. Signed-off-by: Owen Avery --- gcc/rust/ast/rust-fmt.h | 77 +++++++++++++++++-------- libgrust/libformat_parser/src/lib.rs | 84 +++------------------------- 2 files changed, 63 insertions(+), 98 deletions(-) diff --git a/gcc/rust/ast/rust-fmt.h b/gcc/rust/ast/rust-fmt.h index c23499c3709..e59bed3e9d2 100644 --- a/gcc/rust/ast/rust-fmt.h +++ b/gcc/rust/ast/rust-fmt.h @@ -83,38 +83,41 @@ public: const T *end () const { return data + len; } }; -template class FFIOpt +// https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md +template ::value>::type> +class FFIOpt { - struct alignas (T) Inner - { - char data[sizeof (T)]; - } inner; - bool is_some; - public: - template FFIOpt (U &&val) : is_some (true) - { - new (inner.data) T (std::forward (val)); - } + template + FFIOpt (U &&val) : some{Some::KIND, std::forward (val)} + {} - FFIOpt () : is_some (false) {} + FFIOpt () : none{None::KIND} {} - FFIOpt (const FFIOpt &other) : is_some (other.is_some) + FFIOpt (const FFIOpt &other) { - if (is_some) - new (inner.data) T (*(const T *) other.inner.data); + if (other.has_value ()) + new (&some) Some{Some::KIND, other.some.val}; + else + new (&none) None{None::KIND}; } - FFIOpt (FFIOpt &&other) : is_some (other.is_some) + FFIOpt (FFIOpt &&other) { - if (is_some) - new (inner.data) T (std::move (*(const T *) other.inner.data)); + if (other.has_value ()) + new (&some) Some{Some::KIND, std::move (other.some.val)}; + else + new (&none) None{None::KIND}; } ~FFIOpt () { - if (is_some) - ((T *) inner.data)->~T (); + if (has_value ()) + some.~Some (); + else + none.~None (); } FFIOpt &operator= (const FFIOpt &other) @@ -133,13 +136,43 @@ public: tl::optional> get_opt () { - return (T *) inner.data; + if (has_value ()) + return std::ref (some.val); + else + return tl::nullopt; } tl::optional> get_opt () const { - return (const T *) inner.data; + if (has_value ()) + return std::ref (some.val); + else + return tl::nullopt; } + + bool has_value () const { return some.kind == Some::KIND; } + + operator bool () const { return has_value (); } + +private: + struct Some + { + static constexpr uint8_t KIND = 0; + uint8_t kind; + T val; + }; + + struct None + { + static constexpr uint8_t KIND = 1; + uint8_t kind; + }; + + union + { + Some some; + None none; + }; }; struct RustHamster diff --git a/libgrust/libformat_parser/src/lib.rs b/libgrust/libformat_parser/src/lib.rs index 049403db0b7..efb5d00f678 100644 --- a/libgrust/libformat_parser/src/lib.rs +++ b/libgrust/libformat_parser/src/lib.rs @@ -83,87 +83,19 @@ pub mod ffi { } } - #[repr(C)] - pub struct FFIOpt { - val: MaybeUninit, - is_some: bool - } - - impl Clone for FFIOpt - where - T: Clone - { - fn clone(&self) -> Self { - match self.get_opt_ref() { - Some(r) => FFIOpt::new_val(r.clone()), - None => FFIOpt::new_none() - } - } - } - - impl PartialEq for FFIOpt - where - T: PartialEq - { - fn eq(&self, other: &Self) -> bool { - match (self.get_opt_ref(), other.get_opt_ref()) { - (Some(a), Some(b)) => a.eq(b), - _ => false - } - } - } - - impl Eq for FFIOpt - where - T: Eq - {} - - impl Drop for FFIOpt - { - fn drop(&mut self) { - if self.is_some { - unsafe { std::ptr::drop_in_place(self.val.as_mut_ptr()) } - } - } + // https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md + #[repr(u8)] + #[derive(Copy, Clone, PartialEq, Eq)] + pub enum FFIOpt { + Some(T), + None } impl IntoFFI> for Option { fn into_ffi(self) -> FFIOpt { match self { - Some(v) => FFIOpt::new_val(v), - None => FFIOpt::new_none() - } - } - } - - impl FFIOpt { - pub fn new_val(v: T) -> Self { - FFIOpt { - val: MaybeUninit::new(v), - is_some: true - } - } - - pub fn new_none() -> Self { - FFIOpt { - val: MaybeUninit::uninit(), - is_some: false - } - } - - pub fn get_opt_ref(&self) -> Option<&T> { - if self.is_some { - Some(unsafe {&*self.val.as_ptr()}) - } else { - None - } - } - - pub fn get_opt_ref_mut(&mut self) -> Option<&mut T> { - if self.is_some { - Some(unsafe {&mut *self.val.as_mut_ptr()}) - } else { - None + Some(v) => FFIOpt::Some(v), + None => FFIOpt::None } } } -- 2.47.3