Skip to content

Commit

Permalink
Support @rust.Ord structured annotation on unions
Browse files Browse the repository at this point in the history
Summary:
The Rust Thrift templates already correctly handle unions that need an Ord impl; it just turns into a `#[derive(Ord)]` on the generated data structure.

But unless the implementation of the `rust.Ord` annotation declares that it is compatible with unions, the Thrift compiler rejects `rust.Ord` on unions.

```lang=text,counterexample
[ERROR:dataswarm/services/if/operators.thrift:80] `Ord` cannot annotate `UnionOfAny`
```

Reviewed By: zertosh

Differential Revision: D62074164

fbshipit-source-id: a22908d1b54ba1b436aec7667bbceddc0acf712a
  • Loading branch information
David Tolnay authored and facebook-github-bot committed Sep 1, 2024
1 parent b8d5ece commit 1000892
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 34 deletions.
1 change: 1 addition & 0 deletions thrift/annotation/rust.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ struct Exhaustive {

@scope.Struct
@scope.Typedef
@scope.Union
struct Ord {
// # `rust.Ord`
//
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,10 @@ public static function getAllStructuredAnnotations()[write_props]: \TStructAnnot
shape(
)
),
'\facebook\thrift\annotation\Union' => \facebook\thrift\annotation\Union::fromShape(
shape(
)
),
],
'fields' => dict[
],
Expand Down
68 changes: 34 additions & 34 deletions thrift/compiler/test/fixtures/adapter/out/json/gen-json/rust.json
Original file line number Diff line number Diff line change
Expand Up @@ -142,31 +142,31 @@
"column" : 1
},
"end" : {
"line" : 87,
"line" : 88,
"column" : 2
}
}
},
"NewType" : {
"lineno" : 89,
"lineno" : 90,
"is_exception" : false,
"is_union" : false,
"fields" : {

},
"source_range" : {
"begin" : {
"line" : 89,
"line" : 90,
"column" : 1
},
"end" : {
"line" : 108,
"line" : 109,
"column" : 2
}
}
},
"Type" : {
"lineno" : 110,
"lineno" : 111,
"is_exception" : false,
"is_union" : false,
"fields" : {
Expand All @@ -176,29 +176,29 @@
"required" : true,
"source_range" : {
"begin" : {
"line" : 264,
"line" : 265,
"column" : 3
},
"end" : {
"line" : 264,
"line" : 265,
"column" : 18
}
}
}
},
"source_range" : {
"begin" : {
"line" : 110,
"line" : 111,
"column" : 1
},
"end" : {
"line" : 265,
"line" : 266,
"column" : 2
}
}
},
"Serde" : {
"lineno" : 271,
"lineno" : 272,
"is_exception" : false,
"is_union" : false,
"fields" : {
Expand All @@ -208,29 +208,29 @@
"required" : true,
"source_range" : {
"begin" : {
"line" : 273,
"line" : 274,
"column" : 3
},
"end" : {
"line" : 273,
"line" : 274,
"column" : 19
}
}
}
},
"source_range" : {
"begin" : {
"line" : 271,
"line" : 272,
"column" : 1
},
"end" : {
"line" : 274,
"line" : 275,
"column" : 2
}
}
},
"Mod" : {
"lineno" : 280,
"lineno" : 281,
"is_exception" : false,
"is_union" : false,
"fields" : {
Expand All @@ -240,29 +240,29 @@
"required" : true,
"source_range" : {
"begin" : {
"line" : 282,
"line" : 283,
"column" : 3
},
"end" : {
"line" : 282,
"line" : 283,
"column" : 18
}
}
}
},
"source_range" : {
"begin" : {
"line" : 280,
"line" : 281,
"column" : 1
},
"end" : {
"line" : 283,
"line" : 284,
"column" : 2
}
}
},
"Adapter" : {
"lineno" : 285,
"lineno" : 286,
"is_exception" : false,
"is_union" : false,
"fields" : {
Expand All @@ -272,29 +272,29 @@
"required" : true,
"source_range" : {
"begin" : {
"line" : 330,
"line" : 331,
"column" : 3
},
"end" : {
"line" : 330,
"line" : 331,
"column" : 18
}
}
}
},
"source_range" : {
"begin" : {
"line" : 285,
"line" : 286,
"column" : 1
},
"end" : {
"line" : 331,
"line" : 332,
"column" : 2
}
}
},
"Derive" : {
"lineno" : 333,
"lineno" : 334,
"is_exception" : false,
"is_union" : false,
"fields" : {
Expand All @@ -304,29 +304,29 @@
"required" : true,
"source_range" : {
"begin" : {
"line" : 356,
"line" : 357,
"column" : 3
},
"end" : {
"line" : 356,
"line" : 357,
"column" : 27
}
}
}
},
"source_range" : {
"begin" : {
"line" : 333,
"line" : 334,
"column" : 1
},
"end" : {
"line" : 357,
"line" : 358,
"column" : 2
}
}
},
"ServiceExn" : {
"lineno" : 359,
"lineno" : 360,
"is_exception" : false,
"is_union" : false,
"fields" : {
Expand All @@ -336,23 +336,23 @@
"required" : true,
"source_range" : {
"begin" : {
"line" : 410,
"line" : 411,
"column" : 3
},
"end" : {
"line" : 410,
"line" : 411,
"column" : 37
}
}
}
},
"source_range" : {
"begin" : {
"line" : 359,
"line" : 360,
"column" : 1
},
"end" : {
"line" : 411,
"line" : 412,
"column" : 2
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,10 @@ public static function getAllStructuredAnnotations()[write_props]: \TStructAnnot
shape(
)
),
'\facebook\thrift\annotation\Union' => \facebook\thrift\annotation\Union::fromShape(
shape(
)
),
],
'fields' => dict[
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ pub struct U10 {
pub enum U11 {
string(::std::string::String),
integer(::std::primitive::i32),
btreeset(::std::collections::BTreeSet<crate::types::U11>),
UnknownField(::std::primitive::i32),
}

Expand Down Expand Up @@ -2110,6 +2111,11 @@ where
::fbthrift::Serialize::write(inner, p);
p.write_field_end();
}
Self::btreeset(inner) => {
p.write_field_begin("btreeset", ::fbthrift::TType::Set, 3);
::fbthrift::Serialize::write(inner, p);
p.write_field_end();
}
Self::UnknownField(_) => {}
}
p.write_field_stop();
Expand All @@ -2124,6 +2130,7 @@ where
#[inline]
fn read(p: &mut P) -> ::anyhow::Result<Self> {
static FIELDS: &[::fbthrift::Field] = &[
::fbthrift::Field::new("btreeset", ::fbthrift::TType::Set, 3),
::fbthrift::Field::new("integer", ::fbthrift::TType::I32, 2),
::fbthrift::Field::new("str", ::fbthrift::TType::String, 1),
];
Expand All @@ -2142,6 +2149,10 @@ where
once = true;
alt = ::std::option::Option::Some(Self::integer(::fbthrift::Deserialize::read(p)?));
}
(::fbthrift::TType::Set, 3, false) => {
once = true;
alt = ::std::option::Option::Some(Self::btreeset(::fbthrift::Deserialize::read(p)?));
}
(fty, _, false) => p.skip(fty)?,
(badty, badid, true) => return ::std::result::Result::Err(::std::convert::From::from(::fbthrift::ProtocolError::UnwantedExtraUnionField(
"T11".to_string(),
Expand All @@ -2162,6 +2173,7 @@ impl U11 {
match self {
Self::string(_) => ::std::option::Option::Some(("string", "str")),
Self::integer(_) => ::std::option::Option::Some(("integer", "integer")),
Self::btreeset(_) => ::std::option::Option::Some(("btreeset", "btreeset")),
Self::UnknownField(_) => ::std::option::Option::None,
}
}
Expand All @@ -2182,6 +2194,15 @@ impl ::fbthrift::metadata::ThriftAnnotations for U11 {
return r.take();
}

if type_id == ::std::any::TypeId::of::<rust__types::Ord>() {
let mut tmp = ::std::option::Option::Some(rust__types::Ord {
..::std::default::Default::default()
});
let r: &mut dyn ::std::any::Any = &mut tmp;
let r: &mut ::std::option::Option<T> = r.downcast_mut().unwrap();
return r.take();
}

::std::option::Option::None
}

Expand All @@ -2205,6 +2226,8 @@ impl ::fbthrift::metadata::ThriftAnnotations for U11 {
},
2 => {
},
3 => {
},
_ => {}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,12 @@ struct T10 {
typedef i64 t_t1

@rust.Name{name = "U11"}
@rust.Ord
union T11 {
@rust.Name{name = "string"}
1: string str;
2: i32 integer;
3: set<T11> btreeset;
}

// --
Expand Down

0 comments on commit 1000892

Please sign in to comment.