Do not bother with the CanBindTo optimization in MakerType{,For}.

The optimization is important in `Initializer` constructed from an already
constructed object, but it is rare to explicitly wrap constructor arguments
in a way can benefit from copy elision.

`MakerType{,For}::Reference()` is still useful to support
`Initializer::Reference()`, and is compatible with immovable types before C++17
which guarantees copy elision.

If copy elision is guaranteed and `storage` is not provided,
`MakerType{,For}::Reference()` returns the result by value instead of by
reference, which is a more efficient way to construct the temporary.

Rename `internal::CanBindTo` to `internal::CanBindReference`, and limit it to a
single argument case which remains used.

Add `internal::BindReference()` function template, and use it to implicitly
convert references instead of applying the pointer hack where `CanBindReference`
is used (needed because not all compilers implement http://wg21.link/cwg2352).

PiperOrigin-RevId: 686644549

riegeli/base/initializer.h

@@ -194,8 +194,9 @@ class InitializerBase {
 #endif
   }
 
-  // Constructs the `T`, or returns a reference to an already constructed object
-  // if that was passed to the `Initializer`.
+  // Constructs the `T` in `storage` which must outlive the returned reference,
+  // or returns a reference to an already constructed object if a compatible
+  // object was passed to `Initializer` constructor.
   //
   // `Reference()` instead of conversion to `T` can avoid moving the object if
   // the caller does not need to store the object, or if it will be moved later
@@ -237,11 +238,11 @@ class InitializerBase {
                                     TemporaryStorage<T>&& storage);
 
   template <typename Arg,
-            std::enable_if_t<CanBindTo<T&&, Arg&&>::value, int> = 0>
+            std::enable_if_t<CanBindReference<T&&, Arg&&>::value, int> = 0>
   static T&& ReferenceMethodFromObject(TypeErasedRef context,
                                        TemporaryStorage<T>&& storage);
   template <typename Arg,
-            std::enable_if_t<!CanBindTo<T&&, Arg&&>::value, int> = 0>
+            std::enable_if_t<!CanBindReference<T&&, Arg&&>::value, int> = 0>
   static T&& ReferenceMethodFromObject(TypeErasedRef context,
                                        TemporaryStorage<T>&& storage);
 
@@ -921,15 +922,17 @@ T&& InitializerBase<T>::ReferenceMethodDefault(
 }
 
 template <typename T>
-template <typename Arg, std::enable_if_t<CanBindTo<T&&, Arg&&>::value, int>>
+template <typename Arg,
+          std::enable_if_t<CanBindReference<T&&, Arg&&>::value, int>>
 T&& InitializerBase<T>::ReferenceMethodFromObject(
     TypeErasedRef context,
     ABSL_ATTRIBUTE_UNUSED TemporaryStorage<T>&& storage) {
-  return context.Cast<Arg>();
+  return BindReference<T&&>(context.Cast<Arg>());
 }
 
 template <typename T>
-template <typename Arg, std::enable_if_t<!CanBindTo<T&&, Arg&&>::value, int>>
+template <typename Arg,
+          std::enable_if_t<!CanBindReference<T&&, Arg&&>::value, int>>
 T&& InitializerBase<T>::ReferenceMethodFromObject(
     TypeErasedRef context, TemporaryStorage<T>&& storage) {
   return std::move(storage).emplace(context.Cast<Arg>());

riegeli/base/initializer_internal.h

@@ -17,35 +17,51 @@
 
 #include <type_traits>
 
+#include "absl/base/casts.h"
+
 namespace riegeli {
 namespace initializer_internal {
 
-// `CanBindTo<T&&, Args&&...>::value` is `true` if constructing `T(args...)`
-// with `args...` of type `Args&&...` can be elided, with `T&&` binding directly
-// to the only element of `args...` instead.
+// `CanBindReference<T&&, Arg&&>::value` is `true` if `Arg&&` can be implicitly
+// converted to `T&&` without creating a temporary.
 //
 // Due to not all compilers implementing http://wg21.link/cwg2352 (converting
 // `T*&` to `const T* const&` could have bound the result to a temporary),
-// binding a const reference should be done by converting between the
-// corresponding pointer types and then dereferencing.
+// this covers also the case when the corresponding pointers can be converted.
+// `BindReference()` should be used for the actual conversion.
 
-template <typename T, typename... Args>
-struct CanBindTo : std::false_type {};
+template <typename T, typename Arg>
+struct CanBindReference : std::false_type {};
 
 template <typename T, typename Arg>
-struct CanBindTo<T&, Arg&> : std::is_convertible<Arg*, T*> {};
+struct CanBindReference<T&, Arg&> : std::is_convertible<Arg*, T*> {};
 
 template <typename T, typename Arg>
-struct CanBindTo<T&, Arg&&> : std::false_type {};
+struct CanBindReference<T&, Arg&&> : std::false_type {};
 
 template <typename T, typename Arg>
-struct CanBindTo<const T&, Arg&&> : std::is_convertible<Arg*, const T*> {};
+struct CanBindReference<const T&, Arg&&> : std::is_convertible<Arg*, const T*> {
+};
 
 template <typename T, typename Arg>
-struct CanBindTo<T&&, Arg&> : std::false_type {};
+struct CanBindReference<T&&, Arg&> : std::false_type {};
 
 template <typename T, typename Arg>
-struct CanBindTo<T&&, Arg&&> : std::is_convertible<Arg*, T*> {};
+struct CanBindReference<T&&, Arg&&> : std::is_convertible<Arg*, T*> {};
+
+// `BindReference<T&&>(arg)` returns `arg` implicitly converted to `T&&`.
+//
+// Due to not all compilers implementing http://wg21.link/cwg2352 (converting
+// `T*&` to `const T* const&` could have bound the result to a temporary),
+// this is not implemented as a simple implicit conversion, but by converting
+// the reference to a pointer, implicitly converting the pointer, and
+// dereferencing back.
+template <typename T, typename Arg,
+          std::enable_if_t<CanBindReference<T&&, Arg&&>::value, int> = 0>
+inline T&& BindReference(Arg&& arg) {
+  return std::forward<T>(
+      *absl::implicit_cast<std::remove_reference_t<T>*>(&arg));
+}
 
 }  // namespace initializer_internal
 }  // namespace riegeli

riegeli/base/maker.h

@@ -23,10 +23,8 @@
 #include <utility>
 
 #include "absl/base/attributes.h"
-#include "absl/base/casts.h"
 #include "absl/meta/type_traits.h"
 #include "absl/utility/utility.h"
-#include "riegeli/base/initializer_internal.h"
 #include "riegeli/base/reset.h"
 #include "riegeli/base/temporary_storage.h"
 #include "riegeli/base/type_traits.h"
@@ -97,77 +95,53 @@ class MakerType : public ConditionallyAssignable<absl::conjunction<
     ConstructAtImpl<T>(ptr, std::index_sequence_for<Args...>());
   }
 
-  // Constructs the `T`, or returns a reference to an already constructed object
-  // if that was passed to the `MakerType`.
+  // Constructs the `T` in `storage` which must outlive the returned reference.
   //
-  // `Reference()` instead of `Construct()` can avoid moving the object if the
-  // caller does not need to store the object, or if it will be moved later
-  // because the target location for the object is not ready yet.
+  // `Reference()` instead of `Construct()` supports `Initializer::Reference()`,
+  // and is compatible with immovable types before C++17 which guarantees copy
+  // elision.
   //
-  // `storage` must outlive usages of the returned reference.
+  // If copy elision is guaranteed and the `storage` argument is omitted, the
+  // result is returned by value instead of by reference, which is a more
+  // efficient way to construct the temporary.
+#if __cpp_guaranteed_copy_elision
   template <
       typename T,
-      std::enable_if_t<absl::conjunction<std::is_constructible<T, Args&&...>,
-                                         initializer_internal::CanBindTo<
-                                             T&&, Args&&...>>::value,
-                       int> = 0>
-      T&& Reference() && ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    auto&& reference = std::get<0>(std::move(args_));
-    return std::forward<T>(
-        *absl::implicit_cast<std::remove_reference_t<T>*>(&reference));
+      std::enable_if_t<std::is_constructible<T, Args&&...>::value, int> = 0>
+  T Reference() && {
+    return std::move(*this).template Construct<T>();
   }
+#endif
   template <
       typename T,
-      std::enable_if_t<absl::conjunction<std::is_constructible<T, Args&&...>,
-                                         initializer_internal::CanBindTo<
-                                             T&&, Args&&...>>::value,
-                       int> = 0>
-      T&& Reference(ABSL_ATTRIBUTE_UNUSED TemporaryStorage<T>&& storage) &&
-      ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return std::move(*this).template Reference<T>();
-  }
-  template <typename T,
-            std::enable_if_t<absl::conjunction<
-                                 std::is_constructible<T, Args&&...>,
-                                 absl::negation<initializer_internal::CanBindTo<
-                                     T&&, Args&&...>>>::value,
-                             int> = 0>
-  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND =
-                    TemporaryStorage<T>()) && {
+      std::enable_if_t<std::is_constructible<T, Args&&...>::value, int> = 0>
+  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND
+#if !__cpp_guaranteed_copy_elision
+                = TemporaryStorage<T>()
+#endif
+                    ) && {
     return absl::apply(
         [&](Args&&... args) -> T&& {
           return std::move(storage).emplace(std::forward<Args>(args)...);
         },
         std::move(args_));
   }
