Skip to content

Commit

Permalink
refactor Enum code generation
Browse files Browse the repository at this point in the history
Summary:
Changed the way how enums generated in thrift-py3:

1. Use C++ map to store name to unique Enum Python instances cache
2. Dynamically find instances from cache (or create one and add to cache) instead of populating all instances at module load time
3. use `__getattr__` and `__getitem__` instead of code-gen all the getters

Reviewed By: asp2insp

Differential Revision: D23182622

fbshipit-source-id: 75baef997ee2d604b4ef7606f282cfc3306208ac
  • Loading branch information
Nanshu Chen authored and facebook-github-bot committed Aug 20, 2020
1 parent 0580653 commit b974229
Show file tree
Hide file tree
Showing 130 changed files with 2,611 additions and 4,473 deletions.
25 changes: 11 additions & 14 deletions thrift/compiler/generate/t_mstch_py3_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ class mstch_py3_field : public mstch_field {
{"field:isset?", &mstch_py3_field::isSet},
{"field:cppName", &mstch_py3_field::cppName},
{"field:hasModifiedName?", &mstch_py3_field::hasModifiedName},
{"field:enumSafeName", &mstch_py3_field::enumSafeName},
{"field:hasPyName?", &mstch_py3_field::hasPyName},
});
}

Expand Down Expand Up @@ -837,11 +837,8 @@ class mstch_py3_field : public mstch_field {
return pyName_ != cppName_;
}

