Skip to content

Commit 206f87b

Browse files
hchokshimeta-codesync[bot]
authored andcommitted
Replace type module path properties with program-based accessors
Summary: Remove `modulePath`, `module_path_period_separated`, `cbinding_path`, `capi_converter_path`, and `module_auto_migrate_path` properties from the py3 generator's `t_type` prototype. These properties all resolved the true type's program internally and formatted a module path string. Replace all template usages with the equivalent expression using the program's `module_path_for_alias` property accessed via `type.true_type.program.module_path_for_alias` with the appropriate suffix (`_types`, `_cbindings`, `_thrift_converter`, `_thrift_types`). Reviewed By: iahs Differential Revision: D96012491 fbshipit-source-id: 1e4a71c5eeb252101ec0458b1fac911d30d2df56
1 parent 1534fff commit 206f87b

14 files changed

Lines changed: 49 additions & 98 deletions

third-party/thrift/src/thrift/compiler/generate/t_mstch_py3_generator.cc

Lines changed: 9 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -862,9 +862,6 @@ class t_mstch_py3_generator : public t_whisker_generator {
862862
return fmt::format(
863863
"{}", fmt::join(cpp2::get_gen_namespace_components(self), "::"));
864864
});
865-
def.property("py3Namespaces", [](const t_program& self) {
866-
return to_whisker_string_array(get_py3_namespace(&self));
867-
});
868865
def.property("module_path", [](const t_program& self) {
869866
std::vector<std::string> segments = get_py3_namespace_with_name(&self);
870867
assert(!segments.empty());
@@ -1043,6 +1040,15 @@ class t_mstch_py3_generator : public t_whisker_generator {
10431040
auto base = t_whisker_generator::make_prototype_for_type(proto);
10441041
auto def = whisker::dsl::prototype_builder<h_type>::extends(base);
10451042

1043+
// Override for base Whisker generator's `program` property, falling back to
1044+
// the current program for primitives/containers which have a `nullptr`
1045+
// program. This allows us to use the `t_program` module path properties
1046+
// without a mess of conditions in the template.
1047+
def.property("program", [&](const t_type& self) {
1048+
return proto.create<t_program>(
1049+
self.program() == nullptr ? *program_ : *self.program());
1050+
});
1051+
10461052
// Overrides for `t_named` properties, resolving typedefs before computing
10471053
// the value. This is necessary because the py3 generator previously used to
10481054
// erase typedefs when initializing mstch_type, but still used the
@@ -1060,22 +1066,6 @@ class t_mstch_py3_generator : public t_whisker_generator {
10601066
def.property("cpp_name", [](const t_type& self) {
10611067
return cpp2::get_name(self.get_true_type());
10621068
});
1063-
def.property("modulePath", [this](const t_type& self) {
1064-
const t_type* true_type = self.get_true_type();
1065-
const t_program* program = true_type->program() == nullptr
1066-
? get_program()
1067-
: true_type->program();
1068-
return fmt::format(
1069-
"_{}_types", fmt::join(get_py3_namespace_with_name(program), "_"));
1070-
});
1071-
def.property("module_path_period_separated", [this](const t_type& self) {
1072-
const t_type* true_type = self.get_true_type();
1073-
const t_program* program = true_type->program() == nullptr
1074-
? get_program()
1075-
: true_type->program();
1076-
return fmt::format(
1077-
"{}.types", fmt::join(get_py3_namespace_with_name(program), "."));
1078-
});
10791069
def.property("need_module_path?", [this](const t_type& self) {
10801070
return file_type_ == FileType::NotTypesFile ||
10811071
get_true_type_program(self) != get_program();
@@ -1095,24 +1085,6 @@ class t_mstch_py3_generator : public t_whisker_generator {
10951085
return file_type_ != FileType::CBindingsFile ||
10961086
get_true_type_program(self) != get_program();
10971087
});
1098-
def.property("cbinding_path", [this](const t_type& self) {
1099-
return fmt::format(
1100-
"_{}_cbindings",
1101-
fmt::join(
1102-
get_py3_namespace_with_name(get_true_type_program(self)), "_"));
1103-
});
1104-
def.property("capi_converter_path", [this](const t_type& self) {
1105-
return fmt::format(
1106-
"_{}_thrift_converter",
1107-
fmt::join(
1108-
get_py3_namespace_with_name(get_true_type_program(self)), "_"));
1109-
});
1110-
def.property("module_auto_migrate_path", [this](const t_type& self) {
1111-
return fmt::format(
1112-
"_{}_thrift_types",
1113-
fmt::join(
1114-
get_py3_namespace_with_name(get_true_type_program(self)), "_"));
1115-
});
11161088
def.property("cppTemplate", [this](const t_type& self) {
11171089
return context_->get_cached_type_props(&self).cpp_template();
11181090
});

third-party/thrift/src/thrift/compiler/generate/templates/py3/builders.py.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {{include.program.module_path}}.types as {{include.program.module_path_fo
4343
{{/if include.program.has_types?}}
4444
{{/each}}
4545

46-
import {{program.module_path}}.{{> common/auto_migrate_import_prefix }}types as {{program.module_path_for_alias}}_types
46+
import {{program.module_path}}.{{#if program.auto_migrate?}}thrift_{{/if}}types as {{program.module_path_for_alias}}_types
4747

4848

4949
{{#each program.py3_visible_structured_definitions as |struct|}}

third-party/thrift/src/thrift/compiler/generate/templates/py3/clients/callback_body.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ if {{#each function.exceptions as |exc|}}{{!
2121
try:
2222
exc = {{#if root_program.inplace_migrate?}}{{!
2323
}}{{#partial types.python_type type=exc.type}}.from_python({{!
24-
}}{{exc.type.capi_converter_path}}.{{exc.type.true_type.name}}_from_cpp(
24+
}}{{exc.type.true_type.program.module_path_for_alias}}_thrift_converter.{{exc.type.true_type.name}}_from_cpp(
2525
unwrap_exception[{{#partial types.cython_cpp_type type=exc.type}}](result.exception())
2626
))
2727
{{#else}}

third-party/thrift/src/thrift/compiler/generate/templates/py3/clients/cython_cpp_value_to_return_value.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ to a python type.
3535

3636
{{#else if (and function.return_type.structured? program.inplace_migrate?)}}
3737
{{#partial types.python_type type=function.return_type}}.from_python(
38-
{{function.return_type.capi_converter_path}}.{{function.return_type.true_type.name}}_from_cpp(
38+
{{function.return_type.true_type.program.module_path_for_alias}}_thrift_converter.{{function.return_type.true_type.name}}_from_cpp(
3939
__deref_const[{{#partial types.cython_cpp_type type=function.return_type}}](result.value())))
4040

4141
{{#else if function.return_type.structured?}}

third-party/thrift/src/thrift/compiler/generate/templates/py3/clients/cython_python_to_cpp_arg.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ This template assumes that the Python object is in a variable named <field>.
3434
{{#else if param.type.container?}}{{#partial types.container_to_cpp type=param.type}}({{param.py_name}})
3535
{{#else if param.type.enum?}}<{{#partial types.cython_cpp_type type=param.type}}><int>{{param.py_name}}
3636
{{#else if (and param.type.structured? program.inplace_migrate?)}}
37-
{{param.type.capi_converter_path}}.{{param.type.true_type.name}}_convert_to_cpp({{param.py_name}}._to_python())
37+
{{param.type.true_type.program.module_path_for_alias}}_thrift_converter.{{param.type.true_type.name}}_convert_to_cpp({{param.py_name}}._to_python())
3838

3939
{{#else if param.type.structured?}}
4040
deref((<{{#partial types.cython_python_type type=param.type}}>{{param.py_name}}).{{#partial types.cpp_obj}})

third-party/thrift/src/thrift/compiler/generate/templates/py3/common/auto_migrate_import_prefix.mustache

Lines changed: 0 additions & 21 deletions
This file was deleted.

third-party/thrift/src/thrift/compiler/generate/templates/py3/common/constants.mustache

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ file.
3737
3838
{{#else if value.enum?}}
3939
{{#if (not value.enum_value?)}}__BadEnum({{/if}}
40-
{{#if (and ttype.enum? ttype.need_module_path?)}}{{ttype.module_path_period_separated}}.{{/if}}
40+
{{#if (and ttype.enum? ttype.need_module_path?)}}{{ttype.program.module_path}}.types.{{/if}}
4141
{{value.enum_name}}{{#if value.enum_value?}}.{{value.py3_enum_value_name}}{{/if}}
4242
{{#if (not value.enum_value?)}}, {{value.integer_value}}){{/if}}
4343
4444
{{#else if value.structured?}}
45-
{{#if ttype.need_module_path?}}{{ttype.module_path_period_separated}}.{{/if}}{{ttype.name}}
45+
{{#if ttype.need_module_path?}}{{ttype.program.module_path}}.types.{{/if}}{{ttype.name}}
4646
(
4747
{{#each (array.enumerate value.structured_elements with_last=true) as |i elem last?|}}
4848
{{elem.field.name}}={{#partial inplace_const_value value=elem.value}}{{#if (not last?)}}, {{/if}}
@@ -90,9 +90,9 @@ file.
9090
{{#else if value.enum?}}
9191
{{#let enum_type=value.type.true_type}}
9292
{{#if value.enum_value?}}
93-
{{#if enum_type.need_module_path?}}{{enum_type.modulePath}}.{{/if}}{{value.enum_name}}.{{value.py3_enum_value_name}}
93+
{{#if enum_type.need_module_path?}}{{enum_type.program.module_path_for_alias}}_types.{{/if}}{{value.enum_name}}.{{value.py3_enum_value_name}}
9494
{{#else}}
95-
__BadEnum({{#if enum_type.need_module_path?}}{{enum_type.modulePath}}.{{/if}}{{value.enum_name}}, {{value.integer_value}})
95+
__BadEnum({{#if enum_type.need_module_path?}}{{enum_type.program.module_path_for_alias}}_types.{{/if}}{{value.enum_name}}, {{value.integer_value}})
9696
{{/if value.enum_value?}}
9797
{{/if value.bool?}}
9898
{{/let partial}}

third-party/thrift/src/thrift/compiler/generate/templates/py3/common/pep484.mustache

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ thrift-py3 specific container types.
3434
{{#else if type.string?}}str
3535
{{#else if type.void?}}None
3636
{{#else if type.binary?}}{{#if type.iobufWrapper?}}_fbthrift_iobuf.IOBuf{{#else}}bytes{{/if}}
37-
{{#else if (or type.structured? type.enum?)}}{{#if type.need_module_path?}}{{type.modulePath}}.{{/if}}{{type.true_type.name}}
37+
{{#else if (or type.structured? type.enum?)}}{{#if type.need_module_path?}}{{type.true_type.program.module_path_for_alias}}_types.{{/if}}{{type.true_type.name}}
3838
{{#else if type.list?}}_typing.Sequence[{{#partial type_name type=type.true_type.elem_type}}]
3939
{{#else if type.set?}}_typing.AbstractSet[{{#partial type_name type=type.true_type.elem_type}}]
4040
{{#else if type.map?}}_typing.Mapping[{{#partial type_name type=type.true_type.key_type}}, {{#partial type_name type=type.true_type.val_type}}]
@@ -45,7 +45,7 @@ thrift-py3 specific container types.
4545
{{! pep 484 typing but with period separated module path }}
4646
{{#let export partial type_period_separated |type| captures |type_name|}}
4747
{{#pragma ignore-newlines}}
48-
{{#if (or type.structured? type.enum?)}}{{#if type.need_module_path?}}{{type.module_path_period_separated}}.{{/if}}{{type.true_type.name}}
48+
{{#if (or type.structured? type.enum?)}}{{#if type.need_module_path?}}{{type.true_type.program.module_path}}.types.{{/if}}{{type.true_type.name}}
4949
{{#else if type.list?}}_typing.Sequence[{{#partial type_period_separated type=type.true_type.elem_type}}]
5050
{{#else if type.set?}}_typing.AbstractSet[{{#partial type_period_separated type=type.true_type.elem_type}}]
5151
{{#else if type.map?}}_typing.Mapping[{{#partial type_period_separated type=type.true_type.key_type}}, {{#partial type_period_separated type=type.true_type.val_type}}]

third-party/thrift/src/thrift/compiler/generate/templates/py3/common/services.mustache

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ __iobuf_cIOBuf
3131
{{#else if type.iobufRef?}}
3232
unique_ptr__iobuf_cIOBuf
3333
{{#else}}
34-
{{#if type.need_cbinding_path?}}{{type.cbinding_path}}_{{/if}}{{type.cythonCustomType}}
34+
{{#if type.need_cbinding_path?}}{{type.true_type.program.module_path_for_alias}}_cbindings_{{/if}}{{type.cythonCustomType}}
3535
{{/if type.iobuf?}}
3636
{{#else if type.bool?}}cbool
3737
{{#else if type.void?}}cFollyUnit
@@ -45,11 +45,11 @@ unique_ptr__iobuf_cIOBuf
4545
{{#else if type.binary?}}binary
4646
{{#else if (or type.structured? type.enum?)}}
4747
{{#let ttype = type.true_type}}
48-
{{#if ttype.need_cbinding_path?}}{{ttype.cbinding_path}}_{{/if}}{{!
48+
{{#if ttype.need_cbinding_path?}}{{ttype.program.module_path_for_alias}}_cbindings_{{/if}}{{!
4949
}}{{#partial types.c_binding node=ttype}}
5050
{{#else if type.container?}}
5151
{{#let ttype = type.true_type}}
52-
{{#if (and (not type.defaultTemplate?) type.need_cbinding_path?)}}{{type.cbinding_path}}_{{/if}}
52+
{{#if (and (not type.defaultTemplate?) type.need_cbinding_path?)}}{{ttype.program.module_path_for_alias}}_cbindings_{{/if}}
5353
{{#if type.map?}}
5454
{{type.cythonTemplate}}__{{#partial cython_cpp_type_ident type=ttype.key_type}}_{{#partial cython_cpp_type_ident type=ttype.val_type}}
5555
{{#else}}
@@ -133,7 +133,7 @@ cmove(deref((<ServerStream_{{#partial cython_cpp_type_ident type=function.stream
133133
{{#let ttype = type.true_type}}
134134
{{#if (not function.stack_arguments?)}}make_unique[{{#partial types.cython_cpp_type type=type}}]({{/if}}
135135
{{#if root_program.inplace_migrate?}}
136-
{{type.capi_converter_path}}.{{ttype.name}}_convert_to_cpp(result._to_python())
136+
{{ttype.program.module_path_for_alias}}_thrift_converter.{{ttype.name}}_convert_to_cpp(result._to_python())
137137
{{#else}}
138138
deref((<{{#partial types.cython_python_type type=type}}?> result).{{#partial types.cpp_obj}})
139139
{{/if root_program.inplace_migrate?}}

third-party/thrift/src/thrift/compiler/generate/templates/py3/common/stream.mustache

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ identification. Takes a function and produces
4444
{{#else if type.enum?}}{{#partial types.python_type type=type}}(<int> _value)
4545
{{#else if type.structured?}}
4646
{{#if root_program.inplace_migrate?}}
47-
{{#partial types.python_type type=type}}.from_python({{type.capi_converter_path}}.{{type.true_type.name}}_from_cpp(__deref_const[{{#partial types.cython_cpp_type type=type}}](_value)))
47+
{{#partial types.python_type type=type}}.from_python({{type.true_type.program.module_path_for_alias}}_thrift_converter.{{type.true_type.name}}_from_cpp(__deref_const[{{#partial types.cython_cpp_type type=type}}](_value)))
4848
{{#else}}
4949
{{#partial types.cython_python_type type=type}}._create_FBTHRIFT_ONLY_DO_NOT_USE(make_shared[{{#partial types.cython_cpp_type type=type}}](_value))
5050
{{/if root_program.inplace_migrate?}}

0 commit comments

Comments
 (0)