Skip to content

Commit

Permalink
common: Add drake_copyable support for separate definition (RobotLoco…
Browse files Browse the repository at this point in the history
…motion#10480)

Use the new tooling in systems/framework to resolve some TODOs
  • Loading branch information
jwnimmer-tri authored Jan 24, 2019
1 parent 246b2c0 commit de5d10a
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 15 deletions.
46 changes: 46 additions & 0 deletions common/drake_copyable.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,49 @@ class Foo {
(void) static_cast<Classname& (Classname::*)( \
const Classname&)>(&Classname::operator=); \
}

/** DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN declares (but does not define) the
special member functions for copy-construction, copy-assignment,
move-construction, and move-assignment. Drake's Doxygen is customized to
render the declarations with appropriate comments.
This is useful when paired with DRAKE_DEFINE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN_T
to work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57728 whereby the
declaration and definition must be split. Once Drake no longer supports GCC
versions prior to 6.3, this macro could be removed.
Invoke this this macro in the public section of the class declaration, e.g.:
<pre>
template <typename T>
class Foo {
public:
DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN(Foo)
// ...
};
DRAKE_DEFINE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN_T(Foo)
</pre>
*/
#define DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN(Classname) \
Classname(const Classname&); \
Classname& operator=(const Classname&); \
Classname(Classname&&); \
Classname& operator=(Classname&&); \
/* Fails at compile-time if default-copy doesn't work. */ \
static void DRAKE_COPYABLE_DEMAND_COPY_CAN_COMPILE() { \
(void) static_cast<Classname& (Classname::*)( \
const Classname&)>(&Classname::operator=); \
}

/** Helper for DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN. Provides defaulted
definitions for the four special member functions of a templated class.
*/
#define DRAKE_DEFINE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN_T(Classname) \
template <typename T> \
Classname<T>::Classname(const Classname<T>&) = default; \
template <typename T> \
Classname<T>& Classname<T>::operator=(const Classname<T>&) = default; \
template <typename T> \
Classname<T>::Classname(Classname<T>&&) = default; \
template <typename T> \
Classname<T>& Classname<T>::operator=(Classname<T>&&) = default;
8 changes: 8 additions & 0 deletions doc/Doxyfile_CXX.in
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,14 @@ X& operator=(const X&) = default; \
X(X&&) = default; \
X& operator=(X&&) = default; \
/** \} */"
PREDEFINED += DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN(X):=" \
/** \name Implements CopyConstructible, CopyAssignable, MoveConstructible, MoveAssignable */ \
/** \{ */ \
X(const X&) = default; \
X& operator=(const X&) = default; \
X(X&&) = default; \
X& operator=(X&&) = default; \
/** \} */"
# =============================================================================
EXPAND_AS_DEFINED =
SKIP_FUNCTION_MACROS = YES
Expand Down
11 changes: 5 additions & 6 deletions systems/framework/event.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ template <class T>
class WitnessTriggeredEventData : public EventData {
public:
WitnessTriggeredEventData() {}
DRAKE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN(WitnessTriggeredEventData);
DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN(WitnessTriggeredEventData);

/// Gets the witness function that triggered the event handler.
const WitnessFunction<T>* triggered_witness() const {
Expand Down Expand Up @@ -153,6 +153,8 @@ class WitnessTriggeredEventData : public EventData {
const ContinuousState<T>* xcf_{nullptr};
};

DRAKE_DEFINE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN_T(WitnessTriggeredEventData)

/**
* Predefined types of triggers for events. Used at run time to determine why
* the associated event has occurred.
Expand Down Expand Up @@ -596,11 +598,8 @@ UnrestrictedUpdateEvent<T>::UnrestrictedUpdateEvent(
} // namespace systems
} // namespace drake

// TODO(sammy-tri) I would like to use
// DRAKE_DECLARE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS for
// WitnessTriggeredEventData, but DRAKE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN
// breaks due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57728 with
// extern templates.
DRAKE_DECLARE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
class ::drake::systems::WitnessTriggeredEventData)

DRAKE_DECLARE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
class ::drake::systems::Event)
Expand Down
3 changes: 3 additions & 0 deletions systems/framework/event_collection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ DRAKE_DEFINE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
// collection types and vice versa) it's not possible to instantiate the event
// types without the collections available. For that reason, we create the
// instances for both in this file.
DRAKE_DEFINE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
class ::drake::systems::WitnessTriggeredEventData)

DRAKE_DEFINE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
class ::drake::systems::Event)

Expand Down
2 changes: 0 additions & 2 deletions systems/framework/system_output.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#include "drake/systems/framework/system_output.h"

#include "drake/common/default_scalars.h"

DRAKE_DEFINE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
class ::drake::systems::SystemOutput)
20 changes: 13 additions & 7 deletions systems/framework/system_output.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <utility>
#include <vector>

#include "drake/common/default_scalars.h"
#include "drake/common/drake_assert.h"
#include "drake/common/drake_copyable.h"
#include "drake/systems/framework/basic_vector.h"
Expand All @@ -29,9 +30,9 @@ A `SystemOutput<T>` object can only be obtained using
template <typename T>
class SystemOutput {
public:
DRAKE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN(SystemOutput);
DRAKE_DECLARE_COPY_AND_MOVE_AND_ASSIGN(SystemOutput);

~SystemOutput() = default;
~SystemOutput();

/** Returns the number of output ports specified for this %SystemOutput
during allocation. */
Expand Down Expand Up @@ -81,7 +82,7 @@ class SystemOutput {
friend class System<T>;
friend class SystemOutputTest;

SystemOutput() = default;
SystemOutput();

// Add a suitable object to hold values for the next output port.
void add_port(std::unique_ptr<AbstractValue> model_value) {
Expand All @@ -91,10 +92,15 @@ class SystemOutput {
std::vector<copyable_unique_ptr<AbstractValue>> port_values_;
};

// Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57728 which
// should be moved back into the class definition once we no longer need to
// support GCC versions prior to 6.3.
template <typename T> SystemOutput<T>::SystemOutput() = default;
template <typename T> SystemOutput<T>::~SystemOutput() = default;
DRAKE_DEFINE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN_T(SystemOutput);

} // namespace systems
} // namespace drake

// TODO(sammy-tri) I would like to use
// DRAKE_DECLARE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS here, but
// DRAKE_DEFAULT_COPY_AND_MOVE_AND_ASSIGN breaks due to
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57728 with extern templates.
DRAKE_DECLARE_CLASS_TEMPLATE_INSTANTIATIONS_ON_DEFAULT_SCALARS(
class ::drake::systems::SystemOutput)

0 comments on commit de5d10a

Please sign in to comment.