mstch::node enumSafeName() {
if (pyName_ == "name" || pyName_ == "value") {
return pyName_ + "_";
}
return pyName_;
mstch::node hasPyName() {
return pyName_ != field_->get_name();
}

bool has_default_value() {
Expand Down Expand Up @@ -1010,7 +1007,7 @@ class mstch_py3_enum_value : public mstch_enum_value {
{
{"enumValue:py_name", &mstch_py3_enum_value::pyName},
{"enumValue:cppName", &mstch_py3_enum_value::cppName},
{"enumValue:enumSafeName", &mstch_py3_enum_value::enumSafeName},
{"enumValue:hasPyName?", &mstch_py3_enum_value::hasPyName},
});
}

Expand All @@ -1022,12 +1019,8 @@ class mstch_py3_enum_value : public mstch_enum_value {
return get_cppname<t_enum_value>(*enm_value_);
}

mstch::node enumSafeName() {
auto pyName = py3::get_py3_name(*enm_value_);
if (pyName == "name" || pyName == "value") {
return pyName + "_";
}
return pyName;
mstch::node hasPyName() {
return py3::get_py3_name(*enm_value_) != enm_value_->get_name();
}
};

Expand Down Expand Up @@ -1456,13 +1449,16 @@ void t_mstch_py3_generator::generate_module(ModuleType moduleType) {
if (moduleType != ModuleType::REFLECTIONS) {
exts.push_back(".pyi");
}
if (moduleType == ModuleType::TYPES) {
exts.push_back(".h");
}
for (auto ext : exts) {
render_to_file(
nodePtr, module + ext, generateRootPath_ / name / (module + ext));
}
auto cpp_path = boost::filesystem::path{name};
if (moduleType == ModuleType::CLIENTS || moduleType == ModuleType::SERVICES) {
auto basename = module + "_wrapper";
auto cpp_path = boost::filesystem::path{name};
for (auto ext : {".h", ".cpp"}) {
render_to_file(nodePtr, basename + ext, cpp_path / (basename + ext));
}
Expand All @@ -1474,6 +1470,7 @@ void t_mstch_py3_generator::generate_module(ModuleType moduleType) {
}

if (moduleType == ModuleType::TYPES) {
render_to_file(nodePtr, "types.h", cpp_path / "types.h");
programNodePtr->setTypeContext(false);
for (auto ext : {".pxd", ".pyx"}) {
const auto filename = std::string{"types_reflection"} + ext;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ on a struct.
}}{{#type:string?}}.encode('utf-8'))){{/type:string?}}{{!
}}{{#type:binary?}})){{/type:binary?}}{{!
}}{{/type:base?}}{{!
}}{{#type:enum?}}{{!
}}{{> types/CythonPythonType}}_to_cpp({{field:py_name}}){{!
}}{{/type:enum?}}{{!
}}{{#type:enum?}}<{{> types/CythonCppType}}><int>{{field:py_name}}{{/type:enum?}}{{!
}}{{#type:struct?}}{{!
}}deref((<{{> types/CythonPythonType }}?> {{field:py_name}})._cpp_obj){{!
}}{{/type:struct?}}{{!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ This template assumes that the Python object is in a variable named <field>.
}}{{#type:container?}}{{!
}}deref((<{{> types/CythonPythonType}}>{{field:py_name}})._cpp_obj){{!
}}{{/type:container?}}{{!
}}{{#type:enum}}{{> types/CythonPythonType}}_to_cpp({{field:py_name}}){{/type:enum}}{{!
}}{{#type:enum?}}<{{> types/CythonCppType}}><int>{{field:py_name}}{{/type:enum?}}{{!
}}{{/type:hasCustomTypeBehavior?}}
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,5 @@ key, include the parallel CythonPythonToCppKey file instead.
}}{{#type:container?}}{{!
}}deref((<{{> types/CythonPythonType}}>item)._cpp_obj){{!
}}{{/type:container?}}{{!
}}{{#type:enum}}{{> types/CythonPythonType}}_to_cpp(item){{/type:enum}}{{!
}}{{#type:enum?}}<{{> types/CythonCppType}}><int>item{{/type:enum?}}{{!
}}{{/type:hasCustomTypeBehavior?}}
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,4 @@ keys (conventionally used for sets, lists, and map values).
}}{{#type:container?}}{{!
}}deref((<{{> types/CythonPythonType}}>key)._cpp_obj){{!
}}{{/type:container?}}{{!
}}{{#type:enum}}{{> types/CythonPythonType}}_to_cpp(key){{/type:enum}}
}}{{#type:enum?}}<{{> types/CythonCppType}}><int>key{{/type:enum?}}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ on a union.
}}{{/type:binary?}}{{!
}}{{#type:enum?}}{{!
}}deref(c_inst).set_{{field:py_name}}({{!
}}{{> types/CythonPythonType}}_to_cpp({{field:py_name}})){{!
}}<{{> types/CythonCppType}}><int>{{field:py_name}}){{!
}}{{/type:enum?}}{{!
}}{{#type:struct?}}{{!
}}deref(c_inst).set_{{field:py_name}}({{!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ to the client.
}}{{^program:stack_arguments?}}){{/program:stack_arguments?}}{{!
}}{{/type:container?}}{{!
}}{{#type:void?}}c_unit{{/type:void?}}{{!
}}{{#type:enum}}{{> types/CythonPythonType}}_to_cpp(result){{/type:enum}}{{!
}}{{#type:enum?}}<{{> types/CythonCppType}}><int>result{{/type:enum?}}{{!
}}{{/type:hasCustomTypeBehavior?}}
60 changes: 60 additions & 0 deletions thrift/compiler/generate/templates/py3/types.h.mustache
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
{{!
Copyright (c) Facebook, Inc. and its affiliates.
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.
}}{{!
C++ header to store py3 specific Enum and Union type enum data
}}
{{> common/AutoGeneratedC}}

#pragma once

