Update some metafunctions for increased portability.

As the matrix of supported compilers an increasing number of compiler inconsistencies are unfortunately showing, both across clang and gcc, but also across platforms (mac and Linux) and versions (C++17 and C++20). gcc does not seem to permit template specialisations within class definitions so inner helper classes have been moved out of classes (despite the current form being cleaner). Also, gcc seems to take issue with using declarations that shadow types in the outer namespace, so they have been modified with a leading "_", e.g. "using _ExposedType = ...". #42 Tested: Compiled on Ubuntu and Mac platforms with C++20 using clang and gcc. PiperOrigin-RevId: 711515926
google · Jan 6, 2025 · 0ce1f86 · 0ce1f86
1 parent d855896
commit 0ce1f86
Show file tree

Hide file tree

Showing 121 changed files with 6,471 additions and 1,719 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,7 +15,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: test
-      run: bazel query --output=label 'kind("...", //...) except attr("tags", "cpp20", "//...")' | xargs bazel test --cxxopt='-Werror' --cxxopt='-std=c++17' --repo_env=CC=clang --test_output=errors 
+      run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++17' --repo_env=CC=clang --test_output=errors //implementation/legacy/...
 
   ubuntu-latest-cpp20:
     runs-on: ubuntu-latest
@@ -29,7 +29,7 @@ jobs:
     steps:
     - uses: actions/checkout@v4
     - name: test
-      run: bazel query --output=label 'kind("...", //...) except attr(tags, "cpp20", //...)' | xargs bazel test --cxxopt='-Werror' --cxxopt='-std=c++17' --repo_env=CC=clang --test_output=errors 
+      run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++17' --repo_env=CC=clang --test_output=errors //implementation/legacy/...
 
   macos-latest-cpp20:
     runs-on: macos-latest

diff --git a/.gitignore b/.gitignore
@@ -1 +1,2 @@
 bazel*
+.vscode
diff --git a/BUILD b/BUILD
@@ -82,6 +82,7 @@ cc_library(
         "//implementation/jni_helper:static_field_value",
         "//metaprogramming:corpus",
         "//metaprogramming:corpus_tag",
+        "//metaprogramming:string_literal",
     ],
 )
 

diff --git a/implementation/BUILD b/implementation/BUILD
@@ -41,21 +41,15 @@ cc_library(
     name = "array_ref",
     hdrs = ["array_ref.h"],
     deps = [
-        ":array",
         ":array_view",
-        ":class",
         ":class_ref",
-        ":default_class_loader",
         ":forward_declarations",
-        ":jni_type",
         ":local_object",
-        ":object_ref",
+        ":no_idx",
         ":promotion_mechanics_tags",
-        ":ref_base",
         "//:jni_dep",
         "//implementation/jni_helper:jni_array_helper",
         "//implementation/jni_helper:lifecycle",
-        "//implementation/jni_helper:lifecycle_object",
     ],
 )
 
@@ -186,14 +180,17 @@ cc_library(
     hdrs = ["class_loader_ref.h"],
     deps = [
         ":class_loader",
+        ":class_ref",
         ":default_class_loader",
         ":global_object",
         ":id",
         ":id_type",
         ":jni_type",
+        ":jvm",
         ":local_object",
         ":no_idx",
         ":promotion_mechanics",
+        ":promotion_mechanics_tags",
         "//:jni_dep",
         "//class_defs:java_lang_classes",
         "//implementation/jni_helper:jni_env",
@@ -923,8 +920,11 @@ cc_library(
         "//:jni_dep",
         "//implementation/jni_helper:lifecycle",
         "//metaprogramming:invocable_map",
+        "//metaprogramming:invocable_map_20",
         "//metaprogramming:queryable_map",
+        "//metaprogramming:queryable_map_20",
         "//metaprogramming:string_contains",
+        "//metaprogramming:string_literal",
     ],
 )
 
@@ -1071,7 +1071,7 @@ cc_test(
     deps = [
         "//:jni_bind",
         "//:jni_test",
-        "//metaprogramming:concatenate",
+        "//metaprogramming:type_to_type_map",
         "@googletest//:gtest_main",
     ],
 )
@@ -1237,9 +1237,11 @@ cc_library(
         ":method_selection",
         ":no_idx",
         "//:jni_dep",
-        "//implementation/jni_helper:invoke_static",
         "//metaprogramming:invocable_map",
+        "//metaprogramming:invocable_map_20",
         "//metaprogramming:queryable_map",
+        "//metaprogramming:queryable_map_20",
+        "//metaprogramming:string_literal",
     ],
 )
 

