Skip to content

Commit 3b06c42

Browse files
committed
Add __pyclif_codegen_mode__ to all PyCLIF-generated extension modules.
This is to enable replacing ad-hoc inspection of the generated extension module docstrings with more formal conditions, e.g.: ``` if __pyclif_codegen_mode__ == "pybind11": ``` This will make it easier to discover such conditions. (Current company priorities (staffing) make it seem very likely that the two codegen modes will co-exist for an extended period of time.) For completeness and easy reference: * cl/591787852 will update the only use case outside third_party/clif & devtools/clif. * cl/591787855 will make the PyCLIF-pybind11 generated docstrings identical to the PyCLIF-C-API generated docstrings. PiperOrigin-RevId: 592058095
1 parent 0fec475 commit 3b06c42

26 files changed

+69
-36
lines changed

clif/pybind11/generator.py

+1
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ def generate_from(self, ast: ast_pb2.AST):
190190
f'"{self._module_name}", nullptr, '
191191
f'&module_def_{self._module_name});')
192192
yield I + 'try {'
193+
yield I + I + 'm.attr("__pyclif_codegen_mode__") = "pybind11";'
193194
for s in self._generate_import_modules(ast):
194195
yield I + s
195196
yield I + I + ('m.doc() = "CLIF-generated pybind11-based module for '

clif/python/gen.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ def InitFunction(doc, meth_ref, init, dict_):
310310
yield '};'
311311
yield ''
312312
yield 'PyObject* Init() {'
313-
yield I+'PyObject* module = PyModule_Create(&Module);'
313+
yield I+'PyObject* module = ModuleCreateAndSetPyClifCodeGenMode(&Module);'
314314
yield I+'if (!module) return nullptr;'
315315
init_needs_err = False
316316
for s in init:

clif/python/module_test.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def testFuncInit(self):
6868
};
6969
7070
PyObject* Init() {
71-
PyObject* module = PyModule_Create(&Module);
71+
PyObject* module = ModuleCreateAndSetPyClifCodeGenMode(&Module);
7272
if (!module) return nullptr;
7373
return module;
7474
}"""))
@@ -110,7 +110,7 @@ def testModConst(self):
110110
};
111111
112112
PyObject* Init() {
113-
PyObject* module = PyModule_Create(&Module);
113+
PyObject* module = ModuleCreateAndSetPyClifCodeGenMode(&Module);
114114
if (!module) return nullptr;
115115
if (something_wrong) goto err;
116116
if (PyModule_AddObject(module, "ONE", Clif_PyObjFrom(static_cast<int>(kOne), {})) < 0) goto err;

clif/python/runtime.cc

+25
Original file line numberDiff line numberDiff line change
@@ -533,4 +533,29 @@ void SetIsNotConvertibleError(PyObject* py_obj, const char* cpp_type) {
533533
ClassName(py_obj), ClassType(py_obj), cpp_type);
534534
}
535535

536+
namespace {
537+
538+
bool SetPyClifCodeGenMode(PyObject* module, const char* codegen_mode) {
539+
PyObject* codegen_mode_py = PyUnicode_FromString(codegen_mode);
540+
if (codegen_mode_py == nullptr) {
541+
return false;
542+
}
543+
int stat = PyObject_SetAttrString(module, "__pyclif_codegen_mode__",
544+
codegen_mode_py);
545+
Py_DECREF(codegen_mode_py);
546+
return (stat == 0);
547+
}
548+
549+
} // namespace
550+
551+
PyObject* ModuleCreateAndSetPyClifCodeGenMode(PyModuleDef* module_def) {
552+
PyObject* module = PyModule_Create(module_def);
553+
if (!module) return nullptr;
554+
if (!SetPyClifCodeGenMode(module, "c_api")) {
555+
Py_DECREF(module);
556+
return nullptr;
557+
}
558+
return module;
559+
}
560+
536561
} // namespace clif

clif/python/runtime.h

+2
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ void LogFatalIfPythonErrorOccurred();
214214
// PyObjectTypeIsConvertible*() functions above.
215215
void SetIsNotConvertibleError(PyObject* py_obj, const char* cpp_type);
216216

217+
PyObject* ModuleCreateAndSetPyClifCodeGenMode(PyModuleDef* module_def);
218+
217219
} // namespace clif
218220

219221
#endif // CLIF_PYTHON_RUNTIME_H_

clif/testing/python/const_char_ptr_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def testReturnConstCharPtrAsStr(self):
2626

2727
def testReturnConstCharPtrAsBytes(self):
2828
res = const_char_ptr.ReturnConstCharPtrAsBytes()
29-
if 'pybind11' not in const_char_ptr.__doc__:
29+
if const_char_ptr.__pyclif_codegen_mode__ == 'c_api':
3030
# BUG: Return value should be bytes but is str (Python 3).
3131
self.assertIsInstance(res, str) # Should be bytes.
3232
self.assertEqual(res, 'string literal') # Should be b'string literal'.

clif/testing/python/copy_move_types_custom_from_as_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from clif.testing.python import copy_move_types_library
1919

2020
ENV_IX = 1
21-
if "pybind11" in tm.__doc__:
21+
if tm.__pyclif_codegen_mode__ == "pybind11":
2222
ENV_IX += 2
2323

2424

clif/testing/python/extend_init_test.py

+9-9
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,15 @@ def test_init_with_kwargs(self):
4848
self.assertEqual(obj.get_k(), expected_k)
4949

5050
def test_constructor_return_nullptr(self):
51-
if 'pybind11' in extend_init.__doc__:
52-
self.skipTest(
53-
'Pybind11 does not allow factory functions to return nullptr.')
54-
obj = extend_init.TestCase3(1)
55-
with self.assertRaises(ValueError):
56-
obj.get_value()
51+
if extend_init.__pyclif_codegen_mode__ == 'pybind11':
52+
with self.assertRaisesRegex(
53+
TypeError, r'^pybind11::init\(\): factory function returned nullptr$'
54+
):
55+
extend_init.TestCase3(1)
56+
else:
57+
obj = extend_init.TestCase3(1)
58+
with self.assertRaises(ValueError):
59+
obj.get_value()
5760

5861
def test_extend_init_without_default_constructor(self):
5962
expected_value = 0
@@ -78,9 +81,6 @@ def test_py_err_from_constructor(self):
7881
self.assertIsInstance(
7982
extend_init.TestPyErrFromConstructor(0),
8083
extend_init.TestPyErrFromConstructor)
81-
if 'pybind11' in extend_init.__doc__:
82-
self.skipTest(
83-
'Pybind11 does not allow factory functions to return nullptr.')
8484
with self.assertRaises(ValueError) as ctx:
8585
extend_init.TestPyErrFromConstructor(1)
8686
self.assertEqual(str(ctx.exception), 'RaisedFromExtendInit')

clif/testing/python/extend_methods_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ def testConcreteExtend(self):
213213
self.assertEqual(s, 'getter:18:ConcreteHolder')
214214

215215
def testVirtualExtend(self):
216-
if 'pybind11' not in extend_methods.__doc__:
216+
if extend_methods.__pyclif_codegen_mode__ == 'c_api':
217217
expected_doc = 'Added to VirtualBaseHolder.'
218218
self.assertEqual(extend_methods.VirtualBaseHolder.__doc__, expected_doc)
219219
self.assertEqual(extend_methods.VirtualDerivedHolder.__doc__,

clif/testing/python/iterator_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ def testR5(self):
4747
def test_weakref(self):
4848
r = iterator.Ring5()
4949
self.assertIsNotNone(weakref.ref(r)())
50-
if 'pybind11' not in iterator.__doc__:
50+
if iterator.__pyclif_codegen_mode__ == 'c_api':
5151
with self.assertRaises(TypeError) as ctx:
5252
weakref.ref(iter(r))
5353
self.assertIn('cannot create weak reference', str(ctx.exception))

clif/testing/python/lambda_expressions_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def test_context_manager(self):
127127

128128
def test_cpp_function_set_python_exception(self):
129129
expected_exception_type = (
130-
ValueError if 'pybind11' in lambda_expressions.__doc__
130+
ValueError if lambda_expressions.__pyclif_codegen_mode__ == 'pybind11'
131131
else SystemError)
132132
with self.assertRaises(expected_exception_type):
133133
lambda_expressions.python_exception_in_function()

clif/testing/python/mock_uninitializable_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def __init__(self):
7878
class DerivedFromNoCtorTest(absltest.TestCase):
7979

8080
def testInstantiate(self):
81-
if "pybind11" in tm.__doc__:
81+
if tm.__pyclif_codegen_mode__ == "pybind11":
8282
regex_expected = (
8383
r"mock_uninitializable\.NoCtor\.__init__\(\) must be called when"
8484
r" overriding __init__$"

clif/testing/python/pass_none_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ def testPassSharedPtrHolder(self):
5555
res = pass_none.pass_shared_ptr_holder(ih)
5656
self.assertEqual(res, 559)
5757

58-
if 'pybind11' in pass_none.__doc__:
58+
if pass_none.__pyclif_codegen_mode__ == 'pybind11':
5959
self.assertEqual(pass_none.pass_shared_ptr_holder(None), 17)
6060
else:
6161
# The generated C API code does not support shared_ptr from None.

clif/testing/python/pyobject_ptr_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def cb(pvh):
8181
self.assertEqual(cc(cb, PyValueHolder(20)).value, 100)
8282
for _ in range(1000):
8383
self.assertEqual(cc(cb, PyValueHolder("Two")).value, 101)
84-
if "pybind11" in tst.__doc__:
84+
if tst.__pyclif_codegen_mode__ == "pybind11":
8585
for _ in range(1000):
8686
with self.assertRaisesRegex(ValueError, r"^Unknown pvh\.value: 3\.0$"):
8787
cc(cb, PyValueHolder(3.0))

clif/testing/python/raw_pointer_return_member_function_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def testPersistentHolder(self):
2525
self.assertEqual(h.vec_at(1).int_value, 24)
2626

2727
def testTemporaryHolder(self):
28-
if 'pybind11' not in tst.__doc__:
28+
if tst.__pyclif_codegen_mode__ == 'c_api':
2929
self.skipTest(
3030
'Only PyCLIF-pybind11 handles this correctly'
3131
' (avoids creating a dangling pointer).'

clif/testing/python/simple_type_conversions_test.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class SimpleTypeConversions(absltest.TestCase):
2222
def testSignedCharManipulation(self):
2323
self.assertEqual(tm.SignedCharManipulation(2), 29)
2424
for inp in [-129, 128]:
25-
if 'pybind11' in tm.__doc__:
25+
if tm.__pyclif_codegen_mode__ == 'pybind11':
2626
with self.assertRaises(TypeError) as ctx:
2727
tm.SignedCharManipulation(inp)
2828
self.assertIn('incompatible function arguments.', str(ctx.exception))
@@ -37,7 +37,7 @@ def testSignedCharManipulation(self):
3737

3838
def testUnsignedCharManipulation(self):
3939
self.assertEqual(tm.UnsignedCharManipulation(3), 39)
40-
if 'pybind11' in tm.__doc__:
40+
if tm.__pyclif_codegen_mode__ == 'pybind11':
4141
with self.assertRaises(TypeError) as ctx:
4242
tm.SignedCharManipulation(256)
4343
self.assertIn('incompatible function arguments.', str(ctx.exception))
@@ -55,7 +55,7 @@ def testPassUint32InRange(self):
5555
self.assertEqual(tm.PassUint32(2**32 - 1), '4294967295')
5656

5757
def testPassUint32Negative(self):
58-
if 'pybind11' in tm.__doc__:
58+
if tm.__pyclif_codegen_mode__ == 'pybind11':
5959
with self.assertRaisesRegex(
6060
TypeError, r'PassUint32\(\): incompatible function arguments.*'
6161
):
@@ -69,7 +69,7 @@ def testPassUint32Negative(self):
6969
tm.PassUint32(-1)
7070

7171
def testPassUint32OutOfRange(self):
72-
if 'pybind11' in tm.__doc__:
72+
if tm.__pyclif_codegen_mode__ == 'pybind11':
7373
with self.assertRaisesRegex(
7474
TypeError, r'PassUint32\(\): incompatible function arguments.*'
7575
):

clif/testing/python/slots_test.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020

2121

2222
def expected_exception():
23-
return AttributeError if 'pybind11' in slots.__doc__ else NotImplementedError
23+
return (
24+
AttributeError
25+
if slots.__pyclif_codegen_mode__ == 'pybind11'
26+
else NotImplementedError
27+
)
2428

2529

2630
class SlotsTest(absltest.TestCase):

clif/testing/python/smart_ptrs_test.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ def testInfiniteLoopPerformSP(self):
109109

110110
def testInfiniteLoopPerformUP(self):
111111
# No leak (manually verified under cl/362141489).
112-
if 'pybind11' in smart_ptrs.__doc__:
112+
if smart_ptrs.__pyclif_codegen_mode__ == 'pybind11':
113113
return # pybind11 raises a ValueError (exercised above).
114114
while True:
115115
self.assertEqual(smart_ptrs.PerformUP(Add(17, 23)), 40)
@@ -125,7 +125,7 @@ def testInfiniteLoopRunStashedSPOperation(self):
125125

126126
def testInfiniteLoopRunStashedUPOperation(self):
127127
# No leak (manually verified under cl/362141489).
128-
if 'pybind11' in smart_ptrs.__doc__:
128+
if smart_ptrs.__pyclif_codegen_mode__ == 'pybind11':
129129
return # pybind11 raises a ValueError (exercised above).
130130
while True:
131131
s = smart_ptrs.OperationStashUP()

clif/testing/python/std_containers_copy_move_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from clif.testing.python import std_containers_copy_move as tm
2121

2222
ENV_IX = 1
23-
if "pybind11" in tm.__doc__:
23+
if tm.__pyclif_codegen_mode__ == "pybind11":
2424
ENV_IX += 2
2525

2626

clif/testing/python/std_containers_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ def testContainerWithCustomizedHash(self):
167167
std_containers.consume_unordered_set_customized_hash(my_set), 2)
168168

169169
@absltest.skipIf(
170-
'pybind11' not in std_containers.__doc__,
170+
std_containers.__pyclif_codegen_mode__ == 'c_api',
171171
'Legacy PyCLIF does not accept None as smart pointers.')
172172
def testAcceptNoneAsSmartPointers(self):
173173
self.assertIsNone(std_containers.unique_ptr_vector_round_trip(None))

clif/testing/python/t1_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def testFunctionDocstring(self):
6666
self.assertIn('spans multiple lines', t1.Sum3.__doc__)
6767

6868
def testFunctionDocstringNoTrailingWhitespaces(self):
69-
if 'pybind11' in t1.__doc__:
69+
if t1.__pyclif_codegen_mode__ == 'pybind11':
7070
self.skipTest(
7171
'pybind11 automatically adds a trailing whitespace to function '
7272
'docstrings.')

clif/testing/python/t3_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def testEnum(self):
3434
self.assertRaises(TypeError, t3.K().M, (5))
3535

3636
@absltest.skipIf(
37-
'pybind11' in t3.__doc__,
37+
t3.__pyclif_codegen_mode__ == 'pybind11',
3838
'When building with pybind11, t3._New.__module__ is a Module object '
3939
'instead of a string.',
4040
)

clif/testing/python/top_level_pass_test.py

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ def testEmptyModule(self):
2323
self.assertRegexpMatches( # pylint: disable=deprecated-method
2424
top_level_pass.__doc__,
2525
'CLIF-generated .*module for .*top_level_pass.clif')
26+
self.assertIn(top_level_pass.__pyclif_codegen_mode__, ('c_api', 'pybind11'))
2627

2728

2829
if __name__ == '__main__':

clif/testing/python/type_caster_test.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ def test_pyobjas_only(self):
4949
self.assertEqual(type_caster.get_value_pyobjas_only(10), 13)
5050

5151
@absltest.skipIf(
52-
'pybind11' not in type_caster.__doc__,
53-
'Legacy PyCLIF does not recognize `Pybind11Ignore` in '
52+
type_caster.__pyclif_codegen_mode__ == 'c_api',
53+
'PyCLIF-C-API does not recognize `Pybind11Ignore` in '
5454
'`CLIF use` comments')
5555
def test_pybind11_ignore(self):
5656
with self.assertRaises(TypeError):

clif/testing/python/virtual_funcs_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def func(self):
8484
class VirtualTest(absltest.TestCase):
8585

8686
@absltest.skipIf(
87-
'pybind11' in virtual_funcs.__doc__,
87+
virtual_funcs.__pyclif_codegen_mode__ == 'pybind11',
8888
'Currently pybind11 does not throw exceptions when initializing abstract'
8989
'classes.')
9090
def testInitAbstract(self):

clif/testing/python/virtual_py_cpp_mix_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ def testGetFromCppUniquePtrPassingPyDerived(self):
4242

4343
def testCppDerivedGet(self):
4444
d = virtual_py_cpp_mix.CppDerived()
45-
if 'pybind11' in virtual_py_cpp_mix.__doc__:
45+
if virtual_py_cpp_mix.__pyclif_codegen_mode__ == 'pybind11':
4646
expected = 212 # NOT GOOD, but this will be fixed in the switch to ...
4747
else:
4848
expected = 101 # ... pybind11, because this result is correct.

0 commit comments

Comments
 (0)