Skip to content

Commit

Permalink
Introduce PYCLIF_PYBIND11_MODULE() macro.
Browse files Browse the repository at this point in the history
Proactive prevention of accumulating tech debt:

The main purpose of this change is to make it less likely that the boilerplate code usually hidden behind the [`PYBIND11_MODULE()`](https://github.com/pybind/pybind11/blob/8b48ff878c168b51fe5ef7b8c728815b9e1a9857/include/pybind11/detail/common.h#L463-L479) macro leaks into checked-in code when PyCLIF-generated pybind11 bindings are manually converted to pure pybind11 extension modules.

This change also makes it much less likely that the auto-generated module docstring and the (currently) unconditional `pybind11_protobuf::ImportNativeProtoCasters()` call leak into checked-in code.

GitHub testing: #92

TGP testing is currently not needed (PyCLIF-C-API is still the default).

PiperOrigin-RevId: 609818592
  • Loading branch information
rwgk committed Feb 25, 2024
1 parent 4ec6ff7 commit 1ce44c3
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 27 deletions.
44 changes: 17 additions & 27 deletions clif/pybind11/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,27 +181,12 @@ def generate_from(self, ast: ast_pb2.AST):

yield 'namespace {'
yield ''
yield 'PyObject * this_module_init() noexcept {'
yield I + 'PYBIND11_CHECK_PYTHON_VERSION'
yield I + 'PYBIND11_ENSURE_INTERNALS_READY'
yield I + ('static pybind11::module_::module_def '
f'module_def_{self._module_name};')
yield I + ('auto m = pybind11::module_::create_extension_module('
f'"{self._module_name}", nullptr, '
f'&module_def_{self._module_name});')
yield I + 'try {'
yield I + I + 'm.attr("__pyclif_codegen_mode__") = "pybind11";'
yield '// When manually converting this code to a pure pybind11 extension,'
yield '// change this function to:'
yield f'// PYBIND11_MODULE({self._module_name}, m)'
yield 'void PyclifPybind11ModuleInit(py::module_ m) {'
for s in self._generate_import_modules(ast):
yield I + s
yield I + I + f'm.doc() = "CLIF-generated module for {ast.source}";'
if self._codegen_info.requires_status:
yield I + I + ('pybind11::module_::import('
'"util.task.python.error");')
yield I + I + 'pybind11_protobuf::check_unknown_fields::'
yield I + I + ' ExtensionsWithUnknownFieldsPolicy::'
yield I + I + ' WeakEnableFallbackToSerializeParse();'
yield I + I + 'pybind11_protobuf::ImportNativeProtoCasters();'

for decl in ast.decls:
if decl.decltype == ast_pb2.Decl.Type.FUNC:
for s in function.generate_from(
Expand All @@ -217,26 +202,30 @@ def generate_from(self, ast: ast_pb2.AST):
elif decl.decltype == ast_pb2.Decl.Type.ENUM:
for s in enums.generate_from('m', decl.enum):
yield I + s
yield I + I + 'return m.ptr();'
yield I + '}'
yield I + 'PYBIND11_CATCH_INIT_EXCEPTIONS'
yield '}'
yield ''
yield '} // namespace'
yield ''
mangled_module_name = utils.generate_mangled_name_for_module(
self._module_path)
yield f'extern "C" PyObject* GooglePyInit_{mangled_module_name}() {{'
yield I + 'return this_module_init();'
yield '}'
yield ''

yield '// When manually converting this code to a pure pybind11 extension,'
yield '// remove this macro invocation entirely.'
yield 'PYCLIF_PYBIND11_MODULE('
yield f' "{ast.source}",'
yield f' GooglePyInit_{mangled_module_name},'
yield f' "{self._module_name}")'

insert_empty_line = True
for namespace, typedefs in itertools.groupby(
self._types, lambda gen_type: gen_type.cpp_namespace):
if insert_empty_line:
insert_empty_line = False
yield ''
namespace = namespace.strip(':') or 'clif'
yield ' '.join('namespace %s {' % ns for ns in namespace.split('::'))
for t in typedefs:
yield from t.generate_converters()
yield ''
yield '} ' * (1 + namespace.count('::')) + ' // namespace ' + namespace

def _generate_import_modules(self,
Expand Down Expand Up @@ -280,6 +269,7 @@ def _generate_headlines(self):
yield f'#include "{include}"'
yield f'#include "{self._header_path}"'
yield '#include "clif/pybind11/clif_type_casters.h"'
yield '#include "clif/pybind11/pyclif_pybind11_module_macro.h"'
yield '#include "clif/pybind11/runtime.h"'
yield '#include "clif/pybind11/type_casters.h"'
yield '#include "third_party/pybind11_protobuf/native_proto_caster.h"'
Expand Down
41 changes: 41 additions & 0 deletions clif/pybind11/pyclif_pybind11_module_macro.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2024 Google LLC
//
// 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.

#ifndef CLIF_PYBIND11_PYCLIF_PYBIND11_MODULE_MACRO_H_
#define CLIF_PYBIND11_PYCLIF_PYBIND11_MODULE_MACRO_H_

#include "third_party/pybind11/include/pybind11/pybind11.h"
#include "third_party/pybind11_protobuf/native_proto_caster.h"

#define PYCLIF_PYBIND11_MODULE(CLIF_SOURCE_FILENAME, PYINIT_NAME, MODULE_NAME) \
extern "C" PyObject* PYINIT_NAME() { \
PYBIND11_CHECK_PYTHON_VERSION \
PYBIND11_ENSURE_INTERNALS_READY \
static pybind11::module_::module_def this_module_def; \
auto m = pybind11::module_::create_extension_module(MODULE_NAME, nullptr, \
&this_module_def); \
try { \
m.attr("__pyclif_codegen_mode__") = "pybind11"; \
m.doc() = "CLIF-generated module for " CLIF_SOURCE_FILENAME; \
pybind11_protobuf::check_unknown_fields:: \
ExtensionsWithUnknownFieldsPolicy:: \
WeakEnableFallbackToSerializeParse(); \
pybind11_protobuf::ImportNativeProtoCasters(); \
PyclifPybind11ModuleInit(m); \
return m.ptr(); \
} \
PYBIND11_CATCH_INIT_EXCEPTIONS \
}

#endif // CLIF_PYBIND11_PYCLIF_PYBIND11_MODULE_MACRO_H_

0 comments on commit 1ce44c3

Please sign in to comment.