Skip to content

Commit b404117

Browse files
powerboat9CohenArthur
authored andcommitted
Improve FFIOpt
Also fixes #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 <[email protected]>
1 parent 9e1d673 commit b404117

File tree

2 files changed

+63
-98
lines changed

2 files changed

+63
-98
lines changed

gcc/rust/ast/rust-fmt.h

Lines changed: 55 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -83,38 +83,41 @@ template <typename T> class FFIVec
8383
const T *end () const { return data + len; }
8484
};
8585

86-
template <typename T> class FFIOpt
86+
// https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md
87+
template <typename T,
88+
typename =
89+
typename std::enable_if<std::is_standard_layout<T>::value>::type>
90+
class FFIOpt
8791
{
88-
struct alignas (T) Inner
89-
{
90-
char data[sizeof (T)];
91-
} inner;
92-
bool is_some;
93-
9492
public:
95-
template <typename U> FFIOpt (U &&val) : is_some (true)
96-
{
97-
new (inner.data) T (std::forward<U> (val));
98-
}
93+
template <typename U>
94+
FFIOpt (U &&val) : some{Some::KIND, std::forward<U> (val)}
95+
{}
9996

100-
FFIOpt () : is_some (false) {}
97+
FFIOpt () : none{None::KIND} {}
10198

102-
FFIOpt (const FFIOpt &other) : is_some (other.is_some)
99+
FFIOpt (const FFIOpt &other)
103100
{
104-
if (is_some)
105-
new (inner.data) T (*(const T *) other.inner.data);
101+
if (other.has_value ())
102+
new (&some) Some{Some::KIND, other.some.val};
103+
else
104+
new (&none) None{None::KIND};
106105
}
107106

108-
FFIOpt (FFIOpt &&other) : is_some (other.is_some)
107+
FFIOpt (FFIOpt &&other)
109108
{
110-
if (is_some)
111-
new (inner.data) T (std::move (*(const T *) other.inner.data));
109+
if (other.has_value ())
110+
new (&some) Some{Some::KIND, std::move (other.some.val)};
111+
else
112+
new (&none) None{None::KIND};
112113
}
113114

114115
~FFIOpt ()
115116
{
116-
if (is_some)
117-
((T *) inner.data)->~T ();
117+
if (has_value ())
118+
some.~Some ();
119+
else
120+
none.~None ();
118121
}
119122

120123
FFIOpt &operator= (const FFIOpt &other)
@@ -133,13 +136,43 @@ template <typename T> class FFIOpt
133136

134137
tl::optional<std::reference_wrapper<T>> get_opt ()
135138
{
136-
return (T *) inner.data;
139+
if (has_value ())
140+
return std::ref (some.val);
141+
else
142+
return tl::nullopt;
137143
}
138144

139145
tl::optional<std::reference_wrapper<const T>> get_opt () const
140146
{
141-
return (const T *) inner.data;
147+
if (has_value ())
148+
return std::ref (some.val);
149+
else
150+
return tl::nullopt;
142151
}
152+
153+
bool has_value () const { return some.kind == Some::KIND; }
154+
155+
operator bool () const { return has_value (); }
156+
157+
private:
158+
struct Some
159+
{
160+
static constexpr uint8_t KIND = 0;
161+
uint8_t kind;
162+
T val;
163+
};
164+
165+
struct None
166+
{
167+
static constexpr uint8_t KIND = 1;
168+
uint8_t kind;
169+
};
170+
171+
union
172+
{
173+
Some some;
174+
None none;
175+
};
143176
};
144177

145178
struct RustHamster

libgrust/libformat_parser/src/lib.rs

Lines changed: 8 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -83,87 +83,19 @@ pub mod ffi {
8383
}
8484
}
8585

86-
#[repr(C)]
87-
pub struct FFIOpt<T> {
88-
val: MaybeUninit<T>,
89-
is_some: bool
90-
}
91-
92-
impl<T> Clone for FFIOpt<T>
93-
where
94-
T: Clone
95-
{
96-
fn clone(&self) -> Self {
97-
match self.get_opt_ref() {
98-
Some(r) => FFIOpt::new_val(r.clone()),
99-
None => FFIOpt::new_none()
100-
}
101-
}
102-
}
103-
104-
impl<T> PartialEq for FFIOpt<T>
105-
where
106-
T: PartialEq
107-
{
108-
fn eq(&self, other: &Self) -> bool {
109-
match (self.get_opt_ref(), other.get_opt_ref()) {
110-
(Some(a), Some(b)) => a.eq(b),
111-
_ => false
112-
}
113-
}
114-
}
115-
116-
impl<T> Eq for FFIOpt<T>
117-
where
118-
T: Eq
119-
{}
120-
121-
impl<T> Drop for FFIOpt<T>
122-
{
123-
fn drop(&mut self) {
124-
if self.is_some {
125-
unsafe { std::ptr::drop_in_place(self.val.as_mut_ptr()) }
126-
}
127-
}
86+
// https://github.com/rust-lang/rfcs/blob/master/text/2195-really-tagged-unions.md
87+
#[repr(u8)]
88+
#[derive(Copy, Clone, PartialEq, Eq)]
89+
pub enum FFIOpt<T> {
90+
Some(T),
91+
None
12892
}
12993

13094
impl<T> IntoFFI<FFIOpt<T>> for Option<T> {
13195
fn into_ffi(self) -> FFIOpt<T> {
13296
match self {
133-
Some(v) => FFIOpt::new_val(v),
134-
None => FFIOpt::new_none()
135-
}
136-
}
137-
}
138-
139-
impl<T> FFIOpt<T> {
140-
pub fn new_val(v: T) -> Self {
141-
FFIOpt {
142-
val: MaybeUninit::new(v),
143-
is_some: true
144-
}
145-
}
146-
147-
pub fn new_none() -> Self {
148-
FFIOpt {
149-
val: MaybeUninit::uninit(),
150-
is_some: false
151-
}
152-
}
153-
154-
pub fn get_opt_ref(&self) -> Option<&T> {
155-
if self.is_some {
156-
Some(unsafe {&*self.val.as_ptr()})
157-
} else {
158-
None
159-
}
160-
}
161-
162-
pub fn get_opt_ref_mut(&mut self) -> Option<&mut T> {
163-
if self.is_some {
164-
Some(unsafe {&mut *self.val.as_mut_ptr()})
165-
} else {
166-
None
97+
Some(v) => FFIOpt::Some(v),
98+
None => FFIOpt::None
16799
}
168100
}
169101
}

0 commit comments

Comments
 (0)