Skip to content

Commit

Permalink
Update test code to new C++20 Call and Access pattern.
Browse files Browse the repository at this point in the history
#42

These changes were largely made by agent with the following prompt: "Observe the changes I have made replacing uses of () and [] operator member methods, and moved them to .Call and .Access respectively.  Replicate those replacements throughout the file where needed. Don't add uses of Ptr() where they don't exist."

PiperOrigin-RevId: 713044294
  • Loading branch information
jwhpryor authored and copybara-github committed Jan 7, 2025
1 parent 2048800 commit ff297fb
Show file tree
Hide file tree
Showing 51 changed files with 881 additions and 805 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 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
3 changes: 3 additions & 0 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
2 changes: 1 addition & 1 deletion implementation/array_view_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -318,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
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
16 changes: 8 additions & 8 deletions implementation/class_loader_ref_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,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 @@ -130,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 @@ -143,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 @@ -160,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 @@ -172,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 @@ -186,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 @@ -235,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 @@ -309,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
47 changes: 24 additions & 23 deletions implementation/field_ref_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,78 +59,78 @@ TEST_F(JniTest, Field_SimpleGetAndSet) {
Field("SomeField", jint{})};

GlobalObject<java_class_under_test> obj{};
EXPECT_EQ(999, obj["SomeField"].Get());
obj["SomeField"].Set(123);
EXPECT_EQ(999, obj.Access<"SomeField">().Get());
obj.Access<"SomeField">().Set(123);
}

TEST_F(JniTest, Field_BooleanField) {
EXPECT_CALL(*env_, GetBooleanField);
EXPECT_CALL(*env_, SetBooleanField);

LocalObject<java_class_under_test> obj{};
obj["booleanField"].Get();
obj["booleanField"].Set(true);
obj.Access<"booleanField">().Get();
obj.Access<"booleanField">().Set(true);
}

TEST_F(JniTest, Field_ByteField) {
EXPECT_CALL(*env_, GetByteField);
EXPECT_CALL(*env_, SetByteField);

LocalObject<java_class_under_test> obj{};
obj["byteField"].Get();
obj["byteField"].Set(jbyte{123});
obj.Access<"byteField">().Get();
obj.Access<"byteField">().Set(jbyte{123});
}

TEST_F(JniTest, Field_CharField) {
EXPECT_CALL(*env_, GetCharField);
EXPECT_CALL(*env_, SetCharField);

LocalObject<java_class_under_test> obj{};
obj["charField"].Get();
obj["charField"].Set(jchar{'a'});
obj.Access<"charField">().Get();
obj.Access<"charField">().Set(jchar{'a'});
}

TEST_F(JniTest, Field_ShortField) {
EXPECT_CALL(*env_, GetShortField);
EXPECT_CALL(*env_, SetShortField);

LocalObject<java_class_under_test> obj{};
obj["shortField"].Get();
obj["shortField"].Set(jshort{123});
obj.Access<"shortField">().Get();
obj.Access<"shortField">().Set(jshort{123});
}

TEST_F(JniTest, Field_intField) {
EXPECT_CALL(*env_, GetIntField);
EXPECT_CALL(*env_, SetIntField);

LocalObject<java_class_under_test> obj{};
obj["intField"].Get();
obj["intField"].Set(123);
obj.Access<"intField">().Get();
obj.Access<"intField">().Set(123);
}

TEST_F(JniTest, Field_longField) {
EXPECT_CALL(*env_, GetLongField);
EXPECT_CALL(*env_, SetLongField);

LocalObject<java_class_under_test> obj{};
obj["longField"].Get();
obj["longField"].Set(123);
obj.Access<"longField">().Get();
obj.Access<"longField">().Set(123);
}

TEST_F(JniTest, Field_floatField) {
LocalObject<java_class_under_test> obj{};
EXPECT_CALL(*env_, GetFloatField);
EXPECT_CALL(*env_, SetFloatField);
obj["floatField"].Get();
obj["floatField"].Set(123.f);
obj.Access<"floatField">().Get();
obj.Access<"floatField">().Set(123.f);
}

TEST_F(JniTest, Field_doubleField) {
LocalObject<java_class_under_test> obj{};
EXPECT_CALL(*env_, GetDoubleField);
EXPECT_CALL(*env_, SetDoubleField);
obj["doubleField"].Get();
obj["doubleField"].Set(123.);
obj.Access<"doubleField">().Get();
obj.Access<"doubleField">().Set(123.);
}