diff --git a/implementation/array_ref.h b/implementation/array_ref.h
@@ -19,14 +19,17 @@
 
 // IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"
 
+#include <atomic>
 #include <cstddef>
+#include <type_traits>
 
 #include "implementation/array_view.h"
 #include "implementation/class_ref.h"
 #include "implementation/forward_declarations.h"
 #include "implementation/jni_helper/jni_array_helper.h"
 #include "implementation/jni_helper/lifecycle.h"
 #include "implementation/local_object.h"
+#include "implementation/no_idx.h"
 #include "implementation/promotion_mechanics_tags.h"
 #include "jni_dep.h"
 
@@ -50,10 +53,6 @@ class ArrayRef : public ScopedArrayImpl<JniT> {
       : Base(AdoptLocal{},
              JniArrayHelper<SpanType, JniT::kRank>::NewArray(size)) {}
 
-  template <typename T>
-  ArrayRef(const ArrayViewHelper<T>& array_view_helper)
-      : Base(AdoptLocal{}, array_view_helper.val) {}
-
   explicit ArrayRef(int size) : ArrayRef(static_cast<std::size_t>(size)) {}
 
   ArrayView<SpanType, JniT::kRank> Pin(bool copy_on_completion = true) {
@@ -133,9 +132,9 @@ class ArrayRef<
   // e.g.
   //  LocalArray arr { 5, LocalObject<kClass> {args...} };
   //  LocalArray arr { 5, GlobalObject<kClass> {args...} };
-  template <template <const auto&, const auto&, const auto&>
-            class ObjectContainer,
-            const auto& class_v, const auto& class_loader_v, const auto& jvm_v>
+  template <
+      template <const auto&, const auto&, const auto&> class ObjectContainer,
+      const auto& class_v, const auto& class_loader_v, const auto& jvm_v>
   ArrayRef(std::size_t size,
            const ObjectContainer<class_v, class_loader_v, jvm_v>& obj)
       : ArrayRef(size, static_cast<jobject>(obj)) {}

diff --git a/implementation/array_test.cc b/implementation/array_test.cc
@@ -16,10 +16,7 @@
 
 #include <type_traits>
 
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
 #include "jni_bind.h"
-#include "jni_test.h"
 
 namespace {
 

diff --git a/implementation/array_type_conversion_test.cc b/implementation/array_type_conversion_test.cc
@@ -14,8 +14,8 @@
  * limitations under the License.
  */
 
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
+#include <type_traits>
+
 #include "jni_bind.h"
 
 namespace {

diff --git a/implementation/array_view.h b/implementation/array_view.h
@@ -121,6 +121,22 @@ class ArrayView {
   const std::size_t size_;
 };
 
+// Metafunction that returns the type after a single dereference.
+template <typename SpanType, std::size_t>
+struct PinHelper {
+  using type = jobjectArray;
+};
+
+template <typename SpanType>
+struct PinHelper<SpanType, 2> {
+  using type = RegularToArrayTypeMap_t<SpanType>;
+};
+
+template <typename SpanType>
+struct PinHelper<SpanType, 1> {
+  using type = jobject;
+};
+
 // Object arrays, or arrays with rank > 1 (which are object arrays), or strings.
 template <typename SpanType_, std::size_t kRank>
 class ArrayView<
@@ -130,21 +146,7 @@ class ArrayView<
  public:
   using SpanType = SpanType_;
 
-  // Metafunction that returns the type after a single dereference.
-  template <std::size_t>
-  struct PinHelper {
-    using type = jobjectArray;
-  };
-  template <>
-  struct PinHelper<2> {
-    using type = RegularToArrayTypeMap_t<SpanType>;
-  };
-  template <>
-  struct PinHelper<1> {
-    using type = jobject;
-  };
-
-  using PinHelper_t = typename PinHelper<kRank>::type;
+  using PinHelper_t = typename PinHelper<SpanType_, kRank>::type;
 
   struct Iterator {
     using iterator_category = std::random_access_iterator_tag;

diff --git a/implementation/array_view_test.cc b/implementation/array_view_test.cc
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 #include <algorithm>
+#include <array>
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -50,51 +51,43 @@ TEST_F(JniTest, ArrayView_CallsLengthProperly) {
 TEST_F(JniTest, ArrayView_GetsAndReleaseArrayBuffer) {
   EXPECT_CALL(*env_, GetBooleanArrayElements(Eq(Fake<jbooleanArray>()), _))
       .WillOnce(::testing::Return(Fake<jboolean*>()));
-  EXPECT_CALL(*env_, ReleaseBooleanArrayElements(
-                         Eq(Fake<jbooleanArray>()),
-                         Eq(Fake<jboolean*>()), 0));
+  EXPECT_CALL(*env_, ReleaseBooleanArrayElements(Eq(Fake<jbooleanArray>()),
+                                                 Eq(Fake<jboolean*>()), 0));
 
   EXPECT_CALL(*env_, GetByteArrayElements(Eq(Fake<jbyteArray>()), _))
       .WillOnce(::testing::Return(Fake<jbyte*>()));
-  EXPECT_CALL(*env_,
-              ReleaseByteArrayElements(Eq(Fake<jbyteArray>()),
-                                       Eq(Fake<jbyte*>()), 0));
+  EXPECT_CALL(*env_, ReleaseByteArrayElements(Eq(Fake<jbyteArray>()),
+                                              Eq(Fake<jbyte*>()), 0));
 
   EXPECT_CALL(*env_, GetCharArrayElements(Eq(Fake<jcharArray>()), _))
       .WillOnce(::testing::Return(Fake<jchar*>()));
-  EXPECT_CALL(*env_,
-              ReleaseCharArrayElements(Eq(Fake<jcharArray>()),
-                                       Eq(Fake<jchar*>()), 0));
+  EXPECT_CALL(*env_, ReleaseCharArrayElements(Eq(Fake<jcharArray>()),
+                                              Eq(Fake<jchar*>()), 0));
 
   EXPECT_CALL(*env_, GetShortArrayElements(Eq(Fake<jshortArray>()), _))
       .WillOnce(::testing::Return(Fake<jshort*>()));
-  EXPECT_CALL(
-      *env_, ReleaseShortArrayElements(Eq(Fake<jshortArray>()),
-                                       Eq(Fake<jshort*>()), 0));
+  EXPECT_CALL(*env_, ReleaseShortArrayElements(Eq(Fake<jshortArray>()),
+                                               Eq(Fake<jshort*>()), 0));
 
   EXPECT_CALL(*env_, GetIntArrayElements(Eq(Fake<jintArray>()), _))
       .WillOnce(::testing::Return(Fake<jint*>()));
-  EXPECT_CALL(*env_,
-              ReleaseIntArrayElements(Eq(Fake<jintArray>()),
-                                      Eq(Fake<jint*>()), 0));
+  EXPECT_CALL(*env_, ReleaseIntArrayElements(Eq(Fake<jintArray>()),
+                                             Eq(Fake<jint*>()), 0));
 
   EXPECT_CALL(*env_, GetLongArrayElements(Eq(Fake<jlongArray>()), _))
       .WillOnce(::testing::Return(Fake<jlong*>()));
-  EXPECT_CALL(*env_,
-              ReleaseLongArrayElements(Eq(Fake<jlongArray>()),
-                                       Eq(Fake<jlong*>()), 0));
+  EXPECT_CALL(*env_, ReleaseLongArrayElements(Eq(Fake<jlongArray>()),
+                                              Eq(Fake<jlong*>()), 0));
 
   EXPECT_CALL(*env_, GetFloatArrayElements(Eq(Fake<jfloatArray>()), _))
       .WillOnce(::testing::Return(Fake<jfloat*>()));
-  EXPECT_CALL(
-      *env_, ReleaseFloatArrayElements(Eq(Fake<jfloatArray>()),
-                                       Eq(Fake<jfloat*>()), 0));
+  EXPECT_CALL(*env_, ReleaseFloatArrayElements(Eq(Fake<jfloatArray>()),
+                                               Eq(Fake<jfloat*>()), 0));
 
   EXPECT_CALL(*env_, GetDoubleArrayElements(Eq(Fake<jdoubleArray>()), _))
       .WillOnce(::testing::Return(Fake<jdouble*>()));
-  EXPECT_CALL(*env_, ReleaseDoubleArrayElements(
-                         Eq(Fake<jdoubleArray>()),
-                         Eq(Fake<jdouble*>()), 0));
+  EXPECT_CALL(*env_, ReleaseDoubleArrayElements(Eq(Fake<jdoubleArray>()),
+                                                Eq(Fake<jdouble*>()), 0));
 
   LocalArray<jboolean> boolean_array{AdoptLocal{}, Fake<jbooleanArray>()};
   LocalArray<jbyte> byte_array{AdoptLocal{}, Fake<jbyteArray>()};
@@ -118,9 +111,8 @@ TEST_F(JniTest, ArrayView_GetsAndReleaseArrayBuffer) {
 TEST_F(JniTest, LocalArrayView_AllowsCTAD) {
   EXPECT_CALL(*env_, GetBooleanArrayElements(Eq(Fake<jbooleanArray>()), _))
       .WillOnce(::testing::Return(Fake<jboolean*>()));
-  EXPECT_CALL(*env_, ReleaseBooleanArrayElements(
-                         Eq(Fake<jbooleanArray>()),
-                         Eq(Fake<jboolean*>()), 0));
+  EXPECT_CALL(*env_, ReleaseBooleanArrayElements(Eq(Fake<jbooleanArray>()),
+                                                 Eq(Fake<jboolean*>()), 0));
 
   LocalArray<jboolean> boolean_array{AdoptLocal{}, Fake<jbooleanArray>()};
   ArrayView ctad_array_view{boolean_array.Pin()};
@@ -326,7 +318,7 @@ TEST_F(JniTest, ArrayView_RichObjectsAreIterable) {
   int fake_result = 123;
   for (LocalObject<kClass> obj : obj_view) {
     EXPECT_CALL(*env_, CallIntMethodV).WillOnce(::testing::Return(fake_result));
-    EXPECT_EQ(obj("Foo"), fake_result);
+    EXPECT_EQ(obj.Call<"Foo">(), fake_result);
     fake_result++;
   }
 }

diff --git a/implementation/class.h b/implementation/class.h
@@ -36,7 +36,10 @@ namespace jni {
 
 template <typename Extends_, typename Constructors_, typename Static_,
           typename Methods_, typename Fields_>
-struct Class {};
+struct Class {
+  constexpr Class() = default;
+  constexpr Class(const char* name) {}
+};
 
 template <typename Extends_, typename... Constructors_,
           typename... StaticMethods_, typename... StaticFields_,
@@ -70,6 +73,14 @@ struct Class<Extends_, std::tuple<Constructors_...>,
   // provided where they are and aren't present.
   ////////////////////////////////////////////////////////////////////////////////
 
+  // To stifle a test failure.
+  constexpr Class()
+      : Object("__JNI_BIND__NO_CLASS__"),
+        constructors_(Constructor<>{}),
+        static_(),
+        methods_(),
+        fields_() {}
+
   // Methods + Fields.
   explicit constexpr Class(const char* class_name, Methods_... methods,
                            Fields_... fields)

diff --git a/implementation/class_loader_ref.h b/implementation/class_loader_ref.h
@@ -21,6 +21,7 @@
 
 #include "class_defs/java_lang_classes.h"
 #include "implementation/class_loader.h"
+#include "implementation/class_ref.h"
 #include "implementation/default_class_loader.h"
 #include "implementation/global_object.h"
 #include "implementation/id.h"
@@ -29,9 +30,11 @@
 #include "implementation/jni_helper/lifecycle.h"
 #include "implementation/jni_helper/lifecycle_object.h"
 #include "implementation/jni_type.h"
+#include "implementation/jvm.h"
 #include "implementation/local_object.h"
 #include "implementation/no_idx.h"
 #include "implementation/promotion_mechanics.h"
+#include "implementation/promotion_mechanics_tags.h"
 #include "jni_dep.h"
 
 namespace jni {
@@ -73,10 +76,19 @@ class ClassLoaderRef : public ClassLoaderImpl<lifecycleType> {
                   kDefaultClassLoader) {
       ClassRef_t<JniT<jobject, class_v, class_loader_v_, jvm_v_,
                       0>>::PrimeJClassFromClassLoader([&]() {
-        // Prevent the object (which is a runtime instance of a class) from
-        // falling out of scope so it is not released.
+      // Prevent the object (which is a runtime instance of a class) from
+      // falling out of scope so it is not released.
+
+#if __cplusplus >= 202002L
+        LocalObject loaded_class =
+            (*this).template Call<"loadClass">(IdClassT::kNameUsingDots);
+#elif __clang__
         LocalObject loaded_class =
             (*this)("loadClass", IdClassT::kNameUsingDots);
+#else
+        static_assert(
+            false, "JNI Bind requires C++20 (or later) or C++17 with clang.");
+#endif
 
         // We only want to create global references if we are actually going
         // to use them so that they do not leak.

diff --git a/implementation/class_loader_ref_second_test.cc b/implementation/class_loader_ref_second_test.cc
@@ -91,7 +91,7 @@ TEST_F(JniTestWithNoDefaultJvmRef,
   auto second_custom_loader_object =
       class_loader.BuildLocalObject<kClass>(jint{2});
 
-  EXPECT_EQ(custom_loader_object("methodNoCrossTalk", jint{2}), 123);
+  EXPECT_EQ(custom_loader_object.Call<"methodNoCrossTalk">(jint{2}), 123);
 
   TearDown();
 }