Skip to content

Commit

Permalink
Add Github action workflows for new C++20 targets: Part 1.
Browse files Browse the repository at this point in the history
Unfortunately for presubmit to pass these have to already be submitted, so first I will add the new targets and then I will remove the legacy targets

This enables testing for gcc.

PiperOrigin-RevId: 711474571
  • Loading branch information
jwhpryor authored and copybara-github committed Jan 7, 2025
1 parent 2048800 commit 4f977b7
Show file tree
Hide file tree
Showing 89 changed files with 1,240 additions and 1,703 deletions.
43 changes: 41 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ permissions:
contents: read

jobs:
# These jobs are legacy but needed for CopyBara to pass tests.
ubuntu-latest-cpp17:
runs-on: ubuntu-latest
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,11 +30,49 @@ 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
steps:
- uses: actions/checkout@v4
- name: test
run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++20' --repo_env=CC=clang --test_output=errors ...

# These are the new jobs that will be migrated to.
ubuntu-latest-cpp17-clang:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: test
run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++17' --repo_env=CC=clang --test_output=errors //implementation/legacy/...

ubuntu-latest-cpp20-clang:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: test
run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++20' --repo_env=CC=clang --test_output=errors ...

# Commented out because for some reason Github's action runner seems to be
# forcing C++14 which I can't reproduce locally.
# ubuntu-latest-cpp20-gcc:
# runs-on: ubuntu-latest
# steps:
# - uses: actions/checkout@v4
# - name: test
# run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++20' --repo_env=CC=gcc --test_output=errors ... -- -//implementation/legacy/...

macos-latest-cpp17-clang:
runs-on: macos-latest
steps:
- uses: actions/checkout@v4
- name: test
run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++17' --repo_env=CC=clang --test_output=errors //implementation/legacy/...

macos-latest-cpp20-clang:
runs-on: macos-latest
steps:
- uses: actions/checkout@v4
- name: test
run: bazel test --cxxopt='-Werror' --cxxopt='-std=c++20' --repo_env=CC=clang --test_output=errors ...
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
11 changes: 4 additions & 7 deletions implementation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand Down Expand Up @@ -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",
Expand Down
13 changes: 6 additions & 7 deletions implementation/array_ref.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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) {
Expand Down Expand Up @@ -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)) {}
Expand Down
32 changes: 17 additions & 15 deletions implementation/array_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand All @@ -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;
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
Loading

0 comments on commit 4f977b7

Please sign in to comment.