#include <thrift/lib/py3/enums.h>
#include "{{program:includePrefix}}gen-cpp2/{{program:name}}_types.h"
{{#program:enums}}

template<>
const std::vector<std::pair<std::string_view, std::string_view>>& ::thrift::py3::PyEnumTraits<
{{#program:cppNamespaces}}::{{value}}{{/program:cppNamespaces}}::{{enum:name}}>::namesmap() {
static const folly::Indestructible<NamesMap> pairs {
{
{{#enum:values}}{{#enumValue:hasPyName?}}
{"{{enumValue:py_name}}", "{{enumValue:name}}"},
{{/enumValue:hasPyName?}}{{/enum:values}}
}
};
return *pairs;
}

{{/program:enums}}
{{#program:structs}}
{{#struct:union?}}

template<>
const std::vector<std::pair<std::string_view, std::string_view>>& ::thrift::py3::PyEnumTraits<
{{#program:cppNamespaces}}::{{value}}{{/program:cppNamespaces}}::{{struct:name}}::Type>::namesmap() {
static const folly::Indestructible<NamesMap> pairs {
{
{{#struct:fields}}{{#field:hasPyName?}}
{"{{field:py_name}}", "{{field:name}}"},
{{/field:hasPyName?}}{{/struct:fields}}
}
};
return *pairs;
}
{{/struct:union?}}
{{/program:structs}}
9 changes: 4 additions & 5 deletions thrift/compiler/generate/templates/py3/types.pxd.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ from thrift.py3.common cimport RpcOptions as __RpcOptions
{{#program:includeNamespaces}}
cimport {{#includeNamespace}}{{value}}.{{/includeNamespace}}types as _{{#includeNamespace}}{{value}}_{{/includeNamespace}}types
{{/program:includeNamespaces}}
cdef extern from "{{program:includePrefix}}gen-py3/{{program:name}}/types.h":
pass

{{#program:cppIncludes}}
cdef extern from "{{.}}":
Expand All @@ -76,11 +78,8 @@ cdef extern from "{{program:includePrefix}}gen-cpp2/{{program:name}}_types.h"{{!
}} namespace "{{#program:cppNamespaces}}::{{value}}{{/program:cppNamespaces}}":
{{/first?}}
cdef cppclass c{{enum:name}} "::{{#program:cppNamespaces}}{{value}}::{{/program:cppNamespaces}}{{enum:name}}":
bint operator==(c{{enum:name}}&)
bint operator!=(c{{enum:name}}&)
{{#enum:values}}
c{{enum:name}} {{enum:name}}__{{enumValue:py_name}} "::{{#program:cppNamespaces}}{{value}}::{{/program:cppNamespaces}}{{enum:name}}::{{enumValue:cppName}}"
{{/enum:values}}
pass

{{/program:enums}}


Expand Down
4 changes: 2 additions & 2 deletions thrift/compiler/generate/templates/py3/types.pyi.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ __property__ = property
{{#program:enums}}
class {{enum:name}}(thrift.py3.types.{{^enum:flags?}}Enum{{/enum:flags?}}{{#enum:flags?}}Flag{{/enum:flags?}}):
{{#enum:values}}
{{enumValue:enumSafeName}}: {{enum:name}} = ...
{{enumValue:py_name}}: {{enum:name}} = ...
{{/enum:values}}
{{^enum:values}}
pass
Expand Down Expand Up @@ -100,7 +100,7 @@ class {{struct:name}}({{> types/PythonStructClass}}, _typing.Hashable{{^struct:u
class Type(thrift.py3.types.Enum):
EMPTY: {{struct:name}}.Type = ...
{{#struct:fields}}
{{field:enumSafeName}}: {{struct:name}}.Type = ...
{{field:py_name}}: {{struct:name}}.Type = ...
{{/struct:fields}}

@staticmethod
Expand Down
6 changes: 5 additions & 1 deletion thrift/compiler/generate/templates/py3/types.pyx.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ from libcpp.memory cimport shared_ptr, make_shared, unique_ptr, make_unique
from libcpp.string cimport string
from libcpp cimport bool as cbool
from libcpp.iterator cimport inserter as cinserter
from libcpp.utility cimport move as cmove
from cpython cimport bool as pbool
from cython.operator cimport dereference as deref, preincrement as inc, address as ptr_address
import thrift.py3.types
Expand All @@ -51,6 +52,10 @@ from thrift.py3.types cimport (
constant_shared_ptr,
default_inst,
NOTSET as __NOTSET,
EnumData as __EnumData,
EnumFlagsData as __EnumFlagsData,
UnionTypeEnumData as __UnionTypeEnumData,
createEnumDataForUnionType as __createEnumDataForUnionType,
)
cimport thrift.py3.std_libcpp as std_libcpp
cimport thrift.py3.serializer as serializer
Expand All @@ -61,7 +66,6 @@ from folly.optional cimport cOptional
import sys
import itertools
from collections.abc import Sequence, Set, Mapping, Iterable
import warnings
import weakref as __weakref
import builtins as _builtins
{{#program:has_stream?}}
Expand Down
Loading

0 comments on commit b974229

Please sign in to comment.