Skip to content

Commit d2d7354

Browse files
committed
With -Werror and -Woverloaded-virtual, we cannot use createManagedInstance with a Subscriber<T> where T != Payload.
test/simple/SubscriberNonPayloadTest.cpp will fail without the changes to MemoryMixin.h Other mixins also refer to Payload directly, but I can fix those up in subsequent diffs.
1 parent 69e113c commit d2d7354

File tree

3 files changed

+49
-4
lines changed

3 files changed

+49
-4
lines changed

CMakeLists.txt

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ find_package(Threads)
5656

5757
# Common configuration for all build modes.
5858
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
59-
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wno-unused-parameter")
59+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wno-unused-parameter -Werror -Woverloaded-virtual")
6060
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer -momit-leaf-frame-pointer")
6161

6262
# Configuration for Debug build mode.
@@ -289,4 +289,18 @@ target_link_libraries(
289289

290290
add_dependencies(tcpresumeserver gmock)
291291

292+
add_executable(
293+
subscriber_non_payload
294+
test/simple/SubscriberNonPayloadTest.cpp
295+
)
296+
297+
target_link_libraries(
298+
subscriber_non_payload
299+
ReactiveSocket
300+
${FOLLY_LIBRARIES}
301+
${CMAKE_THREAD_LIBS_INIT}
302+
)
303+
304+
add_dependencies(subscriber_non_payload ReactiveSocket ReactiveStreams)
305+
292306
# EOF

src/mixins/MemoryMixin.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ namespace reactivesocket {
3030
/// implicitly specified as virtual, depending on whether the Base class
3131
/// is implementing the (virtual) methods of the
3232
/// Subscription or the Subscriber interface.
33-
template <typename Base>
33+
template <typename Base, typename Payload = Payload>
3434
class MemoryMixin : public Base {
3535
static_assert(
3636
std::is_base_of<IntrusiveDeleter, Base>::value,
@@ -126,10 +126,11 @@ class WithIntrusiveDeleter {
126126

127127
} // namespace details
128128

129-
template <typename Base, typename... TArgs>
129+
template <typename Base, typename Payload = Payload, typename... TArgs>
130130
Base& createManagedInstance(TArgs&&... args) {
131131
auto* instance =
132-
new MemoryMixin<typename details::WithIntrusiveDeleter<Base>::T>(
132+
new MemoryMixin<typename details::WithIntrusiveDeleter<Base>::T,
133+
Payload>(
133134
std::forward<TArgs>(args)...);
134135
return *instance;
135136
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2004-present Facebook. All Rights Reserved.
2+
3+
#include <stddef.h>
4+
5+
#include "src/ReactiveStreamsCompat.h"
6+
#include "src/Payload.h"
7+
#include "src/mixins/IntrusiveDeleter.h"
8+
#include "src/mixins/MemoryMixin.h"
9+
10+
using namespace reactivesocket;
11+
12+
class Foo {
13+
public:
14+
explicit Foo(const std::string&) {}
15+
};
16+
17+
class FooSubscriber : public IntrusiveDeleter, public Subscriber<Foo> {
18+
public:
19+
~FooSubscriber() override = default;
20+
void onSubscribe(Subscription&) override {}
21+
void onNext(Foo) override {}
22+
void onComplete() override {}
23+
void onError(folly::exception_wrapper) override {}
24+
};
25+
26+
int main(int argc, char** argv) {
27+
auto& m = createManagedInstance<FooSubscriber, Foo>();
28+
m.onNext(Foo("asdf"));
29+
return 0;
30+
}

0 commit comments

Comments
 (0)