From 3b41a19a208733a4976ba3f147f8d4173c878075 Mon Sep 17 00:00:00 2001 From: Andrew Hilger Date: Thu, 5 Dec 2024 10:45:18 -0800 Subject: [PATCH] refactor constants template for recursion Summary: The current py3 constants setup heavily uses `types`, which is nice because much of the logic is type-specific. Unfortunately, it works poorly for recursive calls, as are needed for containers and struct migration to python. It ends up being less janky to add some logic to `mstch_py3` to expose the types where needed. This diff takes the current `templates/python/types/constant_value.mustache` and adapts it to py3, so we aren't yet migrating the constants and structs from cbindings to python. Reviewed By: createdbysk Differential Revision: D66724441 fbshipit-source-id: cb822c3b1686ca374183f0221390c34d0754c29e --- .../generate/t_mstch_py3_generator.cc | 79 +++++++++++++++ .../generate/templates/py3/types.pyx.mustache | 7 +- .../py3/types/constant_value.mustache | 95 ++++++++----------- 3 files changed, 122 insertions(+), 59 deletions(-) diff --git a/third-party/thrift/src/thrift/compiler/generate/t_mstch_py3_generator.cc b/third-party/thrift/src/thrift/compiler/generate/t_mstch_py3_generator.cc index b63a79eebd449a..a55acb972d01f2 100644 --- a/third-party/thrift/src/thrift/compiler/generate/t_mstch_py3_generator.cc +++ b/third-party/thrift/src/thrift/compiler/generate/t_mstch_py3_generator.cc @@ -1071,6 +1071,84 @@ class py3_mstch_enum_value : public mstch_enum_value { } }; +class py3_mstch_const_value : public mstch_const_value { + public: + py3_mstch_const_value( + const t_const_value* cv, + mstch_context& ctx, + mstch_element_position pos, + const t_const* current_const, + const t_type* expected_type) + : mstch_const_value(cv, ctx, pos, current_const, expected_type) { + register_cached_methods( + this, + { + {"value:value_for_bool?", &py3_mstch_const_value::is_bool_value}, + {"value:value_for_floating_point?", + &py3_mstch_const_value::is_float_value}, + {"value:py3_binary?", &py3_mstch_const_value::is_binary}, + {"value:const_enum_type", &py3_mstch_const_value::const_enum_type}, + {"value:py3_enum_value_name", + &py3_mstch_const_value::py3_enum_value_name}, + {"value:const_enum_type", &py3_mstch_const_value::const_enum_type}, + {"value:const_container_type", + &py3_mstch_const_value::const_container_type}, + }); + } + + mstch::node is_bool_value() { + if (auto ttype = const_value_->ttype()) { + return ttype->get_true_type()->is_bool(); + } + return false; + } + + mstch::node is_float_value() { + if (auto ttype = const_value_->ttype()) { + return ttype->get_true_type()->is_floating_point(); + } + return false; + } + + mstch::node is_binary() { + auto& ttype = const_value_->ttype(); + return type_ == cv::CV_STRING && ttype && + ttype->get_true_type()->is_binary(); + } + + mstch::node py3_enum_value_name() { + if (!const_value_->is_enum() || const_value_->get_enum_value() == nullptr) { + return mstch::node(); + } + const auto& enum_name = const_value_->get_enum()->get_name(); + return python::get_py3_name_class_scope( + *const_value_->get_enum_value(), enum_name); + } + + mstch::node const_enum_type() { + if (!const_value_->ttype() || type_ != cv::CV_INTEGER || + !const_value_->is_enum()) { + return {}; + } + const auto* type = const_value_->ttype()->get_true_type(); + if (type->is_enum()) { + return context_.type_factory->make_mstch_object(type, context_); + } + return {}; + } + + mstch::node const_container_type() { + if (!const_value_->ttype()) { + return {}; + } + const auto* type = const_value_->ttype()->get_true_type(); + if (type->is_container()) { + return context_.type_factory->make_mstch_object(type, context_); + } + return {}; + } +}; + class py3_mstch_deprecated_annotation : public mstch_deprecated_annotation { public: py3_mstch_deprecated_annotation( @@ -1348,6 +1426,7 @@ void t_mstch_py3_generator::set_mstch_factories() { mstch_context_.add(); mstch_context_.add(); mstch_context_.add(); + mstch_context_.add(); mstch_context_.add(); } diff --git a/third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache b/third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache index fb457a69734c25..dbeaf9422ed4fd 100644 --- a/third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache +++ b/third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache @@ -659,9 +659,10 @@ cdef object {{type:flat_name}}__from_cpp(const {{> types/cython_cpp_type}}& c_ma {{/program:containerTypes}} -{{#program:constants}} -{{#constant:value}}{{> types/constant_value}}{{/constant:value}}{{/program:constants}}{{! -}}{{#program:filtered_typedefs}} +{{#program:constants}}{{#constant:value}} +{{constant:name}} = {{> types/constant_value}} +{{/constant:value}}{{/program:constants}} +{{#program:filtered_typedefs}} {{typedef:name}} = {{#typedef:type}}{{> types/python_type}}{{/typedef:type}} {{/program:filtered_typedefs}} {{#program:has_stream?}} diff --git a/third-party/thrift/src/thrift/compiler/generate/templates/py3/types/constant_value.mustache b/third-party/thrift/src/thrift/compiler/generate/templates/py3/types/constant_value.mustache index ea5f7898d732ba..d1dd9b2012c7ea 100644 --- a/third-party/thrift/src/thrift/compiler/generate/templates/py3/types/constant_value.mustache +++ b/third-party/thrift/src/thrift/compiler/generate/templates/py3/types/constant_value.mustache @@ -16,66 +16,49 @@ }}{{! -Definitions for constant values. These are included from the struct.pyx file +Definitions for constant values. These are included from the types.pyx file and define Python objects that can be imported from the end-user's Python file. -}} -{{#constant:type}}{{! -}} -{{#type:bool?}} -{{constant:name}} = {{#constant:value}}{{! +}}{{#value:bool?}}{{! }}{{#value:nonzero?}}True{{/value:nonzero?}}{{! - }}{{^value:nonzero?}}False{{/value:nonzero?}}{{/constant:value}} -{{/type:bool?}} -{{#type:byte?}} -{{constant:name}} = {{#constant:value}}{{value:integer_value}}{{/constant:value}} -{{/type:byte?}} -{{#type:i16?}} -{{constant:name}} = {{#constant:value}}{{value:integer_value}}{{/constant:value}} -{{/type:i16?}} -{{#type:i32?}} -{{constant:name}} = {{#constant:value}}{{value:integer_value}}{{/constant:value}} -{{/type:i32?}} -{{#type:i64?}} -{{constant:name}} = {{#constant:value}}{{value:integer_value}}{{/constant:value}} -{{/type:i64?}} -{{#type:double?}} -{{constant:name}} = {{#constant:value}}{{! -}}{{#value:integer?}}{{value:integer_value}}.0{{/value:integer?}}{{! + }}{{^value:nonzero?}}False{{/value:nonzero?}}{{! +}}{{/value:bool?}}{{! +}}{{#value:integer?}}{{! + }}{{#value:value_for_bool?}}{{! + }}{{#value:nonzero?}}True{{/value:nonzero?}}{{! + }}{{^value:nonzero?}}False{{/value:nonzero?}}{{! + }}{{/value:value_for_bool?}}{{! + }}{{^value:value_for_bool?}}{{! + }}{{value:integer_value}}{{#value:value_for_floating_point?}}.0{{/value:value_for_floating_point?}}{{! + }}{{/value:value_for_bool?}}{{! +}}{{/value:integer?}}{{! }}{{#value:double?}}{{value:double_value}}{{/value:double?}}{{! -}}{{/constant:value}} -{{/type:double?}} -{{#type:float?}} -{{constant:name}} = {{#constant:value}}{{! -}}{{#value:integer?}}{{value:integer_value}}.0{{/value:integer?}}{{! -}}{{#value:double?}}{{value:double_value}}{{/value:double?}}{{! -}}{{/constant:value}} -{{/type:float?}} -{{#type:string?}} -{{constant:name}} = {{#constant:value}}"{{value:string_value}}"{{/constant:value}} -{{/type:string?}} -{{#type:binary?}} -{{constant:name}} = {{#constant:value}}b"{{value:string_value}}"{{/constant:value}} -{{/type:binary?}} -{{! We take the constant structs/containers and build a non-const shared_ptr - as required by the thrift-py3 wrapper api, generally this is a terrible - idea as constant() is an r-value, and removing the const qualifier is dangerous - but we *promise* in the thrift-py3 api to never change objects. -}} -{{#type:struct?}} -{{constant:name}} = {{> types/cython_python_type}}._create_FBTHRIFT_ONLY_DO_NOT_USE(constant_shared_ptr({{! - }}{{> types/current_module_cbindings}}.{{> types/c_constant }}())) -{{/type:struct?}} -{{#type:container?}} -{{constant:name}} = {{> types/container_from_cpp }}({{> types/current_module_cbindings}}.{{> types/c_constant }}()) -{{/type:container?}} -{{#type:enum}} -{{constant:name}} = {{#constant:value}}{{! +}}{{#value:const_struct?}}{{! + }}{{#value:const_struct_type}}{{! + }}{{> types/cython_python_type}}._create_FBTHRIFT_ONLY_DO_NOT_USE(constant_shared_ptr({{! + }}{{> types/current_module_cbindings}}.{{> types/c_constant }}())){{! + }}{{/value:const_struct_type}}{{! +}}{{/value:const_struct?}}{{! +}}{{^value:const_struct?}}{{! +}}{{#value:string?}}{{! + }}{{#value:py3_binary?}}b{{/value:py3_binary?}}"{{value:string_value}}"{{! +}}{{/value:string?}}{{! +}}{{#value:map?}}{{! + }}{{#value:const_container_type}}{{! + }}{{> types/container_from_cpp }}({{> types/current_module_cbindings}}.{{> types/c_constant }}()){{! + }}{{/value:const_container_type}}{{! +}}{{/value:map?}}{{! +}}{{#value:list?}}{{! + }}{{#value:const_container_type}}{{! + }}{{> types/container_from_cpp }}({{> types/current_module_cbindings}}.{{> types/c_constant }}()){{! + }}{{/value:const_container_type}}{{! +}}{{/value:list?}}{{! +}}{{/value:const_struct?}}{{! +}}{{#value:enum?}}{{! }}{{^value:enum_value?}}__BadEnum({{/value:enum_value?}}{{! - }}{{#type:need_module_path?}}{{type:modulePath}}.{{/type:need_module_path?}}{{! - }}{{value:enum_name}}{{#value:enum_value}}.{{enum_value:py_name}}{{/value:enum_value}}{{! + }}{{#value:const_enum_type}}{{#type:need_module_path?}}{{type:modulePath}}.{{! + }}{{/type:need_module_path?}}{{/value:const_enum_type}}{{! + }}{{value:enum_name}}{{#value:enum_value?}}.{{value:py3_enum_value_name}}{{/value:enum_value?}}{{! }}{{^value:enum_value?}}, {{value:integer_value}}){{/value:enum_value?}}{{! -}}{{/constant:value}} -{{/type:enum}} -{{/constant:type}} +}}{{/value:enum?}}