Skip to content

Commit

Permalink
refactor constants template for recursion
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@ahilger @facebook-github-bot
ahilger authored and facebook-github-bot committed Dec 5, 2024
1 parent ea9aa37 commit 3b41a19
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 59 deletions.
79 changes: 79 additions & 0 deletions third-party/thrift/src/thrift/compiler/generate/t_mstch_py3_generator.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -1348,6 +1426,7 @@ void t_mstch_py3_generator::set_mstch_factories() {
mstch_context_.add<py3_mstch_field>();
mstch_context_.add<py3_mstch_enum>();
mstch_context_.add<py3_mstch_enum_value>();
mstch_context_.add<py3_mstch_const_value>();
mstch_context_.add<py3_mstch_deprecated_annotation>();
}

Expand Down
7 changes: 4 additions & 3 deletions third-party/thrift/src/thrift/compiler/generate/templates/py3/types.pyx.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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?}}
Expand Down
95 changes: 39 additions & 56 deletions third-party/thrift/src/thrift/compiler/generate/templates/py3/types/constant_value.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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?}}

0 comments on commit 3b41a19

Please sign in to comment.