Commit 6e123842 authored by Christos Stratopoulos's avatar Christos Stratopoulos Committed by Facebook Github Bot

Add noexcept-qualified versions of `IsConstMember` (#1201)

Summary:
This is a missing piece from https://github.com/facebook/folly/pull/1178, but more nasty as it can cause runtime rather than compile-time failures.

Given an interface `ICat` with `void meow() const`, if we have `struct cat` with `void meow() const noexcept`, then constructing a `folly::Poly<ICat const&>` for an instance of `cat` will have a null pointer in the vtable entry for the `meow` method.

The culprit seems to be in the template specialization for [`ThunkFn` ](https://github.com/cstratopoulos/folly/blob/feature/poly-update/folly/detail/PolyDetail.h#L539) which may be dispatched on `IsConstMember`.

I've added a fairly trivial test case to `PolyTest.cpp`; but notably it is the only instance of _any_ test case in that file having a `noexcept` method. Without the change in `PolyDetail.h`, the call to `cref->meow()` will cause a null pointer dereference. If something more explicit is desired, maybe we could `EXPECT` that the corresponding vtable entry is non-null, but that doesn't exactly fit the pattern of other test cases in that file.

(Aside: In the linked PR I used `__cpp_noexcept_function_type` directly, but I have since realized that a macro for this is already present in [`Portability.h`](https://github.com/facebook/folly/blob/master/folly/Portability.h#L503) so I'm using that one now)
Pull Request resolved: https://github.com/facebook/folly/pull/1201

Reviewed By: ericniebler

Differential Revision: D16686139

Pulled By: yfeldblum

fbshipit-source-id: 73c7ac132d6e772ffd49f450b5fcd0f8a0dbdb26
parent 6b2c52f3
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include <typeinfo> #include <typeinfo>
#include <utility> #include <utility>
#include <folly/Portability.h>
#include <folly/Traits.h> #include <folly/Traits.h>
#include <folly/Utility.h> #include <folly/Utility.h>
#include <folly/detail/TypeList.h> #include <folly/detail/TypeList.h>
...@@ -404,7 +405,7 @@ struct SignatureOf_<R (C::*)(As...) const, I> { ...@@ -404,7 +405,7 @@ struct SignatureOf_<R (C::*)(As...) const, I> {
using type = Ret<R, I> (*)(Data const&, Arg<As, I>...); using type = Ret<R, I> (*)(Data const&, Arg<As, I>...);
}; };
#ifdef __cpp_noexcept_function_type #if FOLLY_HAVE_NOEXCEPT_FUNCTION_TYPE
template <class R, class C, class... As, class I> template <class R, class C, class... As, class I>
struct SignatureOf_<R (C::*)(As...) noexcept, I> { struct SignatureOf_<R (C::*)(As...) noexcept, I> {
using type = std::add_pointer_t<Ret<R, I>(Data&, Arg<As, I>...) noexcept>; using type = std::add_pointer_t<Ret<R, I>(Data&, Arg<As, I>...) noexcept>;
...@@ -438,7 +439,7 @@ struct ArgTypes_<User, I, Ret (*)(Data, Args...)> { ...@@ -438,7 +439,7 @@ struct ArgTypes_<User, I, Ret (*)(Data, Args...)> {
using type = TypeList<Args...>; using type = TypeList<Args...>;
}; };
#ifdef __cpp_noexcept_function_type #if FOLLY_HAVE_NOEXCEPT_FUNCTION_TYPE
template <FOLLY_AUTO User, class I, class Ret, class Data, class... Args> template <FOLLY_AUTO User, class I, class Ret, class Data, class... Args>
struct ArgTypes_<User, I, Ret (*)(Data, Args...) noexcept> { struct ArgTypes_<User, I, Ret (*)(Data, Args...) noexcept> {
using type = TypeList<Args...>; using type = TypeList<Args...>;
...@@ -521,6 +522,14 @@ struct IsConstMember<R (C::*)(As...) const> : std::true_type {}; ...@@ -521,6 +522,14 @@ struct IsConstMember<R (C::*)(As...) const> : std::true_type {};
template <class R, class C, class... As> template <class R, class C, class... As>
struct IsConstMember<R (*)(C const&, As...)> : std::true_type {}; struct IsConstMember<R (*)(C const&, As...)> : std::true_type {};
#if FOLLY_HAVE_NOEXCEPT_FUNCTION_TYPE
template <class R, class C, class... As>
struct IsConstMember<R (C::*)(As...) const noexcept> : std::true_type {};
template <class R, class C, class... As>
struct IsConstMember<R (*)(C const&, As...) noexcept> : std::true_type {};
#endif
template < template <
class T, class T,
FOLLY_AUTO User, FOLLY_AUTO User,
......
...@@ -833,3 +833,41 @@ TEST(Poly, PolyRefAsArg) { ...@@ -833,3 +833,41 @@ TEST(Poly, PolyRefAsArg) {
// should not throw: // should not throw:
frob.frobnicate(folly::Poly<folly::poly::IRegular&>(x)); frob.frobnicate(folly::Poly<folly::poly::IRegular&>(x));
} }
namespace {
struct ICat {
template <class Base>
struct Interface : Base {
void pet() {
folly::poly_call<0>(*this);
}
int meow() const {
return folly::poly_call<1>(*this);
}
};
template <class T>
using Members = FOLLY_POLY_MEMBERS(&T::pet, &T::meow);
};
struct cat {
void pet() noexcept {
++pet_count;
}
int meow() const noexcept {
return pet_count;
}
int pet_count = 0;
};
} // namespace
TEST(Poly, NoexceptMembers) {
cat c{};
folly::Poly<ICat&> ref = c;
ref->pet();
folly::Poly<ICat const&> cref = ref;
EXPECT_EQ(cref->meow(), 1);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment