Workaround GCC friendship bugs

PiperOrigin-RevId: 533103982

Author: jcking

base/internal/memory_manager.h

@@ -32,8 +32,7 @@ struct HasIsDestructorSkippable : std::false_type {};
 
 template <typename T>
 struct HasIsDestructorSkippable<
-    T,
-    std::void_t<decltype(T::IsDestructorSkippable(std::declval<const T&>()))>>
+    T, std::void_t<decltype(std::declval<const T&>().IsDestructorSkippable())>>
     : std::true_type {};
 
 }  // namespace cel::base_internal

base/memory.h

@@ -185,7 +185,7 @@ class MemoryManager {
           T(std::forward<Args>(args)...);
       if constexpr (!std::is_trivially_destructible_v<T>) {
         if constexpr (base_internal::HasIsDestructorSkippable<T>::value) {
-          if (!T::IsDestructorSkippable(*pointer)) {
+          if (!pointer->IsDestructorSkippable()) {
             OwnDestructor(pointer,
                           &base_internal::MemoryManagerDestructor<T>::Destruct);
           }
@@ -357,6 +357,18 @@ class Allocator {
   bool allocation_only_;
 };
 
+// GCC before 12 has buggy friendship. Instead of calculating friendship at the
+// point of evaluation it does so at the point where it is written. This macro
+// ensures compatibility by friending both so IsDestructorSkippable works
+// correctly.
+#define CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE()                  \
+ private:                                                       \
+  friend class ::cel::MemoryManager;                            \
+  template <typename, typename>                                 \
+  friend struct ::cel::base_internal::HasIsDestructorSkippable; \
+                                                                \
+  bool IsDestructorSkippable() const
+
 namespace base_internal {
 
 template <typename T>

base/types/list_type.h

@@ -24,6 +24,7 @@
 #include "absl/types/span.h"
 #include "base/internal/data.h"
 #include "base/kind.h"
+#include "base/memory.h"
 #include "base/type.h"
 
 namespace cel {
@@ -109,8 +110,8 @@ class ModernListType final : public ListType, public HeapData {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(const ModernListType& type) {
-    return Metadata::IsDestructorSkippable(*type.element());
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
+    return Metadata::IsDestructorSkippable(*element());
   }
 
   explicit ModernListType(Handle<Type> element);

base/types/map_type.h

@@ -24,6 +24,7 @@
 #include "absl/types/span.h"
 #include "base/internal/data.h"
 #include "base/kind.h"
+#include "base/memory.h"
 #include "base/type.h"
 
 namespace cel {
@@ -115,9 +116,9 @@ class ModernMapType final : public MapType, public HeapData {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(const ModernMapType& type) {
-    return Metadata::IsDestructorSkippable(*type.key()) &&
-           Metadata::IsDestructorSkippable(*type.value());
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
+    return Metadata::IsDestructorSkippable(*key()) &&
+           Metadata::IsDestructorSkippable(*value());
   }
 
   explicit ModernMapType(Handle<Type> key, Handle<Type> value);

base/types/optional_type.h

@@ -21,6 +21,7 @@
 #include "absl/log/absl_check.h"
 #include "absl/strings/string_view.h"
 #include "absl/types/span.h"
+#include "base/memory.h"
 #include "base/type.h"
 #include "base/types/opaque_type.h"
 #include "internal/rtti.h"
@@ -57,8 +58,8 @@ class OptionalType final : public OpaqueType {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(const OptionalType& type) {
-    return base_internal::Metadata::IsDestructorSkippable(*type.type());
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
+    return base_internal::Metadata::IsDestructorSkippable(*type());
   }
 
   explicit OptionalType(Handle<Type> type) : type_(std::move(type)) {}

base/values/optional_value.h

@@ -21,6 +21,7 @@
 #include "absl/log/absl_check.h"
 #include "absl/status/statusor.h"
 #include "base/handle.h"
+#include "base/memory.h"
 #include "base/type.h"
 #include "base/types/optional_type.h"
 #include "base/value.h"
@@ -98,8 +99,8 @@ class EmptyOptionalValue final : public OptionalValue {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(const EmptyOptionalValue& value) {
-    return base_internal::Metadata::IsDestructorSkippable(*value.type());
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
+    return base_internal::Metadata::IsDestructorSkippable(*type());
   }
 
   explicit EmptyOptionalValue(Handle<OptionalType> type)
@@ -117,9 +118,9 @@ class FullOptionalValue final : public OptionalValue {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(const FullOptionalValue& value) {
-    return base_internal::Metadata::IsDestructorSkippable(*value.type()) &&
-           base_internal::Metadata::IsDestructorSkippable(*value.value());
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
+    return base_internal::Metadata::IsDestructorSkippable(*type()) &&
+           base_internal::Metadata::IsDestructorSkippable(*value());
   }
 
   FullOptionalValue(Handle<OptionalType> type, Handle<Value> value)

extensions/protobuf/BUILD

@@ -95,8 +95,9 @@ cc_library(
         "type_provider.h",
     ],
     deps = [
+        "//base:data",
         "//base:handle",
-        "//base:type",
+        "//base:memory",
         "//internal:status_macros",
         "@com_google_absl//absl/base:core_headers",
         "@com_google_absl//absl/log:die_if_null",

extensions/protobuf/enum_type.h

@@ -17,6 +17,7 @@
 
 #include "absl/base/attributes.h"
 #include "absl/log/die_if_null.h"
+#include "base/memory.h"
 #include "base/type.h"
 #include "base/type_manager.h"
 #include "base/types/enum_type.h"
@@ -71,8 +72,7 @@ class ProtoEnumType final : public EnumType {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(
-      const ProtoEnumType& type ABSL_ATTRIBUTE_UNUSED) {
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
     // Our destructor is useless, we only hold pointers to protobuf-owned data.
     return true;
   }

extensions/protobuf/struct_type.h

@@ -22,6 +22,7 @@
 #include "absl/status/statusor.h"
 #include "absl/strings/string_view.h"
 #include "base/handle.h"
+#include "base/memory.h"
 #include "base/type.h"
 #include "base/type_manager.h"
 #include "base/types/struct_type.h"
@@ -84,8 +85,7 @@ class ProtoStructType final : public CEL_STRUCT_TYPE_CLASS {
 
   // Called by Arena-based memory managers to determine whether we actually need
   // our destructor called.
-  static bool IsDestructorSkippable(
-      const ProtoStructType& type ABSL_ATTRIBUTE_UNUSED) {
+  CEL_INTERNAL_IS_DESTRUCTOR_SKIPPABLE() {
     // Our destructor is useless, we only hold pointers to protobuf-owned data.
     return true;
   }