+#if __cpp_guaranteed_copy_elision
   template <typename T,
-            std::enable_if_t<
-                absl::conjunction<std::is_constructible<T, const Args&...>,
-                                  initializer_internal::CanBindTo<
-                                      T&&, const Args&...>>::value,
-                int> = 0>
-  T&& Reference() const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return std::forward<T>(
-        *absl::implicit_cast<std::remove_reference_t<T>*>(&std::get<0>(args_)));
-  }
-  template <typename T,
-            std::enable_if_t<
-                absl::conjunction<std::is_constructible<T, const Args&...>,
-                                  initializer_internal::CanBindTo<
-                                      T&&, const Args&...>>::value,
-                int> = 0>
-  T&& Reference(ABSL_ATTRIBUTE_UNUSED TemporaryStorage<T>&& storage)
-      const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return Reference<T>();
+            std::enable_if_t<std::is_constructible<T, const Args&...>::value,
+                             int> = 0>
+  T Reference() const& {
+    return Construct<T>();
   }
+#endif
   template <typename T,
-            std::enable_if_t<absl::conjunction<
-                                 std::is_constructible<T, const Args&...>,
-                                 absl::negation<initializer_internal::CanBindTo<
-                                     T&&, const Args&...>>>::value,
+            std::enable_if_t<std::is_constructible<T, const Args&...>::value,
                              int> = 0>
