manual presubmit testing

clif/backend/BUILD

@@ -73,6 +73,7 @@ cc_library(
     name = "strutil",
     hdrs = ["strutil.h"],
     deps = [
+        "@com_google_absl//absl/strings",
         "@llvm-project//llvm:Support",
     ],
 )

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_;
 }

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::CXXMethodDecl>(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::NamedDecl>(clang_decl)) {

clif/backend/strutil.h

@@ -19,6 +19,7 @@
 #include <string>
 #include <vector>
 
+#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 <typename ProtoType>
+std::string ProtoDebugString(const ProtoType& pb) {
+  return pb.DebugString();
+}
+
 }  // namespace clif
 
 #endif  // CLIF_BACKEND_STRUTIL_H_

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:

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:

clif/pybind11/type_casters.h

@@ -31,48 +31,13 @@
 #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"
 
 namespace pybind11 {
 namespace detail {
 
-namespace type_caster_std_function_specializations {
-
-template <typename... Args>
-struct func_wrapper<absl::Status, Args...> : 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>(args)...)
-          .template cast<absl::Status>();
-    } catch (error_already_set &e) {
-      return util_task_python_clif::StatusFromErrorAlreadySet(e);
-    }
-  }
-};
-
-template <typename PayloadType, typename... Args>
-struct func_wrapper<absl::StatusOr<PayloadType>, Args...> : func_wrapper_base {
-  using func_wrapper_base::func_wrapper_base;
-  absl::StatusOr<PayloadType> operator()(Args... args) const {
-    gil_scoped_acquire acq;
-    try {
-      return hfunc.f.call_with_policies(rvpp, std::forward<Args>(args)...)
-          .template cast<absl::StatusOr<PayloadType>>();
-    } 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<absl::int128> {
  public:

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<int>& values);
+std::unordered_set<int> return_unordered_set_int();
+
+void pass_set_int(const std::set<int>& values);
+std::set<int> return_set_int();
+```
+
+.clif:
+
+```python
+def pass_unordered_set_int(values: set<int>)
+def return_unordered_set_int() -> set<int>
+
+def pass_set_int(values: `std::set` as set<int>)
+def return_set_int() -> `std::set` as set<int>
+```
+
+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<std::list<int>>& clusters);
+```
+
+.clif:
+
+```python
+def pass_set_list_int(clusters: `std::set` as set<`std::list` as list<int>>)
+```
+
+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<R(T, U)>` | `(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>) -> `float` as float
-```
-
-for `float fsum(std::list<float>);` 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
 

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 <Python.h>
+
+#include <cassert>
+#include <string>
+#include <unordered_map>
+
+namespace clif_testing::class_release_gil_before_calling_cpp_dtor {
+
+using RegistryType = std::unordered_map<std::string, int>;
+
+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 &reg = 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 &reg = 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_

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)

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