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` and `lambda_compatible` which depended on the full form. Since they are not used I will just remove them.

PiperOrigin-RevId: 711515926
  • Loading branch information
jwhpryor authored and copybara-github committed Jan 3, 2025
1 parent d855896 commit 5a1ded0
Show file tree
Hide file tree
Showing 95 changed files with 6,072 additions and 1,226 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
9 changes: 7 additions & 2 deletions implementation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -923,8 +923,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 +1074,7 @@ cc_test(
deps = [
"//:jni_bind",
"//:jni_test",
"//metaprogramming:concatenate",
"//metaprogramming:type_to_type_map",
"@googletest//:gtest_main",
],
)
Expand Down Expand Up @@ -1237,9 +1240,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
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 5a1ded0

Please sign in to comment.