diff --git a/clif/backend/BUILD b/clif/backend/BUILD index b4b6d1cc..a643e48f 100644 --- a/clif/backend/BUILD +++ b/clif/backend/BUILD @@ -73,6 +73,7 @@ cc_library( name = "strutil", hdrs = ["strutil.h"], deps = [ + "@com_google_absl//absl/strings", "@llvm-project//llvm:Support", ], ) diff --git a/clif/backend/code_builder.cc b/clif/backend/code_builder.cc index 4b7b7c27..dc80e1cf 100644 --- a/clif/backend/code_builder.cc +++ b/clif/backend/code_builder.cc @@ -390,7 +390,7 @@ const std::string& CodeBuilder::BuildCode( current_line_.pop_back(); current_file_.pop_back(); } - LLVM_DEBUG(llvm::dbgs() << clif_ast->DebugString()); + LLVM_DEBUG(llvm::dbgs() << ProtoDebugString(*clif_ast)); LLVM_DEBUG(llvm::dbgs() << code_); return code_; } diff --git a/clif/backend/matcher.cc b/clif/backend/matcher.cc index a95b4765..d790e1ae 100644 --- a/clif/backend/matcher.cc +++ b/clif/backend/matcher.cc @@ -351,7 +351,7 @@ bool ClifMatcher::CompileMatchAndSet( const std::string& input_file_name, const AST& clif_ast, AST* modified_clif_ast) { - LLVM_DEBUG(llvm::dbgs() << clif_ast.DebugString()); + LLVM_DEBUG(llvm::dbgs() << ProtoDebugString(clif_ast)); *modified_clif_ast = clif_ast; modified_clif_ast->set_clif_matcher_argv0(clif_matcher_argv0_); modified_clif_ast->set_clif_matcher_version_stamp(kMatcherVersionStamp); @@ -406,7 +406,7 @@ std::string ClifMatcher::GetQualTypeClifName(QualType qual_type) const { bool ClifMatcher::MatchAndSetAST(AST* clif_ast) { assert(ast_ != nullptr && "RunCompiler must be called prior to this."); int num_unmatched = MatchAndSetDecls(clif_ast->mutable_decls()); - LLVM_DEBUG(llvm::dbgs() << "Matched proto:\n" << clif_ast->DebugString()); + LLVM_DEBUG(llvm::dbgs() << "Matched proto:\n" << ProtoDebugString(*clif_ast)); return num_unmatched == 0; } @@ -1172,7 +1172,7 @@ bool ClifMatcher::MatchAndSetEnum(EnumDecl* enum_decl) { clif_name->set_native(result.GetFirst()->getNameAsString()); } } - LLVM_DEBUG(llvm::dbgs() << enum_decl->DebugString()); + LLVM_DEBUG(llvm::dbgs() << ProtoDebugString(*enum_decl)); return true; } @@ -2736,7 +2736,11 @@ const FunctionDecl* ClifMatcher::MatchAndSetFuncFromCandidates( if (auto method_decl = llvm::dyn_cast(clang_decl)) { func_decl->set_cpp_const_method(method_decl->isConst()); +#if PYCLIF_LLVM_VERSION_MAJOR >= 18 // llvm/llvm-project#78463 + func_decl->set_is_pure_virtual(method_decl->isPureVirtual()); +#else func_decl->set_is_pure_virtual(method_decl->isPure()); +#endif } if (auto named_decl = llvm::dyn_cast(clang_decl)) { diff --git a/clif/backend/strutil.h b/clif/backend/strutil.h index 4e5ccf04..dd6fca05 100644 --- a/clif/backend/strutil.h +++ b/clif/backend/strutil.h @@ -19,6 +19,7 @@ #include #include +#include "absl/strings/str_cat.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" @@ -88,6 +89,11 @@ class NamespaceVector { ComponentsVector namespace_vector_; }; +template +std::string ProtoDebugString(const ProtoType& pb) { + return pb.DebugString(); +} + } // namespace clif #endif // CLIF_BACKEND_STRUTIL_H_ diff --git a/clif/pybind11/classes.py b/clif/pybind11/classes.py index e7e92dd8..b53f5a64 100644 --- a/clif/pybind11/classes.py +++ b/clif/pybind11/classes.py @@ -100,6 +100,7 @@ def generate_from( base.cpp_canonical_type in codegen_info.dynamic_attr_types): enable_instance_dict = True break + definition += ', py::release_gil_before_calling_cpp_dtor()' if mi_bases: definition += ', py::multiple_inheritance()' if enable_instance_dict: diff --git a/clif/pybind11/generator.py b/clif/pybind11/generator.py index 85f22dfd..80e8c0de 100644 --- a/clif/pybind11/generator.py +++ b/clif/pybind11/generator.py @@ -193,11 +193,13 @@ def generate_from(self, ast: ast_pb2.AST): yield I + I + 'm.attr("__pyclif_codegen_mode__") = "pybind11";' for s in self._generate_import_modules(ast): yield I + s - yield I + I + ('m.doc() = "CLIF-generated pybind11-based module for ' - f'{ast.source}";') + 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: diff --git a/clif/pybind11/type_casters.h b/clif/pybind11/type_casters.h index 68e3357a..3ae79a8a 100644 --- a/clif/pybind11/type_casters.h +++ b/clif/pybind11/type_casters.h @@ -31,7 +31,6 @@ #include "absl/container/flat_hash_set.h" #include "clif/python/types.h" #include "third_party/pybind11/include/pybind11/detail/common.h" -#include "third_party/pybind11/include/pybind11/functional.h" #include "third_party/pybind11_abseil/status_caster.h" #include "third_party/pybind11_abseil/statusor_caster.h" #include "util/task/python/clif/status_return_override.h" @@ -39,40 +38,6 @@ namespace pybind11 { namespace detail { -namespace type_caster_std_function_specializations { - -template -struct func_wrapper : func_wrapper_base { - using func_wrapper_base::func_wrapper_base; - absl::Status operator()(Args... args) const { - gil_scoped_acquire acq; - try { - return hfunc.f.call_with_policies(rvpp, std::forward(args)...) - .template cast(); - } catch (error_already_set &e) { - return util_task_python_clif::StatusFromErrorAlreadySet(e); - } - } -}; - -template -struct func_wrapper, Args...> : func_wrapper_base { - using func_wrapper_base::func_wrapper_base; - absl::StatusOr operator()(Args... args) const { - gil_scoped_acquire acq; - try { - return hfunc.f.call_with_policies(rvpp, std::forward(args)...) - .template cast>(); - } catch (error_already_set &e) { - return util_task_python_clif::StatusFromErrorAlreadySet(e); - } catch (cast_error & e) { - return absl::Status(absl::StatusCode::kInvalidArgument, e.what()); - } - } -}; - -} // namespace type_caster_std_function_specializations - template <> struct type_caster { public: diff --git a/clif/python/README.md b/clif/python/README.md index 6d10e5b7..9e2d6b60 100644 --- a/clif/python/README.md +++ b/clif/python/README.md @@ -755,27 +755,84 @@ This statement is rarely needed. See more on types below. ## Type correspondence -CLIF uses Python types in API descriptions. It's a CLIF job to find a proper C++ -type for each signature. +CLIF uses Python types in API descriptions (.clif files). Generally it's CLIF's +job to find the corresponding C++ types automatically. However, it is common +that multiple C++ types are converted to the same Python type, e.g. C++ +`std::unordered_set` and `std::set` are both converted to the Python `set` type. +In such situations only one of the conversions will work implicitly +(this is a limitation of the implementation), while all others need to be +specified explictly, e.g.: -CLIF is currently limited to a single C++ type per Python type. We apologize for -the inconvenience. This limitation may be lifted in the future -versions. +C++: -To change what C++ type CLIF will use for a given Python type the user can -either +```c++ +void pass_unordered_set_int(const std::unordered_set& values); +std::unordered_set return_unordered_set_int(); + +void pass_set_int(const std::set& values); +std::set return_set_int(); +``` + +.clif: + +```python +def pass_unordered_set_int(values: set) +def return_unordered_set_int() -> set + +def pass_set_int(values: `std::set` as set) +def return_set_int() -> `std::set` as set +``` + +What works implicitly can be customized with the [use][use] statement. + +The syntax for nested types is, e.g.: + +C++: + +```c++ +void pass_set_list_int(const std::set>& clusters); +``` + +.clif: + +```python +def pass_set_list_int(clusters: `std::set` as set<`std::list` as list>) +``` + +Note that the backtick syntax also works for simpler types, e.g.: + +C++: + +```c++ +void pass_size_t(std::size_t value); +``` + +.clif: + +```python +def pass_size_t(value: `std::size_t` as int) +``` + +However, in most cases the simpler -* specify an exact C++ type you want to use (ie. ``-> `size_t` as int``) or -* change the default with the [use][use] statement. +``` +def pass_size_t(value: int) +``` + +will also work, if there is an implicit C++ conversion +(in this example between `std::size_t` and `int`). + +NOTE: CLIF will reject unknown types and produce an error. It can be parse-time +error for CLIF types or compile-time error for C++ types. ### Predefined types -CLIF knows some basic types (predefined in `clif/python/types.h`): +CLIF knows some basic types (predefined via `clif/python/types.h`) including: Default C++ type | CLIF type[^type] ------------------------ | ------------------- `int` | `int` -`string` | `bytes` *or* `str` +`string` | `bytes` or `str` `bool` | `bool` `double` | `float` `complex<>` | `complex` @@ -786,31 +843,25 @@ Default C++ type | CLIF type[^type] `std::function` | `(t: T, u: U) -> R` `PyObject*` | `object`[^object] +Note: **Default** in the header row above means that the C++ type does not have + to be specified explicitly in .clif files (unless a `use` statement + changes the default). + [^type]: CLIF types named after the corresponding Python types. [^object]: Be careful when you use `object`, CLIF assumes you **know** what you're doing with Python C API and all its caveats. -CLIF also knows how to handle several compatible types - -Compatible C++ types | Python type ----------------------------------- | ------------------------------ -float | float -map | dict -set | set -list, array, stack | list -deque, queue, priority_queue | list -const char* (as return value only) | str (bytes is *not* supported) - -To use a compatible type mention it explicitly in the declaration, e.g. - -```python -def fsum(array: `std::list` as list) -> `float` as float -``` - -for `float fsum(std::list);` C++ function. - -NOTE: CLIF will reject unknown types and produce an error. It can be parse-time -error for CLIF types or compile-time error for C++ types. +CLIF also knows how to handle various other types including: + +C++ type | CLIF type +------------------------------------ | ------------------------------ +`[u]intXX_t` (e.g. `int8_t`) | `int` +`float` | `float` +`map` | `dict` +`set` | `set` +`list`, `array`, `stack` | `list` +`deque`, `queue`, `priority_queue` | `list` +`const char*` (as return value only) | `str` (`bytes` is **not** supported) ### Unicode diff --git a/clif/testing/class_release_gil_before_calling_cpp_dtor.h b/clif/testing/class_release_gil_before_calling_cpp_dtor.h new file mode 100644 index 00000000..5ac85efa --- /dev/null +++ b/clif/testing/class_release_gil_before_calling_cpp_dtor.h @@ -0,0 +1,63 @@ +/* + * 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. + */ + +// Similar to pybind11/tests/test_class_release_gil_before_calling_cpp_dtor.cpp + +#ifndef CLIF_TESTING_CLASS_RELEASE_GIL_BEFORE_CALLING_CPP_DTOR_H_ +#define CLIF_TESTING_CLASS_RELEASE_GIL_BEFORE_CALLING_CPP_DTOR_H_ + +#include + +#include +#include +#include + +namespace clif_testing::class_release_gil_before_calling_cpp_dtor { + +using RegistryType = std::unordered_map; + +inline RegistryType &PyGILState_Check_Results() { + static auto *singleton = new RegistryType(); + return *singleton; +} + +struct ProbeType { + private: + std::string unique_key; + + public: + explicit ProbeType(const std::string &unique_key) : unique_key{unique_key} {} + + ~ProbeType() { + RegistryType ® = PyGILState_Check_Results(); + assert(reg.count(unique_key) == 0); + reg[unique_key] = PyGILState_Check(); + } +}; + +inline std::string PopPyGILState_Check_Result(const std::string &unique_key) { + RegistryType ® = PyGILState_Check_Results(); + if (reg.count(unique_key) == 0) { + return "MISSING"; + } + int res = reg[unique_key]; + reg.erase(unique_key); + return std::to_string(res); +} + +} // namespace clif_testing::class_release_gil_before_calling_cpp_dtor + +#endif // CLIF_TESTING_CLASS_RELEASE_GIL_BEFORE_CALLING_CPP_DTOR_H_ diff --git a/clif/testing/python/CMakeLists.txt b/clif/testing/python/CMakeLists.txt index c001315a..753fe372 100644 --- a/clif/testing/python/CMakeLists.txt +++ b/clif/testing/python/CMakeLists.txt @@ -247,4 +247,6 @@ add_pyclif_library_for_test(std_containers_copy_move std_containers_copy_move.cl CLIF_DEPS clif_testing_python_copy_move_types_library ) +add_pyclif_library_for_test(class_release_gil_before_calling_cpp_dtor class_release_gil_before_calling_cpp_dtor.clif) + # OMITTED: instrumentation_for_testing_test (pytype currently not supported on GitHub) diff --git a/clif/testing/python/class_release_gil_before_calling_cpp_dtor.clif b/clif/testing/python/class_release_gil_before_calling_cpp_dtor.clif new file mode 100644 index 00000000..f3cb98a4 --- /dev/null +++ b/clif/testing/python/class_release_gil_before_calling_cpp_dtor.clif @@ -0,0 +1,20 @@ +# 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. + +from "clif/testing/class_release_gil_before_calling_cpp_dtor.h": + namespace `clif_testing::class_release_gil_before_calling_cpp_dtor`: + class ProbeType: + def __init__(self, unique_key: str) + + def PopPyGILState_Check_Result(unique_key: str) -> str diff --git a/clif/testing/python/class_release_gil_before_calling_cpp_dtor_test.py b/clif/testing/python/class_release_gil_before_calling_cpp_dtor_test.py new file mode 100644 index 00000000..99465dbb --- /dev/null +++ b/clif/testing/python/class_release_gil_before_calling_cpp_dtor_test.py @@ -0,0 +1,32 @@ +# 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. + +import gc + +from absl.testing import absltest + +from clif.testing.python import class_release_gil_before_calling_cpp_dtor as tm + + +class ReleaseGilBeforeCallingCppDtorTest(absltest.TestCase): + + def test_with_probe_type(self): + unique_key = "release_gil_before_calling_cpp_dtor" + tm.ProbeType(unique_key) + gc.collect() + self.assertEqual(tm.PopPyGILState_Check_Result(unique_key), "0") + + +if __name__ == "__main__": + absltest.main() diff --git a/clif/testing/python/std_containers_copy_move_test.py b/clif/testing/python/std_containers_copy_move_test.py index 3ff77712..18186cb0 100644 --- a/clif/testing/python/std_containers_copy_move_test.py +++ b/clif/testing/python/std_containers_copy_move_test.py @@ -23,6 +23,10 @@ if tm.__pyclif_codegen_mode__ == "pybind11": ENV_IX += 2 +# NOTE: The tests here are known to be hyper-sensitive. +# It is expected that they will need to be adjusted or made more +# permissive (similar to e.g. return_value_policy_test.py). + class StdContainersCopyMoveTestCase(parameterized.TestCase): @@ -40,7 +44,7 @@ class StdContainersCopyMoveTestCase(parameterized.TestCase): [ "DefaultCtor_CpLhs_MvLhs@", "DefaultCtor_CpLhs_MvLhs@", - "DefaultCtor_CpLhs_MvCtorTo@", + "DefaultCtor_CpCtor_MvCtorTo_MvCtorTo@", ][ENV_IX], ), ]) diff --git a/clif/testing/python/top_level_pass_test.py b/clif/testing/python/top_level_pass_test.py index a7c28880..78e6ecb9 100644 --- a/clif/testing/python/top_level_pass_test.py +++ b/clif/testing/python/top_level_pass_test.py @@ -22,7 +22,7 @@ class TopLevelPassTest(absltest.TestCase): def testEmptyModule(self): self.assertRegexpMatches( # pylint: disable=deprecated-method top_level_pass.__doc__, - 'CLIF-generated .*module for .*top_level_pass.clif') + 'CLIF-generated module for .*top_level_pass.clif') self.assertIn(top_level_pass.__pyclif_codegen_mode__, ('c_api', 'pybind11'))