Skip to content

Commit 4af5600

Browse files
Mizuchimeta-codesync[bot]
authored andcommitted
Rewrite float in structured annotations
Summary: Currently in metadata.thrift, float is always stored as double, but in schema.thrift we will convert it to float. This conversion created a small discrepancy between metadata.thrift and schema.thrift. This diff changed the structured annotations to have the same logic as schema.thrift. Without this change, equality comparison will fail for annotations that contains float. Reviewed By: sadroeck Differential Revision: D86642320 fbshipit-source-id: 65c95a6e1be0bcdc736f2408102044d3c879b244
1 parent 820294e commit 4af5600

7 files changed

Lines changed: 33 additions & 5 deletions

File tree

third-party/thrift/src/thrift/compiler/sema/sema.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,15 @@ void match_type_with_const_value(
270270
value->assign(t_const_value(*constant->value()));
271271
}
272272

273+
if (value && type->is_float() && value->kind() == t_const_value::CV_DOUBLE) {
274+
// Check whether double is in float range.
275+
// If it is out of range, it will fail the validation later.
276+
if (std::numeric_limits<float>::lowest() <= value->get_double() &&
277+
value->get_double() <= std::numeric_limits<float>::max()) {
278+
value->set_double(static_cast<float>(value->get_double()));
279+
}
280+
}
281+
273282
if (const t_list* list = type->try_as<t_list>()) {
274283
if (value->kind() == t_const_value::CV_LIST) {
275284
for (auto list_val : value->get_list()) {

third-party/thrift/src/thrift/compiler/test/fixtures/mcpp2-compare/out/cpp2/gen-cpp2/module_constants.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ namespace module_constants {
5050
}
5151

5252
/** Glean {"constant": "aFloat"} */
53-
constexpr float const aFloat_ = static_cast<float>(0.1);
53+
constexpr float const aFloat_ = static_cast<float>(0.10000000149011612);
5454
/** Glean {"constant": "aFloat"} */
5555
constexpr float aFloat() {
5656
return aFloat_;

third-party/thrift/src/thrift/compiler/test/fixtures/mcpp2-compare/out/py3/gen-py3/module/constants_FBTHRIFT_ONLY_DO_NOT_USE.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
a16BitInt = 12
7272
a32BitInt = 123
7373
a64BitInt = 1234
74-
aFloat = 0.1
74+
aFloat = 0.10000000149011612
7575
aDouble = 0.12
7676
aString = "Joe Doe"
7777
aList = List__bool((True, False, ))

third-party/thrift/src/thrift/compiler/test/fixtures/mcpp2-compare/out/py3/gen-py3/module/types.pyx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4822,7 +4822,7 @@ aByte = 1
48224822
a16BitInt = 12
48234823
a32BitInt = 123
48244824
a64BitInt = 1234
4825-
aFloat = 0.1
4825+
aFloat = 0.10000000149011612
48264826
aDouble = 0.12
48274827
aString = "Joe Doe"
48284828
aList = List__bool__from_cpp(_module_cbindings.caList())

third-party/thrift/src/thrift/compiler/test/json_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ TEST(JsonTest, to_json_t_const_value) {
6060
my_int = 1,
6161
my_string = "hello",
6262
my_double = 9.9,
63-
my_list = [0.1, -0.2],
63+
my_list = [0.125, -0.25],
6464
my_nested = Nested{
6565
nested_int = 0,
6666
nested_enum = MyEnum.SECOND,
@@ -81,7 +81,7 @@ TEST(JsonTest, to_json_t_const_value) {
8181
std::string to_json_result = to_json(annotations.at(0).value());
8282
EXPECT_EQ(
8383
"{\"my_bool\": true, \"my_int\": 1, \"my_string\": \"hello\", "
84-
"\"my_double\": 9.9, \"my_list\": [0.1, -0.2], "
84+
"\"my_double\": 9.9, \"my_list\": [0.125, -0.25], "
8585
"\"my_nested\": {\"nested_int\": 0, \"nested_enum\": 2}}",
8686
to_json_result);
8787
}

third-party/thrift/src/thrift/lib/cpp2/test/metadata/annotations.thrift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,13 @@ service TestService {
146146
1: TestException ex,
147147
);
148148
}
149+
150+
@Annotation{floatField = 1.0}
151+
struct TestFloat1 {}
152+
153+
// Delta < (1 / (2^26)), which can't be stored in float that only has 24 bits precision.
154+
@Annotation{floatField = 1.00000001}
155+
struct TestFloat2 {}
156+
157+
@Annotation{floatField = 1.0000001}
158+
struct TestFloat3 {}

third-party/thrift/src/thrift/lib/cpp2/test/metadata/annotations_test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,15 @@ TEST(Annotations, Normalization) {
185185
}
186186
}
187187

188+
TEST(Annotations, TestFloat) {
189+
metadata::ThriftMetadata md1, md2, md3;
190+
const auto& t1 = detail::md::StructMetadata<TestFloat1>::gen(md1);
191+
const auto& t2 = detail::md::StructMetadata<TestFloat2>::gen(md2);
192+
const auto& t3 = detail::md::StructMetadata<TestFloat3>::gen(md3);
193+
EXPECT_EQ(t1.structured_annotations(), t2.structured_annotations());
194+
EXPECT_NE(t1.structured_annotations(), t3.structured_annotations());
195+
}
196+
188197
metadata::ThriftStruct expectedStruct() {
189198
metadata::ThriftStruct ret;
190199
ret.name() = "annotations.TestStruct";

0 commit comments

Comments
 (0)