Skip to content

Commit 6c2b520

Browse files
jwhpryorjwhpryor
committed
Proxying now returns RefBase objects.
Thrash test.
1 parent eefd622 commit 6c2b520

File tree

16 files changed

+7875
-131
lines changed

16 files changed

+7875
-131
lines changed

‎implementation/BUILD‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ cc_library(
294294
name = "field_ref",
295295
hdrs = ["field_ref.h"],
296296
deps = [
297+
":proxy_temporary",
297298
":default_class_loader",
298299
":field_selection",
299300
":id",
@@ -788,6 +789,7 @@ cc_library(
788789
name = "method_ref",
789790
hdrs = ["overload_ref.h"],
790791
deps = [
792+
":proxy_temporary",
791793
":configuration",
792794
":id_type",
793795
":promotion_mechanics_tags",
@@ -1052,6 +1054,7 @@ cc_library(
10521054
name = "proxy_definitions_string",
10531055
hdrs = ["proxy_definitions_string.h"],
10541056
deps = [
1057+
":proxy_temporary",
10551058
":default_class_loader",
10561059
":forward_declarations",
10571060
":jvm",
@@ -1076,6 +1079,14 @@ cc_test(
10761079
],
10771080
)
10781081

1082+
################################################################################
1083+
# ProxyTemporary.
1084+
################################################################################
1085+
cc_library(
1086+
name = "proxy_temporary",
1087+
hdrs = ["proxy_temporary.h"],
1088+
)
1089+
10791090
################################################################################
10801091
# RefBase.
10811092
################################################################################

‎implementation/field_ref.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@
2222
#include <cstddef>
2323
#include <type_traits>
2424
#include <utility>
25+
#include <utility>
2526
#include <vector>
2627

28+
#include "implementation/proxy_temporary.h"
2729
#include "implementation/default_class_loader.h"
2830
#include "implementation/field_selection.h"
2931
#include "implementation/id.h"
@@ -115,8 +117,9 @@ class FieldRef {
115117
void Set(T&& value) {
116118
FieldHelper<CDecl_t<typename IdT::RawValT>, IdT::kRank,
117119
IdT::kIsStatic>::SetValue(SelfVal(), GetFieldID(class_ref_),
118-
Proxy_t<T>::ProxyAsArg(
119-
std::forward<T>(value)));
120+
ForwardWithRefStrip(
121+
Proxy_t<T>::ProxyAsArg(
122+
std::forward<T>(value))));
120123
}
121124

122125
private:

‎implementation/jni_helper/BUILD‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,6 @@ cc_test(
2626
],
2727
)
2828

29-
################################################################################
30-
# DryRun.
31-
################################################################################
32-
cc_test(
33-
name = "dry_run_test",
34-
srcs = ["dry_run_test.cc"],
35-
defines = [
36-
"DRY_RUN",
37-
"ENABLE_DEBUG_OUTPUT",
38-
],
39-
deps = [
40-
"//:jni_bind",
41-
"@googletest//:gtest_main",
42-
],
43-
)
44-
4529
################################################################################
4630
# Fake Test Constants.
4731
################################################################################

‎implementation/jni_helper/dry_run_test.cc‎

Lines changed: 0 additions & 26 deletions
This file was deleted.

‎implementation/jni_helper/field_value.h‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ struct FieldHelper<jboolean, 0, false, void> {
5353
}
5454

5555
static inline void SetValue(const jobject object_ref,
56-
const jfieldID field_ref_, jboolean&& value) {
56+
const jfieldID field_ref_, jboolean value) {
5757
Trace(metaprogramming::LambdaToStr(STR("GetBooleanValue")), object_ref,
5858
field_ref_);
5959

@@ -79,7 +79,7 @@ struct FieldHelper<jbyte, 0, false, void> {
7979
}
8080

8181
static inline void SetValue(const jobject object_ref,
82-
const jfieldID field_ref_, jbyte&& value) {
82+
const jfieldID field_ref_, jbyte value) {
8383
Trace(metaprogramming::LambdaToStr(STR("SetByteValue")), object_ref,
8484
field_ref_);
8585

@@ -105,7 +105,7 @@ struct FieldHelper<jchar, 0, false, void> {
105105
}
106106

107107
static inline void SetValue(const jobject object_ref,
108-
const jfieldID field_ref_, jchar&& value) {
108+
const jfieldID field_ref_, jchar value) {
109109
Trace(metaprogramming::LambdaToStr(STR("SetCharValue")), object_ref,
110110
field_ref_);
111111

@@ -131,7 +131,7 @@ struct FieldHelper<jshort, 0, false, void> {
131131
}
132132

133133
static inline void SetValue(const jobject object_ref,
134-
const jfieldID field_ref_, jshort&& value) {
134+
const jfieldID field_ref_, jshort value) {
135135
Trace(metaprogramming::LambdaToStr(STR("SetShortValue")), object_ref,
136136
field_ref_);
137137

@@ -157,7 +157,7 @@ struct FieldHelper<jint, 0, false, void> {
157157
}
158158

159159
static inline void SetValue(const jobject object_ref,
160-
const jfieldID field_ref_, jint&& value) {
160+
const jfieldID field_ref_, jint value) {
161161
Trace(metaprogramming::LambdaToStr(STR("SetIntValue")), object_ref,
162162
field_ref_);
163163

@@ -183,7 +183,7 @@ struct FieldHelper<jlong, 0, false, void> {
183183
}
184184

185185
static inline void SetValue(const jobject object_ref,
186-
const jfieldID field_ref_, jlong&& value) {
186+
const jfieldID field_ref_, jlong value) {
187187
Trace(metaprogramming::LambdaToStr(STR("SetLongField")), object_ref,
188188
field_ref_);
189189

@@ -209,7 +209,7 @@ struct FieldHelper<jfloat, 0, false, void> {
209209
}
210210

211211
static inline void SetValue(const jobject object_ref,
212-
const jfieldID field_ref_, jfloat&& value) {
212+
const jfieldID field_ref_, jfloat value) {
213213
Trace(metaprogramming::LambdaToStr(STR("SetFloatField")), object_ref,
214214
field_ref_);
215215

@@ -235,7 +235,7 @@ struct FieldHelper<jdouble, 0, false, void> {
235235
}
236236

237237
static inline void SetValue(const jobject object_ref,
238-
const jfieldID field_ref_, jdouble&& value) {
238+
const jfieldID field_ref_, jdouble value) {
239239
Trace(metaprogramming::LambdaToStr(STR("SetDoubleField")), object_ref,
240240
field_ref_);
241241

@@ -261,7 +261,7 @@ struct FieldHelper<jobject, 0, false, void> {
261261
}
262262

263263
static inline void SetValue(const jobject object_ref,
264-
const jfieldID field_ref_, jobject&& new_value) {
264+
const jfieldID field_ref_, jobject new_value) {
265265
Trace(metaprogramming::LambdaToStr(STR("SetObjectField")), object_ref,
266266
field_ref_);
267267

@@ -288,7 +288,7 @@ struct FieldHelper<jstring, 0, false, void> {
288288
}
289289

290290
static inline void SetValue(const jobject object_ref,
291-
const jfieldID field_ref_, jstring&& new_value) {
291+
const jfieldID field_ref_, jstring new_value) {
292292
Trace(metaprogramming::LambdaToStr(STR("SetObjectField")), object_ref,
293293
field_ref_);
294294

‎implementation/legacy/local_array_string_test.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,8 @@ TEST_F(JniTest, Array_LocalVanillaObjectRValuesCanBeSet) {
109109
// In the future, this should drop to 1.
110110
EXPECT_CALL(*env_, FindClass(StrEq("java/lang/String"))).Times(2);
111111

112-
EXPECT_CALL(*env_, DeleteLocalRef(_)).Times(2); // array, in place obj
112+
EXPECT_CALL(*env_, DeleteLocalRef(_)).Times(3); // array, in place obj
113113
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jclass>())).Times(2); // FindClass
114-
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jstring>())).Times(0);
115114

116115
LocalArray<jobject, 1, kJavaLangString> arr{
117116
3, LocalObject<kJavaLangString>{"Foo"}};

‎implementation/legacy/string_ref_test.cc‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,10 @@ TEST_F(JniTest, LocalString_CreatesFromStringView) {
111111

112112
// jclass for temp String class reference.
113113
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jclass>()));
114+
// Temporary xref created during construction from NewStringUTF.
115+
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jstring>()));
114116
// The variable str (which is itself an object).
115117
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jobject>()));
116-
// TODO(b/143908983): Currently strings leak one local during proxying.
117-
// Temporary xref created during construction.
118-
// EXPECT_CALL(*env_, DeleteLocalRef(Fake<jstring>()));
119118

120119
LocalString str{std::string_view{char_ptr}};
121120
}
@@ -126,11 +125,11 @@ TEST_F(JniTest, LocalString_CreatesFromString) {
126125

127126
// jclass for temp String class reference.
128127
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jclass>()));
128+
// Temporary xref created during construction from NewStringUTF.
129+
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jstring>()));
129130
// The variable str (which is itself an object).
130131
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jobject>()));
131-
// TODO(b/143908983): Currently strings leak one local during proxying.
132-
// Temporary xref created during construction.
133-
// EXPECT_CALL(*env_, DeleteLocalRef(Fake<jstring>()));
132+
134133
LocalString str{std::string{"TestString"}};
135134
}
136135