-  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND =
-                    TemporaryStorage<T>()) const& {
+  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND
+#if !__cpp_guaranteed_copy_elision
+                = TemporaryStorage<T>()
+#endif
+  ) const& {
     return absl::apply(
         [&](const Args&... args) -> T&& {
           return std::move(storage).emplace(args...);
@@ -261,35 +235,23 @@ class MakerTypeForBase
     std::move(*this).maker().template ConstructAt<T>(ptr);
   }
 
-  // Constructs the `T`, or returns a reference to an already constructed object
-  // if that was passed to the `MakerTypeFor`.
+  // Constructs the `T` in `storage` which must outlive the returned reference.
   //
-  // `Reference()` instead of conversion to `T` can avoid moving the object if
-  // the caller does not need to store the object, or if it will be moved later
-  // because the target location for the object is not ready yet.
+  // `Reference()` instead of conversion to `T` supports
+  // `Initializer::Reference()`, and is compatible with immovable types before
+  // C++17 which guarantees copy elision.
   //
-  // `storage` must outlive usages of the returned reference.
-  template <typename DependentT = T,
-            std::enable_if_t<
-                initializer_internal::CanBindTo<DependentT&&, Args&&...>::value,
-                int> = 0>
-      T&& Reference() && ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return std::move(*this).maker().template Reference<T>();
-  }
-  template <typename DependentT = T,
-            std::enable_if_t<
-                initializer_internal::CanBindTo<DependentT&&, Args&&...>::value,
-                int> = 0>
-      T&& Reference(ABSL_ATTRIBUTE_UNUSED TemporaryStorage<T>&& storage) &&
-      ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return std::move(*this).Reference();
-  }
-  template <typename DependentT = T,
-            std::enable_if_t<!initializer_internal::CanBindTo<DependentT&&,
-                                                              Args&&...>::value,
-                             int> = 0>
-  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND =
-                    TemporaryStorage<T>()) && {
+  // If copy elision is guaranteed and the `storage` argument is omitted, the
+  // result is returned by value instead of by reference, which is a more
+  // efficient way to construct the temporary.
+#if __cpp_guaranteed_copy_elision
+  T Reference() && { return std::move(*this).maker().template Reference<T>(); }
+#endif
+  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND
+#if !__cpp_guaranteed_copy_elision
+                = TemporaryStorage<T>()
+#endif
+                    ) && {
     return std::move(*this).maker().template Reference<T>(std::move(storage));
   }
 
@@ -357,28 +319,24 @@ class MakerTypeForByConstBase</*is_const_makable=*/true, T, Args...>
     this->maker().template ConstructAt<T>(ptr);
   }
 
+  // Constructs the `T` in `storage` which must outlive the returned reference.
+  //
+  // `Reference()` instead of conversion to `T` supports
+  // `Initializer::Reference()`, and is compatible with immovable types before
+  // C++17 which guarantees copy elision.
+  //
+  // If copy elision is guaranteed and the `storage` argument is omitted, the
+  // result is returned by value instead of by reference, which is a more
+  // efficient way to construct the temporary.
   using MakerTypeForByConstBase::MakerTypeForBase::Reference;
-  template <typename DependentT = T,
-            std::enable_if_t<initializer_internal::CanBindTo<
-                                 DependentT&&, const Args&...>::value,
-                             int> = 0>
-  T&& Reference() const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return this->maker().template Reference<T>();
-  }
-  template <typename DependentT = T,
-            std::enable_if_t<initializer_internal::CanBindTo<
-                                 DependentT&&, const Args&...>::value,
-                             int> = 0>
-  T&& Reference(ABSL_ATTRIBUTE_UNUSED TemporaryStorage<T>&& storage)
-      const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
-    return Reference();
-  }
-  template <typename DependentT = T,
-            std::enable_if_t<!initializer_internal::CanBindTo<
-                                 DependentT&&, const Args&...>::value,
-                             int> = 0>
-  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND =
-                    TemporaryStorage<T>()) const& {
+#if __cpp_guaranteed_copy_elision
+  T Reference() const& { return this->maker().template Reference<T>(); }
+#endif
+  T&& Reference(TemporaryStorage<T>&& storage ABSL_ATTRIBUTE_LIFETIME_BOUND
+#if !__cpp_guaranteed_copy_elision
+                = TemporaryStorage<T>()
+#endif
+  ) const& {
     return this->maker().template Reference<T>(std::move(storage));
   }