Skip to content

Commit

Permalink
Mark callback and friends as trivial ABI everywhere except Windows.
Browse files Browse the repository at this point in the history
A local official Linux build goes from 209.06 MB to 208.09 MB. For
whatever reason, clang-cl is not happy with the use of TRIVIAL_ABI in
these contexts, so for now, this is TRIVIAL_ABI everywhere except
Windows.

To prevent the workaround macros from leaking, the definitions are
duplicated in both callback.h and callback_internal.h, since there's a
few files that include callback_internal.h directly.

In addition, build/build_config.h is already transitively included, so
removing the hack later should be trivial.

Bug: 1383397
Change-Id: I755d0b2c4b2e6eec89026ee16a88c572ab7794a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4021547
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1070593}
  • Loading branch information
zetafunction authored and Chromium LUCI CQ committed Nov 12, 2022
1 parent 11057b0 commit b7ece23
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 17 deletions.
53 changes: 39 additions & 14 deletions base/functional/callback.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@
#include <utility>

#include "base/check.h"
#include "base/compiler_specific.h"
#include "base/functional/bind.h"
#include "base/functional/callback_forward.h" // IWYU pragma: export
#include "base/functional/callback_internal.h"
#include "base/functional/callback_tags.h"
#include "base/functional/function_ref.h"
#include "base/notreached.h"
#include "base/types/always_false.h"
#include "build/build_config.h"

// -----------------------------------------------------------------------------
// Usage documentation
Expand Down Expand Up @@ -86,8 +88,20 @@ ToDoNothingCallback(DoNothingCallbackTag::WithBoundArguments<BoundArgs...> t) {

} // namespace internal

// TODO(https://crbug.com/1383397): Temporary workaround for clang-cl
// ignoring TRIVIAL_ABI on `CallbackBaseCopyable`, `OnceCallback`, and
// `RepeatingCallback` because, for some reason, it considers them to have
// non-trivial abi base classes, despite all the base classes being annotated
// with TRIVIAL_ABI...
#if BUILDFLAG(IS_WIN)
#define TRIVIAL_ABI_EXCEPT_WIN
#else
#define TRIVIAL_ABI_EXCEPT_WIN TRIVIAL_ABI
#endif

