Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing ambiguous equality operators for C++20 migration. #347

Merged
merged 1 commit into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions implementation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ cc_library(
deps = [
":array_type_conversion",
"//:jni_dep",
"//implementation/jni_helper:get_array_element_result",
"//implementation/jni_helper:jni_array_helper",
"//implementation/jni_helper:lifecycle",
],
Expand Down Expand Up @@ -296,23 +297,19 @@ cc_library(
name = "field_ref",
hdrs = ["field_ref.h"],
deps = [
":class_ref",
":default_class_loader",
":field_selection",
":id",
":id_type",
":no_idx",
":promotion_mechanics_tags",
":proxy",
":proxy_convenience_aliases",
":ref_base",
":signature",
"//:jni_dep",
"//implementation/jni_helper",
"//implementation/jni_helper:field_value_getter",
"//implementation/jni_helper:static_field_value",
"//metaprogramming:double_locked_value",
"//metaprogramming:optional_wrap",
"//metaprogramming:queryable_map",
],
)

Expand Down Expand Up @@ -365,10 +362,8 @@ cc_library(
name = "forward_declarations",
hdrs = ["forward_declarations.h"],
deps = [
":default_class_loader",
":id_type",
":jvm",
"//implementation/jni_helper:lifecycle_object",
"//implementation/jni_helper:lifecycle",
],
)

Expand Down Expand Up @@ -397,8 +392,10 @@ cc_library(
name = "global_object",
hdrs = ["global_object.h"],
deps = [
":default_class_loader",
":forward_declarations",
":jni_type",
":jvm",
":local_object",
":promotion_mechanics",
":ref_base",
Expand Down Expand Up @@ -724,6 +721,7 @@ cc_library(
":forward_declarations",
":jvm",
":promotion_mechanics_tags",
"//:jni_dep",
"//implementation/jni_helper:lifecycle",
],
)
Expand Down
82 changes: 58 additions & 24 deletions implementation/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ struct Array;
template <std::size_t kRank>
struct Rank {};

struct ArrayBase {};

////////////////////////////////////////////////////////////////////////////////
// Array Non-Object Implementation.
////////////////////////////////////////////////////////////////////////////////
template <typename RawType, std::size_t kRank>
struct ArrayNonObjectTypeImpl {
struct ArrayNonObjectTypeImpl : ArrayBase {
RawType raw_;

constexpr ArrayNonObjectTypeImpl(RawType raw) : raw_(raw) {}
Expand All @@ -52,21 +54,48 @@ struct ArrayNonObjectTypeImpl {
"JNI Error: Invalid array declaration, use Array { type{}, "
"Rank<kRank>{} }.");
}
};

template <typename RawTypeRhs, std::size_t kRankRhs>
constexpr bool operator==(const Array<RawTypeRhs, kRankRhs>& rhs) const {
if constexpr (std::is_same_v<RawType, RawTypeRhs>) {
return (raw_ == rhs.raw_);
}
return false;
}
template <typename T>
struct ArrayComparisonHelper {};

template <typename RawTypeRhs, std::size_t kRankRhs>
constexpr bool operator!=(const Array<RawTypeRhs, kRankRhs>& rhs) const {
return !(*this == rhs);
}
template <typename RawType, std::size_t kRank>
struct ArrayComparisonHelper<Array<ArrayNonObjectTypeImpl<RawType, kRank>>> {
using type = RawType;
};

template <typename RawType, std::size_t kRank>
struct ArrayComparisonHelper<Array<RawType, kRank>> {
using type = RawType;
};

template <typename T>
using ArrayComparisonHelper_t = typename ArrayComparisonHelper<T>::type;

template <typename T1, typename T2>
static constexpr bool IsArrayComparable() {
return std::is_base_of_v<ArrayBase, T1> && std::is_base_of_v<ArrayBase, T2>;
};

////////////////////////////////////////////////////////////////////////////////
// Equality operators.
////////////////////////////////////////////////////////////////////////////////
template <typename T1, typename T2>
constexpr std::enable_if_t<IsArrayComparable<T1, T2>(), bool> operator==(
const T1& lhs, const T2& rhs) {
if constexpr (std::is_same_v<ArrayComparisonHelper_t<T1>,
ArrayComparisonHelper_t<T2>>) {
return (lhs.raw_ == rhs.raw_);
}
return false;
}

template <typename T1, typename T2>
constexpr std::enable_if_t<IsArrayComparable<T1, T2>(), bool> operator!=(
const T1& lhs, const T2& rhs) {
return !(lhs == rhs);
}

// Primitive array implementaiton.
template <typename T, std::size_t kRank, bool HoldsObject>
struct ArrayImpl : public ArrayNonObjectTypeImpl<T, kRank>,
Expand All @@ -79,27 +108,32 @@ struct ArrayImpl : public ArrayNonObjectTypeImpl<T, kRank>,
// Array Object Implementation.
////////////////////////////////////////////////////////////////////////////////
template <typename RawType, std::size_t kRank_>
struct ArrayImpl<RawType, kRank_, true> : public ArrayTag<jobjectArray> {
struct ArrayImpl<RawType, kRank_, true> : public ArrayTag<jobjectArray>,
ArrayBase {
RawType raw_;

constexpr ArrayImpl(RawType raw) : raw_(raw) {}

template <std::size_t kRank>
constexpr ArrayImpl(RawType raw, Rank<kRank>) : raw_(raw) {}
};

template <typename RawTypeRhs, std::size_t kRank>
constexpr bool operator==(const Array<RawTypeRhs, kRank>& rhs) const {
if constexpr (std::is_same_v<RawType, RawTypeRhs>) {
return (raw_ == rhs.raw_);
}
return false;
template <typename RawTypeLhs, std::size_t kRankLhs, typename RawTypeRhs,
std::size_t kRankRhs>
constexpr bool operator==(const ArrayImpl<RawTypeLhs, kRankLhs, true>& lhs,
const ArrayImpl<RawTypeRhs, kRankRhs, true>& rhs) {
if constexpr (std::is_same_v<RawTypeLhs, RawTypeRhs>) {
return (lhs.raw_ == rhs.raw_);
}
return false;
}

template <typename RawTypeRhs, std::size_t kRank>
constexpr bool operator!=(const Array<RawTypeRhs, kRank>& rhs) const {
return !(*this == rhs);
}
};
template <typename RawTypeLhs, std::size_t kRankLhs, typename RawTypeRhs,
std::size_t kRankRhs>
constexpr bool operator!=(const ArrayImpl<RawTypeLhs, kRankLhs, true>& lhs,
const Array<RawTypeRhs, kRankRhs>& rhs) {
return !(lhs == rhs);
}

// This type correlates to those used in declarations,
// e.g. Field { Array { Array { jint {} } } }.
Expand Down
3 changes: 2 additions & 1 deletion implementation/array_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

#include <type_traits>

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include "jni_bind.h"
Expand Down Expand Up @@ -108,5 +110,4 @@ static_assert(
static_assert(FullArrayStripV(arr8) == kClass);
static_assert(FullArrayStripV(arr9) == kClass);


} // namespace
21 changes: 15 additions & 6 deletions implementation/array_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,26 +21,33 @@

#include <cstddef>
#include <iterator>
#include <type_traits>

#include "implementation/array_type_conversion.h"
#include "implementation/jni_helper/get_array_element_result.h"
#include "implementation/jni_helper/jni_array_helper.h"
#include "implementation/jni_helper/lifecycle.h"
#include "jni_dep.h"

namespace jni {

struct ArrayViewHelperBase {};

template <typename T>
struct ArrayViewHelper {
struct ArrayViewHelper : ArrayViewHelperBase {
using SpanType = T;
const T val_;
operator T() const { return val_; }

ArrayViewHelper(const T& val) : val_(val) {}
};

// Primitive Rank 1 Arrays.
template <typename SpanType, std::size_t kRank = 1, typename Enable = void>
template <typename SpanType_, std::size_t kRank = 1, typename Enable = void>
class ArrayView {
public:
using SpanType = SpanType_;

struct Iterator {
using iterator_category = std::random_access_iterator_tag;
using difference_type = std::size_t;
Expand Down Expand Up @@ -115,12 +122,14 @@ class ArrayView {
};

// Object arrays, or arrays with rank > 1 (which are object arrays), or strings.
template <typename SpanType, std::size_t kRank>
template <typename SpanType_, std::size_t kRank>
class ArrayView<
SpanType, kRank,
std::enable_if_t<(kRank > 1) || std::is_same_v<SpanType, jobject> ||
std::is_same_v<SpanType, jstring>>> {
SpanType_, kRank,
std::enable_if_t<(kRank > 1) || std::is_same_v<SpanType_, jobject> ||
std::is_same_v<SpanType_, jstring>>> {
public:
using SpanType = SpanType_;

// Metafunction that returns the type after a single dereference.
template <std::size_t>
struct PinHelper {
Expand Down
56 changes: 37 additions & 19 deletions implementation/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,27 +156,45 @@ struct Class<Extends_, std::tuple<Constructors_...>,
static_(statik),
methods_(methods...),
fields_(fields...) {}

////////////////////////////////////////////////////////////////////////////////
// Equality operators.
////////////////////////////////////////////////////////////////////////////////
template <typename ParentClass, typename... Params, typename... Constructors,
typename... StaticMethods, typename... StaticFields,
typename... Fields, typename... Methods>
constexpr bool operator==(
const Class<ParentClass, std::tuple<Constructors...>,
std::tuple<Static<std::tuple<StaticMethods...>,
std::tuple<StaticFields...>>>,
std::tuple<Methods...>, std::tuple<Fields...>>& rhs) const {
// Don't compare the other parameters so classes can be used as parameters
// or return values before the class itself is defined.
return std::string_view(name_) == std::string_view(rhs.name_);
}

constexpr bool operator==(const NoClass&) const { return false; }
constexpr bool operator!=(const NoClass&) const { return false; }
};

////////////////////////////////////////////////////////////////////////////////
// Equality operators.
////////////////////////////////////////////////////////////////////////////////
template <typename LhsParentClass, typename... LhsParams,
typename... LhsConstructors, typename... LhsStaticMethods,
typename... LhsStaticFields, typename... LhsFields,
typename... LhsMethods, typename RhsParentClass,
typename... RhsParams, typename... RhsConstructors,
typename... RhsStaticMethods, typename... RhsStaticFields,
typename... RhsFields, typename... RhsMethods>
constexpr bool operator==(
const Class<LhsParentClass, std::tuple<LhsConstructors...>,
std::tuple<Static<std::tuple<LhsStaticMethods...>,
std::tuple<LhsStaticFields...>>>,
std::tuple<LhsMethods...>, std::tuple<LhsFields...>>& lhs,
const Class<RhsParentClass, std::tuple<RhsConstructors...>,
std::tuple<Static<std::tuple<RhsStaticMethods...>,
std::tuple<RhsStaticFields...>>>,
std::tuple<RhsMethods...>, std::tuple<RhsFields...>>& rhs) {
// Don't compare the other parameters so classes can be used as parameters
// or return values before the class itself is defined.
return std::string_view(lhs.name_) == std::string_view(rhs.name_);
}

template <typename... Ts>
constexpr bool operator==(const Class<Ts...>& lhs, const NoClass&) {
return false;
}

template <typename... Ts>
constexpr bool operator!=(const Class<Ts...>& lhs, const NoClass&) {
return true;
}

////////////////////////////////////////////////////////////////////////////////
// CTAD.
////////////////////////////////////////////////////////////////////////////////
template <typename... Params>
Class(const char*, Params...)
-> Class<metaprogramming::BaseFilterWithDefault_t<
Expand Down
45 changes: 31 additions & 14 deletions implementation/class_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

// IWYU pragma: private, include "third_party/jni_wrapper/jni_bind.h"

#include <cstddef>
#include <tuple>
#include <type_traits>
#include <utility>
Expand Down Expand Up @@ -68,20 +69,6 @@ class ClassLoader : public Object {
parent_loader_(parent_loader),
supported_classes_(supported_class_set.supported_classes_) {}

bool constexpr operator==(
const ClassLoader<ParentLoader_, SupportedClasses_...>& rhs) const {
return (*this).parent_loader_ == rhs.parent_loader_ &&
(*this).supported_classes_ == rhs.supported_classes_;
}
template <typename T>
bool constexpr operator==(const T& rhs) const {
return false;
}
template <typename T>
bool constexpr operator!=(const T& rhs) const {
return !(*this == rhs);
}

template <const auto& class_v, std::size_t... Is>
constexpr std::size_t IdxOfClassHelper(
std::integer_sequence<std::size_t, Is...>) const {
Expand Down Expand Up @@ -131,6 +118,33 @@ class ClassLoader : public Object {
}
};

////////////////////////////////////////////////////////////////////////////////
// Equality operators.
////////////////////////////////////////////////////////////////////////////////
template <typename ParentLoader_, typename... SupportedClasses_>
bool constexpr operator==(
const ClassLoader<ParentLoader_, SupportedClasses_...>& lhs,
const ClassLoader<ParentLoader_, SupportedClasses_...>& rhs) {
return lhs.parent_loader_ == rhs.parent_loader_ &&
lhs.supported_classes_ == rhs.supported_classes_;
}

template <typename ParentLoader_, typename... SupportedClasses_, typename T>
bool constexpr operator==(
const ClassLoader<ParentLoader_, SupportedClasses_...>& lhs, const T& rhs) {
return false;
}

template <typename ParentLoader_, typename... SupportedClasses_, typename T>
bool constexpr operator!=(
const ClassLoader<ParentLoader_, SupportedClasses_...>& lhs, const T& rhs) {
return !(lhs == rhs);
}

////////////////////////////////////////////////////////////////////////////////
// CTAD.
////////////////////////////////////////////////////////////////////////////////

// Note: Null is chosen, not default, because LoadedBy requires a syntax like
// LoadedBy{ClassLoader{"kClass"}} (using the CTAD loader type below), but
// we want to prevent explicit usage of a default loader (as it makes no sense).
Expand All @@ -146,6 +160,9 @@ template <typename... SupportedClasses_>
ClassLoader(SupportedClassSet<SupportedClasses_...>)
-> ClassLoader<DefaultClassLoader, SupportedClasses_...>;

////////////////////////////////////////////////////////////////////////////////
// Ancestral lookups.
////////////////////////////////////////////////////////////////////////////////
template <typename T, std::size_t I>
constexpr auto& GetAncestor(const T& loader) {
if constexpr (I == 0) {
Expand Down
Loading