From 6804932e0902853047f9ca79290935afe77e7f67 Mon Sep 17 00:00:00 2001 From: Yedidya Feldblum Date: Thu, 12 Jan 2017 22:46:19 -0800 Subject: [PATCH] Support indirection in C++ static reflection Summary: [Thrift] Support indirection in C++ static reflection. Indirection is done with a type alias with special annotations: ``` typedef i32 (cpp.type = 'CppTypeWrappingI32', cpp.indirection = '.chain().of.member().accesses') WrappedI32 struct HasOneIntegralField { 1: WrappedI32 integral_field, } ``` Previously, `reflected_struct_data_member::getter` had no way to see the wrapped object. And code using it had no way to know, supposing the field type is a wrapper, how to access the wrapped object. Here, we change the `getter` to see through any wrappers to access the wrapped object directly. However, this leaves reflection without any way to access the wrapper object. Tradeoffs. Reviewed By: juchem Differential Revision: D4406389 fbshipit-source-id: 0904e9fdefb55f1e0c1a48899d76c1d76659b8c8 --- .../compiler/py/generate/t_cpp_generator.py | 40 ++++++++- .../gen-cpp2/module_fatal_struct.h | 38 ++++++++- .../fatal/gen-cpp2/module_fatal_struct.h | 38 ++++++++- .../cpp2/fatal/internal/reflection-inl-post.h | 14 ++++ thrift/lib/cpp2/fatal/reflection.h | 1 + .../test/fatal_reflection_indirection.thrift | 15 ++++ .../fatal_reflection_indirection_test.cpp | 83 +++++++++++++++++++ .../test/fatal_reflection_indirection_types.h | 64 ++++++++++++++ 8 files changed, 287 insertions(+), 6 deletions(-) create mode 100644 thrift/test/fatal_reflection_indirection.thrift create mode 100644 thrift/test/fatal_reflection_indirection_test.cpp create mode 100644 thrift/test/fatal_reflection_indirection_types.h diff --git a/thrift/compiler/py/generate/t_cpp_generator.py b/thrift/compiler/py/generate/t_cpp_generator.py index b7696e03135..8f2a9d10500 100644 --- a/thrift/compiler/py/generate/t_cpp_generator.py +++ b/thrift/compiler/py/generate/t_cpp_generator.py @@ -5148,6 +5148,8 @@ def _generate_fatal_struct_impl(self, sns, program): safe_ns = self._get_namespace().replace('.', '_') dtmclsprefix = '{0}_{1}__struct_unique_data_member_getters_list'.format( safe_ns, name) + indclsprefix = '{0}_{1}__struct_unique_indirections_list'.format( + safe_ns, name) mpdclsprefix = '{0}_{1}__struct_unique_member_pod_list'.format( safe_ns, name) annclsprefix = '{0}_{1}__struct_unique_annotations'.format( @@ -5187,6 +5189,26 @@ def _generate_fatal_struct_impl(self, sns, program): with detail.cls('struct {0}'.format(dtmclsprefix)).scope as dtm: for i in members: dtm('FATAL_DATA_MEMBER_GETTER({0}, {0});'.format(i)) + for i in structs: + if i.is_union: + continue + if any(self._type_access_suffix(m.type) for m in i.members): + with detail.cls('struct {0}_{1}'.format( + i.name, indclsprefix)).scope as ind: + for m in i.members: + note = self._type_access_suffix(m.type) + if not note: + continue + with ind.cls('struct {0}'.format( + m.name)).scope as ind: + out('template \n' + 'static auto val(T{0} &&{0}) {{\n' + ' return std::forward({0}){1};\n' + '}}'.format('__thrift__arg__', note)) + out('template \n' + 'static auto &&ref(T{0} &&{0}) {{\n' + ' return std::forward({0}){1};\n' + '}}'.format('__thrift__arg__', note)) with detail.cls('struct {0}'.format(mpdclsprefix)).scope as mpd: pod_arg = 'T_{0}_{1}_struct_member_pod'.format(safe_ns, name) for i in members: @@ -5231,8 +5253,22 @@ def _generate_fatal_struct_impl(self, sns, program): cmnf(' {0},'.format(m.key)) cmnf(' ::apache::thrift::optionality::{0},'.format( self._render_fatal_required_qualifier(m.req))) - cmnf(' {0}::{1}::{2},'.format( - self.fatal_detail_ns, dtmclsprefix, m.name)) + if self._type_access_suffix(m.type): + cmnf(' ::fatal::chained_data_member_getter<') + cmnf(' {0}::{1}::{2},'.format( + self.fatal_detail_ns, dtmclsprefix, m.name)) + cmnf(' ::apache::thrift::detail::' + 'reflection_indirection_getter<') + cmnf(' {0}::{1}_{2}::{3}'.format( + self.fatal_detail_ns, + i.name, + indclsprefix, + m.name)) + cmnf(' >') + cmnf(' >,') + else: + cmnf(' {0}::{1}::{2},'.format( + self.fatal_detail_ns, dtmclsprefix, m.name)) cmnf(' {0},'.format( self._render_fatal_type_class(m.type))) cmnf(' {0}::{1}::{2}_{3}_struct_member_pod_{4},' diff --git a/thrift/compiler/test/fixtures/fatal-compat/gen-cpp2/module_fatal_struct.h b/thrift/compiler/test/fixtures/fatal-compat/gen-cpp2/module_fatal_struct.h index d448e325749..5c8d0053a95 100644 --- a/thrift/compiler/test/fixtures/fatal-compat/gen-cpp2/module_fatal_struct.h +++ b/thrift/compiler/test/fixtures/fatal-compat/gen-cpp2/module_fatal_struct.h @@ -106,6 +106,30 @@ struct test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list { FATAL_DATA_MEMBER_GETTER(result, result); }; +struct struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_indirections_list { + struct number { + template + static auto val(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).number; + } + template + static auto &&ref(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).number; + } + }; + + struct result { + template + static auto val(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).foo().result(); + } + template + static auto &&ref(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).foo().result(); + } + }; +}; + struct test_cpp2_cpp_reflection_module__struct_unique_member_pod_list { template @@ -3386,7 +3410,12 @@ struct struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_m ::test_cpp2::cpp_reflection::HasANumber, 3, ::apache::thrift::optionality::required_of_writer, - thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::number, + ::fatal::chained_data_member_getter< + thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::number, + ::apache::thrift::reflection_indirection_getter< + thrift_fatal_impl_detail::struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_indirections_list::number + > + >, ::apache::thrift::type_class::integral, thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_member_pod_list::test_cpp2_cpp_reflection_module_struct_member_pod_number, ::apache::thrift::reflected_annotations, @@ -3398,7 +3427,12 @@ struct struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_m ::test_cpp2::cpp_reflection::HasAResult, 4, ::apache::thrift::optionality::required_of_writer, - thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::result, + ::fatal::chained_data_member_getter< + thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::result, + ::apache::thrift::reflection_indirection_getter< + thrift_fatal_impl_detail::struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_indirections_list::result + > + >, ::apache::thrift::type_class::integral, thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_member_pod_list::test_cpp2_cpp_reflection_module_struct_member_pod_result, ::apache::thrift::reflected_annotations, diff --git a/thrift/compiler/test/fixtures/fatal/gen-cpp2/module_fatal_struct.h b/thrift/compiler/test/fixtures/fatal/gen-cpp2/module_fatal_struct.h index 92988cd8990..5c66c215b7f 100644 --- a/thrift/compiler/test/fixtures/fatal/gen-cpp2/module_fatal_struct.h +++ b/thrift/compiler/test/fixtures/fatal/gen-cpp2/module_fatal_struct.h @@ -111,6 +111,30 @@ struct test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list { FATAL_DATA_MEMBER_GETTER(result, result); }; +struct struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_indirections_list { + struct number { + template + static auto val(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).number; + } + template + static auto &&ref(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).number; + } + }; + + struct result { + template + static auto val(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).foo().result(); + } + template + static auto &&ref(T__thrift__arg__ &&__thrift__arg__) { + return std::forward(__thrift__arg__).foo().result(); + } + }; +}; + struct test_cpp2_cpp_reflection_module__struct_unique_member_pod_list { template @@ -3535,7 +3559,12 @@ struct struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_m ::test_cpp2::cpp_reflection::HasANumber, 3, ::apache::thrift::optionality::required_of_writer, - thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::number, + ::fatal::chained_data_member_getter< + thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::number, + ::apache::thrift::reflection_indirection_getter< + thrift_fatal_impl_detail::struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_indirections_list::number + > + >, ::apache::thrift::type_class::integral, thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_member_pod_list::test_cpp2_cpp_reflection_module_struct_member_pod_number, ::apache::thrift::reflected_annotations, @@ -3547,7 +3576,12 @@ struct struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_m ::test_cpp2::cpp_reflection::HasAResult, 4, ::apache::thrift::optionality::required_of_writer, - thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::result, + ::fatal::chained_data_member_getter< + thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_data_member_getters_list::result, + ::apache::thrift::reflection_indirection_getter< + thrift_fatal_impl_detail::struct_with_indirections_test_cpp2_cpp_reflection_module__struct_unique_indirections_list::result + > + >, ::apache::thrift::type_class::integral, thrift_fatal_impl_detail::test_cpp2_cpp_reflection_module__struct_unique_member_pod_list::test_cpp2_cpp_reflection_module_struct_member_pod_result, ::apache::thrift::reflected_annotations, diff --git a/thrift/lib/cpp2/fatal/internal/reflection-inl-post.h b/thrift/lib/cpp2/fatal/internal/reflection-inl-post.h index f2a557ba548..0f26604b8dd 100644 --- a/thrift/lib/cpp2/fatal/internal/reflection-inl-post.h +++ b/thrift/lib/cpp2/fatal/internal/reflection-inl-post.h @@ -53,6 +53,20 @@ struct mark_set { } // reflection_impl +template +struct reflection_indirection_getter { + template + using type = decltype(Impl::val(std::declval())); + + template + using reference = decltype(Impl::ref(std::declval())); + + template + static auto &&ref(T &&arg) { + return Impl::ref(std::forward(arg)); + } +}; + template struct type_common_metadata_impl { using module = Module; diff --git a/thrift/lib/cpp2/fatal/reflection.h b/thrift/lib/cpp2/fatal/reflection.h index b7c2abaebf1..393cab1086d 100644 --- a/thrift/lib/cpp2/fatal/reflection.h +++ b/thrift/lib/cpp2/fatal/reflection.h @@ -31,6 +31,7 @@ #include #include +#include #include #include diff --git a/thrift/test/fatal_reflection_indirection.thrift b/thrift/test/fatal_reflection_indirection.thrift new file mode 100644 index 00000000000..938219f8c39 --- /dev/null +++ b/thrift/test/fatal_reflection_indirection.thrift @@ -0,0 +1,15 @@ +cpp_include "thrift/test/fatal_reflection_indirection_types.h" + +namespace cpp2 reflection_indirection + +typedef i32 (cpp.type = 'CppFakeI32') FakeI32 +typedef i32 (cpp.type = 'CppHasANumber', cpp.indirection = '.number') HasANumber +typedef i32 (cpp.type = 'CppHasAResult', cpp.indirection = '.foo().result()') + HasAResult + +struct struct_with_indirections { + 1: i32 real, + 2: FakeI32 fake, + 3: HasANumber number, + 4: HasAResult result, +} diff --git a/thrift/test/fatal_reflection_indirection_test.cpp b/thrift/test/fatal_reflection_indirection_test.cpp new file mode 100644 index 00000000000..fe1191a76c9 --- /dev/null +++ b/thrift/test/fatal_reflection_indirection_test.cpp @@ -0,0 +1,83 @@ +/* + * Copyright 2017 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "thrift/test/gen-cpp2/fatal_reflection_indirection_fatal.h" + +#include + +#include +#include +#include + +namespace { + +class FatalReflectionIndirectionTest : public testing::Test {}; +} + +using type = reflection_indirection::struct_with_indirections; +using info = apache::thrift::reflect_struct; + +TEST_F(FatalReflectionIndirectionTest, sanity_check_no_indirection) { + type obj; + obj.real = 12; + EXPECT_EQ(12, obj.real); +} + +TEST_F(FatalReflectionIndirectionTest, simple_alias_no_indirection) { + type obj; + obj.fake = 15; + EXPECT_EQ(15, obj.fake); +} + +TEST_F(FatalReflectionIndirectionTest, indirection_via_single_member_field) { + using getter = info::member::number::getter; + EXPECT_SAME< + std::int32_t &, + decltype(getter::ref(std::declval())) + >(); + EXPECT_SAME< + std::int32_t &&, + decltype(getter::ref(std::declval())) + >(); + EXPECT_SAME< + std::int32_t const&, + decltype(getter::ref(std::declval())) + >(); + + type obj; + obj.number.number = -43; + EXPECT_EQ(-43, getter::ref(obj)); +} + +TEST_F(FatalReflectionIndirectionTest, indirection_via_chained_member_funcs) { + using getter = info::member::result::getter; + EXPECT_SAME< + std::int32_t &, + decltype(getter::ref(std::declval())) + >(); + EXPECT_SAME< + std::int32_t &&, + decltype(getter::ref(std::declval())) + >(); + EXPECT_SAME< + std::int32_t const&, + decltype(getter::ref(std::declval())) + >(); + + type obj; + obj.result.foo().result() = -2; + EXPECT_EQ(-2, getter::ref(obj)); +} diff --git a/thrift/test/fatal_reflection_indirection_types.h b/thrift/test/fatal_reflection_indirection_types.h new file mode 100644 index 00000000000..ce27272975a --- /dev/null +++ b/thrift/test/fatal_reflection_indirection_types.h @@ -0,0 +1,64 @@ +/* + * Copyright 2017 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +namespace reflection_indirection { + +using CppFakeI32 = std::int32_t; + +struct CppHasANumber { + std::int32_t number{}; + CppHasANumber() {} + explicit CppHasANumber(std::int32_t number_) : number(number_) {} + bool operator==(CppHasANumber that) const { return number == that.number; } + bool operator!=(CppHasANumber that) const { return number != that.number; } +}; + +class CppHasAResult { + public: + class Foo { + public: + explicit Foo(std::int32_t& obj) : obj_(obj) {} + auto& result() & { return obj_; } + auto&& result() && { return std::move(obj_); } + auto const& result() const& { return obj_; } + private: + std::int32_t& obj_; + }; + + CppHasAResult() {} + explicit CppHasAResult(std::int32_t result) : result_(result) {} + CppHasAResult(CppHasAResult const& that) : result_(that.result_) {} + CppHasAResult& operator=(CppHasAResult const& that) { + this->~CppHasAResult(); + return *::new (this) CppHasAResult(that); + } + + bool operator==(CppHasAResult that) const { return result_ == that.result_; } + bool operator!=(CppHasAResult that) const { return result_ != that.result_; } + + Foo& foo() & { return foo_; } + Foo&& foo() && { return static_cast(foo_); } + Foo const& foo() const& { return foo_; } + + private: + std::int32_t result_{}; + Foo foo_{result_}; +}; +}