Skip to content

Commit

Permalink
Update some metafunctions for increased portability.
Browse files Browse the repository at this point in the history
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).

`InvocableMap`s have been updated not to have derived non-type template parameters, so they're now just passed as explicit types. This is objectively absurd, as what I wrote is valid C+++ and gcc should know better, but it just straight up segfaults so better luck next time gcc.

Also removed `conjunction`, `lambda_compatible`, and `type_map` which depended on the full form or needed clang Since they are not used I will just remove them.

#42

Tested: Compiled on Ubuntu and Mac platforms with C++20 using clang and gcc.
PiperOrigin-RevId: 711515926
  • Loading branch information
jwhpryor authored and copybara-github committed Jan 3, 2025
1 parent d855896 commit d32baf2
Show file tree
Hide file tree
Showing 105 changed files with 6,207 additions and 1,547 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
bazel*
.vscode
1 change: 1 addition & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ cc_library(
"//implementation/jni_helper:static_field_value",
"//metaprogramming:corpus",
"//metaprogramming:corpus_tag",
"//metaprogramming:string_literal",
],
)

Expand Down
12 changes: 10 additions & 2 deletions implementation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -186,14 +186,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",
Expand Down Expand Up @@ -923,8 +926,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",
],
)

Expand Down Expand Up @@ -1071,7 +1077,7 @@ cc_test(
deps = [
"//:jni_bind",
"//:jni_test",
"//metaprogramming:concatenate",
"//metaprogramming:type_to_type_map",
"@googletest//:gtest_main",
],
)
Expand Down Expand Up @@ -1237,9 +1243,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",
],
)

Expand Down
3 changes: 0 additions & 3 deletions implementation/array_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@

#include <type_traits>

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "jni_bind.h"
#include "jni_test.h"

namespace {

Expand Down
4 changes: 2 additions & 2 deletions implementation/array_type_conversion_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* limitations under the License.
*/

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <type_traits>

#include "jni_bind.h"

namespace {
Expand Down
48 changes: 20 additions & 28 deletions implementation/array_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
#include <algorithm>
#include <array>

#include <gmock/gmock.h>
#include <gtest/gtest.h>
Expand Down Expand Up @@ -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>()};
Expand All @@ -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()};
Expand Down Expand Up @@ -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++;
}
}
Expand Down
13 changes: 12 additions & 1 deletion implementation/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_,
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions implementation/class_loader_ref.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion implementation/class_loader_ref_second_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
17 changes: 8 additions & 9 deletions implementation/class_loader_ref_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <optional>
#include <utility>

#include <gmock/gmock.h>
Expand Down Expand Up @@ -119,7 +118,7 @@ TEST_F(JniTest, LocalObject_SupportsPassingAnObjectWithAClassLoader) {
// LocalObject<kClass2, kClassLoader> a{}; // doesn't compile (good).
LocalObject<kClass1, kClassLoader> a{Fake<jobject>()};
LocalObject<kClass2> b{};
b("Foo", a);
b.Call<"Foo">(a);

default_globals_made_that_should_be_released_.clear();
}
Expand All @@ -131,7 +130,7 @@ TEST_F(JniTestForClassLoaders,
LocalClassLoader<kClassLoader> local_class_loader{Fake<jobject>()};
auto a = local_class_loader.BuildLocalObject<kClass1>();
LocalObject<kClass2> b{a};
b("Foo", a);
b.Call<"Foo">(a);

default_globals_made_that_should_be_released_.clear();
TearDown();
Expand All @@ -144,7 +143,7 @@ TEST_F(JniTestForClassLoaders,
LocalClassLoader<kClassLoader> local_class_loader{Fake<jobject>()};
auto a = local_class_loader.BuildGlobalObject<kClass1>();
LocalObject<kClass2> b{a};
b("Foo", a);
b.Call<"Foo">(a);

default_globals_made_that_should_be_released_.clear();
TearDown();
Expand All @@ -161,7 +160,7 @@ TEST_F(JniTestForClassLoaders,
LocalObject<kClass1, kClassLoader> a =
local_class_loader.BuildLocalObject<kClass1>();
LocalObject<kClass2> b{a};
b("Foo", a);
b.Call<"Foo">(a);

TearDown();
}
Expand All @@ -173,7 +172,7 @@ TEST_F(JniTestForClassLoaders, ClassLoaderRefTest_ConstructsFromRValue) {
LocalObject<kClass1, kClassLoader, kJvm> b{
local_class_loader.BuildLocalObject<kClass1>()};

LocalObject<kClass2, kClassLoader, kJvm> c{b("Foo")};
LocalObject<kClass2, kClassLoader, kJvm> c{b.Call<"Foo">()};

TearDown();
}
Expand All @@ -187,7 +186,7 @@ TEST_F(JniTestForClassLoaders,
LocalObject<kClass1> a{};
LocalObject<kClass2, kClassLoader, kJvm> b =
local_class_loader.BuildLocalObject<kClass2>(a);
b("Foo", a);
b.Call<"Foo">(a);

TearDown();
}
Expand Down Expand Up @@ -236,7 +235,7 @@ TEST_F(JniTestWithNoDefaultJvmRef,
LocalObject<kClass1, kClassLoader, kJvm> a =
local_class_loader.BuildLocalObject<kClass1>();
LocalObject<kClass2> b{};
b("Foo", a);
b.Call<"Foo">(a);

TearDown();
}
Expand Down Expand Up @@ -310,7 +309,7 @@ TEST_F(JniTestWithNoDefaultJvmRef,
kDefaultConfiguration};
jni::LocalObject<class_under_test, class_loader, atypical_jvm_definition>
obj1{AdoptLocal{}, Fake<jobject>(1)};
obj1("Foo");
obj1.Call<"Foo">();

this->TearDown();
}
Expand Down
2 changes: 0 additions & 2 deletions implementation/class_loader_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <gtest/gtest.h>
#include "jni_bind.h"
#include "jni_test.h"

namespace {

Expand Down
1 change: 0 additions & 1 deletion implementation/class_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "jni_bind.h"
#include "jni_test.h"
Expand Down
1 change: 1 addition & 0 deletions implementation/constructor_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "jni_bind.h"
#include "jni_test.h"
Expand Down
Loading

0 comments on commit d32baf2

Please sign in to comment.