Enable the Vec negate operator only if the component is negatable.

Robert Maynard pointed out that the unary operator- I added to Vec could
lead to undesirable behavior for vectors of unsigned integer types. This
changed makes the definition of operator- conditional on the component
type being either a signed integer or a float type.

I also added some more actual testing of the new operator.
This commit is contained in:
Kenneth Moreland 2015-06-30 13:58:56 -06:00
parent b5398babc5
commit 0b6d351a49
2 changed files with 92 additions and 8 deletions

@ -24,6 +24,11 @@
#include <vtkm/internal/Configure.h>
#include <vtkm/internal/ExportMacros.h>
#include <boost/mpl/or.hpp>
#include <boost/type_traits/is_floating_point.hpp>
#include <boost/type_traits/is_signed.hpp>
#include <boost/utility/enable_if.hpp>
/*!
* \namespace vtkm
* \brief VTKm Toolkit.
@ -857,14 +862,6 @@ public:
vtkm::internal::Divide());
}
VTKM_EXEC_CONT_EXPORT
DerivedClass operator-() const
{
return vtkm::internal::VecComponentWiseUnaryOperation<Size>()(
*reinterpret_cast<const DerivedClass*>(this),
vtkm::internal::Negate());
}
protected:
ComponentType Components[NUM_COMPONENTS];
};
@ -1127,4 +1124,22 @@ vtkm::Vec<T, Size> operator*(T scalar, const vtkm::Vec<T, Size> &vec)
vtkm::internal::BindLeftBinaryOp<T,vtkm::internal::Multiply>(scalar));
}
// The enable_if for this operator is effectively disabling the negate
// operator for Vec of unsigned integers. Another approach would be
// to use disable_if<is_unsigned>. That would be more inclusive but would
// also allow other types like Vec<Vec<unsigned> >. If necessary, we could
// change this implementation to be more inclusive.
template<typename T, vtkm::IdComponent Size>
VTKM_EXEC_CONT_EXPORT
typename boost::enable_if<
typename boost::mpl::or_<
typename boost::is_floating_point<T>::type,
typename boost::is_signed<T>::type>::type,
vtkm::Vec<T,Size> >::type
operator-(const vtkm::Vec<T,Size> &x)
{
return vtkm::internal::VecComponentWiseUnaryOperation<Size>()(
x, vtkm::internal::Negate());
}
#endif //vtk_m_Types_h

@ -58,6 +58,73 @@ void CheckTypeSizes()
VTKM_TEST_ASSERT(sizeof(vtkm::Float64) == 8, "Float32 wrong size.");
}
// This part of the test has to be broken out of GeneralVecTypeTest because
// the negate operation is only supported on vectors of signed types.
template<typename ComponentType, vtkm::IdComponent Size>
void DoGeneralVecTypeTestNegate(const vtkm::Vec<ComponentType,Size> &)
{
typedef vtkm::Vec<ComponentType,Size> VectorType;
for (vtkm::Id valueIndex = 0; valueIndex < 10; valueIndex++)
{
VectorType original = TestValue(valueIndex, VectorType());
VectorType negative = -original;
for (vtkm::IdComponent componentIndex = 0;
componentIndex < Size;
componentIndex++)
{
VTKM_TEST_ASSERT(
test_equal(-(original[componentIndex]), negative[componentIndex]),
"Vec did not negate correctly.");
}
VTKM_TEST_ASSERT(test_equal(original, -negative),
"Double Vec negative is not positive.");
}
}
template<typename ComponentType, vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<ComponentType,Size> &)
{
// Do not test the negate operator unless it is a negatable type.
}
template<vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<vtkm::Int8,Size> &x)
{
DoGeneralVecTypeTestNegate(x);
}
template<vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<vtkm::Int16,Size> &x)
{
DoGeneralVecTypeTestNegate(x);
}
template<vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<vtkm::Int32,Size> &x)
{
DoGeneralVecTypeTestNegate(x);
}
template<vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<vtkm::Int64,Size> &x)
{
DoGeneralVecTypeTestNegate(x);
}
template<vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<vtkm::Float32,Size> &x)
{
DoGeneralVecTypeTestNegate(x);
}
template<vtkm::IdComponent Size>
void GeneralVecTypeTestNegate(const vtkm::Vec<vtkm::Float64,Size> &x)
{
DoGeneralVecTypeTestNegate(x);
}
//general type test
template<typename ComponentType, vtkm::IdComponent Size>
void GeneralVecTypeTest(const vtkm::Vec<ComponentType,Size> &)
@ -144,6 +211,8 @@ void GeneralVecTypeTest(const vtkm::Vec<ComponentType,Size> &)
VTKM_TEST_ASSERT( (c != a), "operator != wrong");
VTKM_TEST_ASSERT( (a != c), "operator != wrong");
GeneralVecTypeTestNegate(T());
}
template<typename ComponentType, vtkm::IdComponent Size>