‎implementation/local_array_string_test.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,8 @@ TEST_F(JniTest, Array_LocalVanillaObjectRValuesCanBeSet) {
107107
// In the future, this should drop to 1.
108108
EXPECT_CALL(*env_, FindClass(StrEq("java/lang/String"))).Times(2);
109109

110-
EXPECT_CALL(*env_, DeleteLocalRef(_)).Times(2); // array, in place obj
110+
EXPECT_CALL(*env_, DeleteLocalRef(_)).Times(3); // array, in place obj
111111
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jclass>())).Times(2); // FindClass
112-
EXPECT_CALL(*env_, DeleteLocalRef(Fake<jstring>())).Times(0);
113112

114113
LocalArray<jobject, 1, kJavaLangString> arr{
115114
3, LocalObject<kJavaLangString>{"Foo"}};

‎implementation/overload_ref.h‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@
4141
#include "jni_dep.h"
4242
#include "metaprogramming/double_locked_value.h"
4343
#include "metaprogramming/string_concatenate.h"
44+
#include "implementation/id.h"
4445

4546
namespace jni {
46-
4747
// Transforms a OverloadRef IdT into a fully qualified ID. Storage is keyed
4848
// against these IDs to reduce excess MethodID lookups.
4949
template <typename IdT>
@@ -75,18 +75,18 @@ struct OverloadRef {
7575
static jmethodID GetMethodID(jclass clazz) {
7676
static auto get_lambda =
7777
[clazz](metaprogramming::DoubleLockedValue<jmethodID>* storage) {
78-
if (kConfiguration.release_method_ids_on_teardown_) {
79-
DefaultRefs<jmethodID>().push_back(storage);
80-
}
81-
82-
if constexpr (IdT::kIsStatic) {
83-
return jni::JniHelper::GetStaticMethodID(clazz, IdT::Name(),
84-
Signature_v<IdT>.data());
85-
} else {
86-
return jni::JniHelper::GetMethodID(clazz, IdT::Name(),
87-
Signature_v<IdT>.data());
88-
}
89-
};
78+
if (kConfiguration.release_method_ids_on_teardown_) {
79+
DefaultRefs<jmethodID>().push_back(storage);
80+
}
81+
82+
if constexpr (IdT::kIsStatic) {
83+
return jni::JniHelper::GetStaticMethodID(clazz, IdT::Name(),
84+
Signature_v<IdT>.data());
85+
} else {
86+
return jni::JniHelper::GetMethodID(clazz, IdT::Name(),
87+
Signature_v<IdT>.data());
88+
}
89+
};
9090

9191
return RefStorage<decltype(get_lambda), OverloadRefUniqueId<IdT>>::Get(
9292
get_lambda);
@@ -100,27 +100,35 @@ struct OverloadRef {
100100
const jmethodID mthd = OverloadRef::GetMethodID(clazz);
101101

102102
if constexpr (std::is_same_v<ReturnProxied, void>) {
103-
return InvokeHelper<void, kRank, kStatic>::Invoke(
103+
InvokeHelper<void, kRank, kStatic>::Invoke(
104104
object, clazz, mthd,
105-
Proxy_t<Params>::ProxyAsArg(std::forward<Params>(params))...);
105+
ForwardWithRefStrip(Proxy_t<Params>::ProxyAsArg(
106+
std::forward<Params>(params)))...);
106107
} else if constexpr (IdT::kIsConstructor) {
107108
return ReturnProxied{
108109
AdoptLocal{},
109110
LifecycleHelper<jobject, LifecycleType::LOCAL>::Construct(
110111
clazz, mthd,
111-
Proxy_t<Params>::ProxyAsArg(std::forward<Params>(params))...)};
112+
ForwardWithRefStrip(
113+
Proxy_t<Params>::ProxyAsArg(
114+
std::forward<Params>(params)))...)};
112115
} else {
113116
if constexpr (std::is_base_of_v<RefBaseBase, ReturnProxied>) {
114-
return ReturnProxied{
115-
AdoptLocal{},
116-
InvokeHelper<typename ReturnIdT::CDecl, kRank, kStatic>::Invoke(
117-
object, clazz, mthd,
118-
Proxy_t<Params>::ProxyAsArg(std::forward<Params>(params))...)};
117+
return
118+
ReturnProxied{
119+
AdoptLocal{},
120+
InvokeHelper<typename ReturnIdT::CDecl, kRank, kStatic>::Invoke(
121+
object, clazz, mthd,
122+
ForwardWithRefStrip(
123+
Proxy_t<Params>::ProxyAsArg(std::forward<Params>(params)))
124+
...)};
119125
} else {
120126
return static_cast<ReturnProxied>(
121-
InvokeHelper<typename ReturnIdT::CDecl, kRank, kStatic>::Invoke(
122-
object, clazz, mthd,
123-
Proxy_t<Params>::ProxyAsArg(std::forward<Params>(params))...));
127+
InvokeHelper<typename ReturnIdT::CDecl, kRank, kStatic>::Invoke(
128+
object, clazz, mthd,
129+
ForwardWithRefStrip(
130+
Proxy_t<Params>::ProxyAsArg(
131+
std::forward<Params>(params)))...));
124132
}
125133
}
126134
}

0 commit comments

Comments
 (0)