template <typename R, typename... Args>
class OnceCallback<R(Args...)> : public internal::CallbackBase {
class TRIVIAL_ABI_EXCEPT_WIN OnceCallback<R(Args...)>
: public internal::CallbackBase {
public:
using ResultType = R;
using RunType = R(Args...);
Expand All @@ -97,6 +111,16 @@ class OnceCallback<R(Args...)> : public internal::CallbackBase {
constexpr OnceCallback() = default;
OnceCallback(std::nullptr_t) = delete;

OnceCallback(const OnceCallback&) = delete;
OnceCallback& operator=(const OnceCallback&) = delete;

// Since this type is marked as TRIVIAL_ABI, the move operations must leave
// the moved-from object in a trivially destructible state.
OnceCallback(OnceCallback&&) noexcept = default;
OnceCallback& operator=(OnceCallback&&) noexcept = default;

~OnceCallback() = default;

constexpr OnceCallback(internal::NullCallbackTag) : OnceCallback() {}
constexpr OnceCallback& operator=(internal::NullCallbackTag) {
*this = OnceCallback();
Expand Down Expand Up @@ -142,12 +166,6 @@ class OnceCallback<R(Args...)> : public internal::CallbackBase {
explicit OnceCallback(internal::BindStateBase* bind_state)
: internal::CallbackBase(bind_state) {}

OnceCallback(const OnceCallback&) = delete;
OnceCallback& operator=(const OnceCallback&) = delete;

OnceCallback(OnceCallback&&) noexcept = default;
OnceCallback& operator=(OnceCallback&&) noexcept = default;

OnceCallback(RepeatingCallback<RunType> other)
: internal::CallbackBase(std::move(other)) {}

Expand Down Expand Up @@ -228,7 +246,8 @@ class OnceCallback<R(Args...)> : public internal::CallbackBase {
};

template <typename R, typename... Args>
class RepeatingCallback<R(Args...)> : public internal::CallbackBaseCopyable {
class TRIVIAL_ABI_EXCEPT_WIN RepeatingCallback<R(Args...)>
: public internal::CallbackBaseCopyable {
public:
using ResultType = R;
using RunType = R(Args...);
Expand All @@ -238,6 +257,16 @@ class RepeatingCallback<R(Args...)> : public internal::CallbackBaseCopyable {
constexpr RepeatingCallback() = default;
RepeatingCallback(std::nullptr_t) = delete;

RepeatingCallback(const RepeatingCallback&) = default;
RepeatingCallback& operator=(const RepeatingCallback&) = default;

// Since this type is marked as TRIVIAL_ABI, the move operations must leave
// the moved-from object in a trivially destructible state.
RepeatingCallback(RepeatingCallback&&) noexcept = default;
RepeatingCallback& operator=(RepeatingCallback&&) noexcept = default;

~RepeatingCallback() = default;

constexpr RepeatingCallback(internal::NullCallbackTag)
: RepeatingCallback() {}
constexpr RepeatingCallback& operator=(internal::NullCallbackTag) {
Expand Down Expand Up @@ -285,12 +314,6 @@ class RepeatingCallback<R(Args...)> : public internal::CallbackBaseCopyable {
explicit RepeatingCallback(internal::BindStateBase* bind_state)
: internal::CallbackBaseCopyable(bind_state) {}

// Copyable and movable.
RepeatingCallback(const RepeatingCallback&) = default;
RepeatingCallback& operator=(const RepeatingCallback&) = default;
RepeatingCallback(RepeatingCallback&&) noexcept = default;
RepeatingCallback& operator=(RepeatingCallback&&) noexcept = default;

bool operator==(const RepeatingCallback& other) const {
return EqualsInternal(other);
}
Expand Down Expand Up @@ -374,6 +397,8 @@ class RepeatingCallback<R(Args...)> : public internal::CallbackBaseCopyable {
}
};

#undef TRIVIAL_ABI_EXCEPT_WIN

} // namespace base

#endif // BASE_FUNCTIONAL_CALLBACK_H_
30 changes: 27 additions & 3 deletions base/functional/callback_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,21 @@
#include <utility>

#include "base/base_export.h"
#include "base/compiler_specific.h"
#include "base/functional/callback_forward.h"
#include "base/memory/ref_counted.h"
#include "build/build_config.h"

// TODO(https://crbug.com/1383397): Temporary workaround for clang-cl
// ignoring TRIVIAL_ABI on `CallbackBaseCopyable`, `OnceCallback`, and
// `RepeatingCallback` because, for some reason, it considers them to have
// non-trivial abi base classes, despite all the base classes being annotated
// with TRIVIAL_ABI...
#if BUILDFLAG(IS_WIN)
#define TRIVIAL_ABI_EXCEPT_WIN
#else
#define TRIVIAL_ABI_EXCEPT_WIN TRIVIAL_ABI
#endif

namespace base {

Expand Down Expand Up @@ -110,8 +123,10 @@ class BASE_EXPORT BindStateBase
// template bloat.
// CallbackBase<MoveOnly> is a direct base class of MoveOnly callbacks, and
// CallbackBase<Copyable> uses CallbackBase<MoveOnly> for its implementation.
class BASE_EXPORT CallbackBase {
class BASE_EXPORT TRIVIAL_ABI_EXCEPT_WIN CallbackBase {
public:
// Since this type is marked as TRIVIAL_ABI, the move operations must leave
// the moved-from object in a trivially destructible state.
inline CallbackBase(CallbackBase&& c) noexcept;
CallbackBase& operator=(CallbackBase&& c) noexcept;

Expand Down Expand Up @@ -165,6 +180,9 @@ class BASE_EXPORT CallbackBase {
// Force the destructor to be instantiated inside this translation unit so
// that our subclasses will not get inlined versions. Avoids more template
// bloat.
//
// Since type is marked as TRIVIAL_ABI, the destructor must be a no-op when
// `this` is a moved-from object.
~CallbackBase();

scoped_refptr<BindStateBase> bind_state_;
Expand All @@ -176,11 +194,15 @@ CallbackBase::CallbackBase(BindStateBase* bind_state)
: bind_state_(AdoptRef(bind_state)) {}

// CallbackBase<Copyable> is a direct base class of Copyable Callbacks.
class BASE_EXPORT CallbackBaseCopyable : public CallbackBase {
class BASE_EXPORT TRIVIAL_ABI_EXCEPT_WIN CallbackBaseCopyable
: public CallbackBase {
public:
CallbackBaseCopyable(const CallbackBaseCopyable& c);
CallbackBaseCopyable(CallbackBaseCopyable&& c) noexcept = default;
CallbackBaseCopyable& operator=(const CallbackBaseCopyable& c);

// Since this type is marked as TRIVIAL_ABI, the move operations must leave
// the moved-from object in a trivially destructible state.
CallbackBaseCopyable(CallbackBaseCopyable&& c) noexcept = default;
CallbackBaseCopyable& operator=(CallbackBaseCopyable&& c) noexcept;

protected:
Expand Down Expand Up @@ -247,4 +269,6 @@ struct ThenHelper<OriginalCallback<OriginalR(OriginalArgs...)>,
} // namespace internal
} // namespace base

#undef TRIVIAL_ABI_EXCEPT_WIN

#endif // BASE_FUNCTIONAL_CALLBACK_INTERNAL_H_

0 comments on commit b7ece23

Please sign in to comment.