TEST_F(JniTest, Field_ObjectGet) {
Expand All @@ -144,7 +144,7 @@ TEST_F(JniTest, Field_ObjectGet) {
static constexpr Class kClass2{"kClass2"};

LocalObject<kClass> obj{};
LocalObject<kClass2> obj2 = obj["classField"].Get();
LocalObject<kClass2> obj2 = obj.Access<"classField">().Get();
}

TEST_F(JniTest, Field_ObjectSet) {
Expand All @@ -161,10 +161,11 @@ TEST_F(JniTest, Field_ObjectSet) {

LocalObject<kClass> obj{};
LocalObject<kClass2> some_obj{AdoptLocal{}, Fake<jobject>()};
obj["classField"].Set(Fake<jobject>());
obj["classField"].Set(LocalObject<kClass2>{AdoptLocal{}, Fake<jobject>()});
obj["classField"].Set(some_obj);
obj["classField"].Set(std::move(some_obj));
obj.Access<"classField">().Set(Fake<jobject>());
obj.Access<"classField">().Set(
LocalObject<kClass2>{AdoptLocal{}, Fake<jobject>()});
obj.Access<"classField">().Set(some_obj);
obj.Access<"classField">().Set(std::move(some_obj));
}

} // namespace
8 changes: 8 additions & 0 deletions implementation/find_class_fallback.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,15 @@ inline jclass FindClassFallback(const char* class_name) {
GlobalClassLoader<kDefaultClassLoader> loader{AdoptGlobal{},
FallbackLoader()};

#if __cplusplus >= 202002L
jni::LocalObject loaded_class = loader.Call<"loadClass">(class_name);
#elif __clang__
jni::LocalObject loaded_class = loader("loadClass", class_name);
#else
static_assert(false,
"JNI Bind requires C++20 (or later) or C++17 with clang.");
#endif

jclass ret{static_cast<jclass>(static_cast<jobject>(loaded_class.Release()))};

loader.Release();
Expand Down
28 changes: 14 additions & 14 deletions implementation/global_object_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,15 @@ TEST_F(JniTest, GlobalObject_ValuesWorkAfterMoveConstructor) {
Method{"Foo", jni::Return<jint>{}, Params<jint>{}},
Field{"BarField", jint{}}};
GlobalObject<kClass> global_object_1{AdoptGlobal{}, Fake<jobject>(1)};
global_object_1("Foo", 1);
global_object_1("Foo", 2);
global_object_1["BarField"].Set(1);
global_object_1.Call<"Foo">(1);
global_object_1.Call<"Foo">(2);
global_object_1.Access<"BarField">().Set(1);

GlobalObject<kClass> global_object_2{std::move(global_object_1)};
global_object_2("Foo", 3);
global_object_2["BarField"].Set(2);
global_object_2["BarField"].Set(3);
global_object_2["BarField"].Set(4);
global_object_2.Call<"Foo">(3);
global_object_2.Access<"BarField">().Set(2);
global_object_2.Access<"BarField">().Set(3);
global_object_2.Access<"BarField">().Set(4);

GlobalObject<kClass> global_object_3{AdoptGlobal{}, Fake<jobject>(1)};
}
Expand Down Expand Up @@ -245,10 +245,10 @@ TEST_F(JniTest, GlobalObject_ObjectReturnsInstanceMethods) {
Params<jint, jfloat, jint, jfloat, jdouble>{}}};

GlobalObject<java_class_under_test> global_object{};
global_object("Foo", 1);
global_object("Baz", 2.f);
global_object(
"AMethodWithAReallyLongNameThatWouldPossiblyBeHardForTemplatesToHandle",
global_object.Call<"Foo">(1);
global_object.Call<"Baz">(2.f);
global_object.Call<
"AMethodWithAReallyLongNameThatWouldPossiblyBeHardForTemplatesToHandle">(
int{}, float{}, int{}, float{}, double{});
}

Expand All @@ -268,7 +268,7 @@ TEST_F(JniTest, GlobalObject_SupportsPassingAPrvalue) {

GlobalObject<kTestClass1> a{};
GlobalObject<kTestClass2> b{};
b("Foo", std::move(a));
b.Call<"Foo">(std::move(a));
}

TEST_F(JniTest, GlobalObjects_PromoteRValuesFromEmittedLValues) {
Expand All @@ -277,9 +277,9 @@ TEST_F(JniTest, GlobalObjects_PromoteRValuesFromEmittedLValues) {
"TestClass2", Method{"Foo", jni::Return{kClass1}, jni::Params{}}};

LocalObject<kClass2> b{};
GlobalObject<kClass1> a{b("Foo")};
GlobalObject<kClass1> a{b.Call<"Foo">()};

a = b("Foo");
a = b.Call<"Foo">();
}

} // namespace
9 changes: 9 additions & 0 deletions implementation/jvm_ref.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,17 @@ class JvmRef : public JvmRefBase {
// Sets a "fallback" loader for use when default Jvm classes fail to load.
// `host_object *must* be local and will *not* be released.
void SetFallbackClassLoaderFromJObject(jobject host_object) {
#if __cplusplus >= 202002L
SetFallbackClassLoader(LocalObject<kJavaLangObject>{host_object}
.Call<"getClass">()
.Call<"getClassLoader">());
#elif __clang__
SetFallbackClassLoader(LocalObject<kJavaLangObject>{host_object}(
"getClass")("getClassLoader"));
#else
static_assert(false,
"JNI Bind requires C++20 (or later) or C++17 with clang.");
#endif
}

private:
Expand Down
Loading

0 comments on commit ff297fb

Please sign in to comment.