From 4709203de66246db56ada7781f6373e5bb63ccac Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 9 Sep 2016 14:54:10 -0400 Subject: [PATCH] Make forward-declaring bssl::UniquePtr actually work. The compiler complains about: error: explicit specialization of 'bssl::internal::Deleter' after instantiation This is because, although the deleter's operator() is not instantiated without emitting std::unique_ptr's destructor, the deleter itself *is*. Deleters are allowed to have non-zero size, so a std::unique_ptr actually embeds a copy of the deleter, so it needs the size of the deleter. As with all problems in computer science, we fix this with a layer of indirection. Instead of specializing the deleter, we specialize bssl::internal::DeleterImpl which, when specialized, has a static method Free. That is only instantiated inside bssl::internal::Deleter::operator(), giving us the desired properties. (Did I mention forward decls are terrible? I wish people wouldn't want them so much.) Also appease clang-format. Change-Id: I9a07b2fd13e8bdfbd204e225ac72c52d20a397dc Reviewed-on: https://boringssl-review.googlesource.com/10964 Reviewed-by: Matt Braithwaite Commit-Queue: David Benjamin CQ-Verified: CQ bot account: commit-bot@chromium.org --- include/openssl/base.h | 42 ++++++++++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/include/openssl/base.h b/include/openssl/base.h index ff3f7eea1..fab293ea6 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -335,7 +335,23 @@ namespace bssl { namespace internal { -template struct Deleter {}; +template +struct DeleterImpl {}; + +template +struct Deleter { + void operator()(T *ptr) { + // Rather than specialize Deleter for each type, we specialize + // DeleterImpl. This allows bssl::UniquePtr to be used while only + // including base.h as long as the destructor is not emitted. This matches + // std::unique_ptr's behavior on forward-declared types. + // + // DeleterImpl itself is specialized in the corresponding module's header + // and must be included to release an object. If not included, the compiler + // will error that DeleterImpl does not have a method Free. + DeleterImpl::Free(ptr); + } +}; template @@ -358,22 +374,24 @@ class StackAllocated { } // namespace internal -#define BORINGSSL_MAKE_DELETER(type, deleter) \ - namespace internal { \ - template <> struct Deleter { \ - void operator()(type* ptr) { deleter(ptr); } \ - }; \ +#define BORINGSSL_MAKE_DELETER(type, deleter) \ + namespace internal { \ + template <> \ + struct DeleterImpl { \ + static void Free(type *ptr) { deleter(ptr); } \ + }; \ } // This makes a unique_ptr to STACK_OF(type) that owns all elements on the // stack, i.e. it uses sk_pop_free() to clean up. -#define BORINGSSL_MAKE_STACK_DELETER(type, deleter) \ +#define BORINGSSL_MAKE_STACK_DELETER(type, deleter) \ namespace internal { \ - template <> struct Deleter { \ - void operator()(STACK_OF(type)* ptr) { \ - sk_##type##_pop_free(ptr, deleter); \ - } \ - }; \ + template <> \ + struct DeleterImpl { \ + static void Free(STACK_OF(type) *ptr) { \ + sk_##type##_pop_free(ptr, deleter); \ + } \ + }; \ } // Holds ownership of heap-allocated BoringSSL structures. Sample usage: