Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Aspect/Composite-relate tests on Windows/MSVC by marking every class with virtual base with appropriate pragmas #1541

Merged
merged 16 commits into from
Mar 17, 2021
Merged
2 changes: 1 addition & 1 deletion .github/workflows/ccpp.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ jobs:
runs-on: windows-2019
strategy:
matrix:
toolset: ["", "-T ClangCl"]
toolset: [""]
env:
BUILD_TYPE: Release
VCPKG_ROOT: "C:/dartsim/vcpkg"
Expand Down
1 change: 1 addition & 0 deletions dart/common/Aspect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#include <string>

#include "dart/common/ClassWithVirtualBase.hpp"
#include "dart/common/Cloneable.hpp"
#include "dart/common/detail/NoOp.hpp"

Expand Down
47 changes: 47 additions & 0 deletions dart/common/ClassWithVirtualBase.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright (c) 2011-2021, The DART development contributors
* All rights reserved.
*
* The list of contributors can be found at:
* https://github.com/dartsim/dart/blob/master/LICENSE
*
* This file is provided under the following "BSD-style" License:
* Redistribution and use in source and binary forms, with or
* without modification, are permitted provided that the following
* conditions are met:
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
* USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/

#ifndef DART_COMMON_CLASSWITHVIRTUALBASE_HPP_
#define DART_COMMON_CLASSWITHVIRTUALBASE_HPP_

// This macro is used to mark all the class that inherit
// virtually from another to avoid problems on MSVC
// See https://github.com/dartsim/dart/issues/1522
#if defined(_MSC_VER)
# define DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN __pragma(vtordisp(push, 2))
# define DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END __pragma(vtordisp(pop))
#else
# define DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
# define DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END
#endif

#endif // DART_COMMON_CLASSWITHVIRTUALBASE_HPP_
1 change: 1 addition & 0 deletions dart/common/Composite.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#ifndef DART_COMMON_COMPOSITE_HPP_
#define DART_COMMON_COMPOSITE_HPP_

#include "dart/common/ClassWithVirtualBase.hpp"
#include "dart/common/detail/CompositeData.hpp"

namespace dart {
Expand Down
7 changes: 7 additions & 0 deletions dart/common/EmbeddedAspect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class EmbeddedStateAspect : public detail::EmbeddedStateAspect<
/// It is possible to customize the way an EmbeddedStateAspect interacts with
/// your Composite by using the dart::common::detail::EmbeddedStateAspect class
/// directly instead of inheriting this class.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN

template <class DerivedT, typename StateDataT>
class EmbedState : public virtual common::RequiresAspect<
common::EmbeddedStateAspect<DerivedT, StateDataT> >
Expand Down Expand Up @@ -114,6 +116,7 @@ class EmbedState : public virtual common::RequiresAspect<
/// Aspect::State data, directly accessible to your derived class
AspectState mAspectState;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// This is an alternative to EmbedState which allows your class to also inherit
Expand Down Expand Up @@ -193,6 +196,7 @@ class EmbeddedPropertiesAspect
/// with your Composite by using the
/// dart::common::detail::EmbeddedPropertiesAspect class directly instead of
/// inheriting this class.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
template <class DerivedT, typename PropertiesDataT>
class EmbedProperties
: public virtual common::RequiresAspect<
Expand Down Expand Up @@ -223,6 +227,7 @@ class EmbedProperties
/// Aspect::Properties data, directly accessible to your derived class
AspectProperties mAspectProperties;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// This is an alternative to EmbedProperties which allows your class to also
Expand Down Expand Up @@ -366,6 +371,7 @@ class EmbeddedStateAndPropertiesAspect
/// void setAspectState(const AspectState& state);
/// void setAspectProperties(const AspectProperties& state);
/// \endcode
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
template <class DerivedT, typename StateDataT, typename PropertiesDataT>
class EmbedStateAndProperties : public virtual common::RequiresAspect<
common::EmbeddedStateAndPropertiesAspect<
Expand Down Expand Up @@ -411,6 +417,7 @@ class EmbedStateAndProperties : public virtual common::RequiresAspect<
/// Aspect::Properties data, directly accessible to your derived class
AspectProperties mAspectProperties;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// This is an alternative to EmbedStateAndProperties which allows your class to
Expand Down
3 changes: 3 additions & 0 deletions dart/common/LocalResource.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@
#ifndef DART_COMMON_LOCALRESOURCE_HPP_
#define DART_COMMON_LOCALRESOURCE_HPP_

#include "dart/common/ClassWithVirtualBase.hpp"
#include "dart/common/Resource.hpp"

namespace dart {
namespace common {

DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class LocalResource : public virtual Resource
{
public:
Expand Down Expand Up @@ -66,6 +68,7 @@ class LocalResource : public virtual Resource
private:
std::FILE* mFile;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace common
} // namespace dart
Expand Down
3 changes: 3 additions & 0 deletions dart/common/SpecializedForAspect.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ class SpecializedForAspect
//==============================================================================
/// SpecializedForAspect allows classes that inherit Composite to have
/// constant-time access to a specific type of Aspect
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN

template <class SpecAspect>
class SpecializedForAspect<SpecAspect> : public virtual Composite
{
Expand Down Expand Up @@ -181,6 +183,7 @@ class SpecializedForAspect<SpecAspect> : public virtual Composite
/// Iterator that points to the Aspect of this SpecializedForAspect
Composite::AspectMap::iterator mSpecAspectIterator;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// This is the variadic version of the SpecializedForAspect class which
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/BodyNode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class Marker;
///
/// BodyNode inherits Frame, and a parent Frame of a BodyNode is the parent
/// BodyNode of the BodyNode.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class BodyNode
: public detail::BodyNodeCompositeBase,
public virtual BodyNodeSpecializedFor<ShapeNode, EndEffector, Marker>,
Expand Down Expand Up @@ -1174,6 +1175,7 @@ class BodyNode
/// it never gets destroyed.
std::shared_ptr<NodeDestructor> mSelfDestructor;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
3 changes: 3 additions & 0 deletions dart/dynamics/DegreeOfFreedom.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <string>
#include <Eigen/Core>

#include "dart/common/ClassWithVirtualBase.hpp"
#include "dart/common/Subject.hpp"
#include "dart/dynamics/SmartPointer.hpp"

Expand All @@ -49,6 +50,7 @@ class BodyNode;

/// DegreeOfFreedom class is a proxy class for accessing single degrees of
/// freedom (aka generalized coordinates) of the Skeleton.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class DegreeOfFreedom : public virtual common::Subject
{
public:
Expand Down Expand Up @@ -403,6 +405,7 @@ class DegreeOfFreedom : public virtual common::Subject
// DegreeOfFreedom and every DegreeOfFreedom is deleted when its Joint is
// destructed.
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
4 changes: 4 additions & 0 deletions dart/dynamics/Entity.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class Frame;
/// may have different policies about how/if their reference frame or name
/// can be changed. Use the Detachable class to create an Entity whose reference
/// Frame can be changed arbitrarily.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Entity : public virtual common::Subject
{
public:
Expand Down Expand Up @@ -228,9 +229,11 @@ class Entity : public virtual common::Subject
/// Whether or not this Entity is a Frame
bool mAmFrame;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

/// The Detachable class is a special case of the Entity base class. Detachable
/// allows the Entity's reference Frame to be changed arbitrarily by the user.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Detachable : public virtual Entity
{
public:
Expand All @@ -245,6 +248,7 @@ class Detachable : public virtual Entity
/// arguments
Detachable();
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/FixedFrame.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ namespace dynamics {
/// zero relative acceleration. It does not move within its parent Frame after
/// its relative transform is set. However, classes that inherit the FixedFrame
/// class may alter its relative transform or change what its parent Frame is.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class FixedFrame
: public virtual Frame,
public virtual common::VersionCounter,
Expand Down Expand Up @@ -95,6 +96,7 @@ class FixedFrame
// To get byte-aligned Eigen vectors
EIGEN_MAKE_ALIGNED_OPERATOR_NEW
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/Frame.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace dynamics {
/// Entity class is inherited by using virtual inheritence to solve the
/// so-called "diamond problem". Because of that, the Entity's constructor will
/// be called directly by the most derived class's constructor.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Frame : public virtual Entity
{
public:
Expand Down Expand Up @@ -378,6 +379,7 @@ class WorldFrame : public Frame
// To get byte-aligned Eigen vectors
EIGEN_MAKE_ALIGNED_OPERATOR_NEW
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/JacobianNode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class InverseKinematics;
/// The JacobianNode class serves as a common interface for BodyNodes and
/// EndEffectors to both be used as references for IK modules. This is a pure
/// abstract class.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class JacobianNode : public virtual Frame, public Node
{
public:
Expand Down Expand Up @@ -303,6 +304,7 @@ class JacobianNode : public virtual Frame, public Node
/// JacobianNode children that descend from this JacobianNode
std::unordered_set<JacobianNode*> mChildJacobianNodes;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/Joint.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class Skeleton;
class DegreeOfFreedom;

/// class Joint
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Joint : public virtual common::Subject,
public virtual common::VersionCounter,
public common::EmbedProperties<Joint, detail::JointProperties>
Expand Down Expand Up @@ -1071,6 +1072,7 @@ class Joint : public virtual common::Subject,
// To get byte-aligned Eigen vectors
EIGEN_MAKE_ALIGNED_OPERATOR_NEW
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/Node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class NodeDestructor final
///
/// In most cases, when creating your own custom Node class, you will also want
/// to inherit from AccessoryNode using CRTP.
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Node : public virtual common::Subject,
public virtual common::VersionCounter
{
Expand Down Expand Up @@ -226,6 +227,7 @@ class Node : public virtual common::Subject,
/// Index of this Node within its tree
std::size_t mIndexInTree;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// AccessoryNode provides an interface for Nodes to get their index within the
Expand Down
3 changes: 3 additions & 0 deletions dart/dynamics/Shape.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#include <Eigen/Dense>

#include "dart/common/ClassWithVirtualBase.hpp"
#include "dart/common/Deprecated.hpp"
#include "dart/common/Signal.hpp"
#include "dart/common/Subject.hpp"
Expand All @@ -47,6 +48,7 @@
namespace dart {
namespace dynamics {

DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Shape : public virtual common::Subject,
public virtual common::VersionCounter
{
Expand Down Expand Up @@ -229,6 +231,7 @@ class Shape : public virtual common::Subject,
/// Use this to subscribe to version change signals
common::SlotRegister<VersionChangedSignal> onVersionChanged;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/ShapeFrame.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ class DynamicsAspect final : public common::AspectWithVersionedProperties<
};

//==============================================================================
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class ShapeFrame : public virtual common::VersionCounter,
public detail::ShapeFrameCompositeBase,
public virtual Frame
Expand Down Expand Up @@ -281,6 +282,7 @@ class ShapeFrame : public virtual common::VersionCounter,
common::SlotRegister<RelativeTransformUpdatedSignal>
onRelativeTransformUpdated;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
2 changes: 2 additions & 0 deletions dart/dynamics/Skeleton.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ namespace dart {
namespace dynamics {

/// class Skeleton
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
class Skeleton : public virtual common::VersionCounter,
public MetaSkeleton,
public SkeletonSpecializedFor<ShapeNode, EndEffector, Marker>,
Expand Down Expand Up @@ -1337,6 +1338,7 @@ class Skeleton : public virtual common::VersionCounter,
///
std::size_t mUnionIndex;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

} // namespace dynamics
} // namespace dart
Expand Down
4 changes: 4 additions & 0 deletions dart/dynamics/SpecializedNodeManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class BodyNodeSpecializedFor
//==============================================================================
/// BodyNodeSpecializedFor allows classes that inherit BodyNode to
/// have constant-time access to a specific type of Node
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
template <class SpecNode>
class BodyNodeSpecializedFor<SpecNode>
: public virtual detail::BasicNodeManagerForBodyNode
Expand Down Expand Up @@ -102,6 +103,7 @@ class BodyNodeSpecializedFor<SpecNode>
/// Iterator that allows direct access to the specialized Nodes
BasicNodeManagerForBodyNode::NodeMap::iterator mSpecNodeIterator;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// This is the variadic version of the BodyNodeSpecializedFor class
Expand All @@ -125,6 +127,7 @@ class SkeletonSpecializedFor
//==============================================================================
/// SkeletonSpecializedForNode allows classes that inherit Skeleton to
/// have constant-time access to a specific type of Node
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_BEGIN
template <class SpecNode>
class SkeletonSpecializedFor<SpecNode>
: public virtual detail::BasicNodeManagerForSkeleton,
Expand Down Expand Up @@ -204,6 +207,7 @@ class SkeletonSpecializedFor<SpecNode>
/// Nodes
NodeNameMgrMap::iterator mSpecNodeNameMgrIterator;
};
DART_DECLARE_CLASS_WITH_VIRTUAL_BASE_END

//==============================================================================
/// This is the variadic version of the SkeletonSpecializedForNode class
Expand Down
Loading