Allow Variant to be trivial

Although `vtkm::internal::Variant` respected the trivially copyable
attribute of the types it contains, it was never totally trivial (i.e.
`std::is_trivial<Variant<...>>` was never true). The reason was that
`Variant` was initializing its `Index` parameter to signify that it was
not initialized. However, the fact that `Index` was initialized meant
that it was not trivially constructed.

Now, `Variant` type checks its types to see if they are all trivially
constructible. If so, it makes itself trivially constructible.

This means that `Index` may or may not be valid if `Variant` is
constructed without an argument. This in turn means that the result of
`Variant::IsValid` becomes undefined. That should be OK in practice.
`Index` will "point" to an uninitialized object, but that object is
trivially constructed anyway. However, that could cause problems if
developers used `IsValid` to determine if something is selected.
This commit is contained in:
Kenneth Moreland 2020-09-22 16:00:26 -06:00
parent 819d5f043b
commit 4a7aae86f9
5 changed files with 226 additions and 52 deletions

@ -16,7 +16,7 @@
#include <vtkm/List.h>
#include <vtkmstd/aligned_union.h>
#include <vtkmstd/is_trivially_copyable.h>
#include <vtkmstd/is_trivial.h>
namespace vtkm
{
@ -108,12 +108,74 @@ struct VariantTriviallyCopyable<vtkm::internal::Variant<Ts...>> : AllTriviallyCo
{
};
template <typename... Ts>
struct AllTriviallyConstructible;
template <>
struct AllTriviallyConstructible<> : std::true_type
{
};
template <typename T0>
struct AllTriviallyConstructible<T0>
: std::integral_constant<bool, (vtkmstd::is_trivially_constructible<T0>::value)>
{
};
template <typename T0, typename T1>
struct AllTriviallyConstructible<T0, T1>
: std::integral_constant<bool,
(vtkmstd::is_trivially_constructible<T0>::value &&
vtkmstd::is_trivially_constructible<T1>::value)>
{
};
template <typename T0, typename T1, typename T2>
struct AllTriviallyConstructible<T0, T1, T2>
: std::integral_constant<bool,
(vtkmstd::is_trivially_constructible<T0>::value &&
vtkmstd::is_trivially_constructible<T1>::value &&
vtkmstd::is_trivially_constructible<T2>::value)>
{
};
template <typename T0, typename T1, typename T2, typename T3>
struct AllTriviallyConstructible<T0, T1, T2, T3>
: std::integral_constant<bool,
(vtkmstd::is_trivially_constructible<T0>::value &&
vtkmstd::is_trivially_constructible<T1>::value &&
vtkmstd::is_trivially_constructible<T2>::value &&
vtkmstd::is_trivially_constructible<T3>::value)>
{
};
template <typename T0, typename T1, typename T2, typename T3, typename T4, typename... Ts>
struct AllTriviallyConstructible<T0, T1, T2, T3, T4, Ts...>
: std::integral_constant<bool,
(vtkmstd::is_trivially_constructible<T0>::value &&
vtkmstd::is_trivially_constructible<T1>::value &&
vtkmstd::is_trivially_constructible<T2>::value &&
vtkmstd::is_trivially_constructible<T3>::value &&
vtkmstd::is_trivially_constructible<T4>::value &&
AllTriviallyConstructible<Ts...>::value)>
{
};
template <typename VariantType>
struct VariantTriviallyConstructible;
template <typename... Ts>
struct VariantTriviallyConstructible<vtkm::internal::Variant<Ts...>>
: AllTriviallyConstructible<Ts...>
{
};
template <typename... Ts>
struct VariantStorageImpl
{
typename vtkmstd::aligned_union<0, Ts...>::type Storage;
vtkm::IdComponent Index = -1;
vtkm::IdComponent Index;
template <vtkm::IdComponent Index>
using TypeAt = typename vtkm::ListAt<vtkm::List<Ts...>, Index>;
@ -125,7 +187,10 @@ struct VariantStorageImpl
}
VTKM_EXEC_CONT vtkm::IdComponent GetIndex() const noexcept { return this->Index; }
VTKM_EXEC_CONT bool IsValid() const noexcept { return this->GetIndex() >= 0; }
VTKM_EXEC_CONT bool IsValid() const noexcept
{
return (this->Index >= 0) && (this->Index < static_cast<vtkm::IdComponent>(sizeof...(Ts)));
}
VTKM_EXEC_CONT void Reset() noexcept
{
@ -166,11 +231,14 @@ struct VariantStorageImpl
};
template <typename VariantType,
typename TriviallyConstructible =
typename VariantTriviallyConstructible<VariantType>::type,
typename TriviallyCopyable = typename VariantTriviallyCopyable<VariantType>::type>
struct VariantConstructorImpl;
// Can trivially construct, deconstruct, and copy all data. (Probably all trivial classes.)
template <typename... Ts>
struct VariantConstructorImpl<vtkm::internal::Variant<Ts...>, std::true_type>
struct VariantConstructorImpl<vtkm::internal::Variant<Ts...>, std::true_type, std::true_type>
: VariantStorageImpl<Ts...>
{
VariantConstructorImpl() = default;
@ -182,12 +250,29 @@ struct VariantConstructorImpl<vtkm::internal::Variant<Ts...>, std::true_type>
VariantConstructorImpl& operator=(VariantConstructorImpl&&) = default;
};
// Can trivially copy, but cannot trivially construct. Common if a class is simple but
// initializes itself.
template <typename... Ts>
struct VariantConstructorImpl<vtkm::internal::Variant<Ts...>, std::false_type>
struct VariantConstructorImpl<vtkm::internal::Variant<Ts...>, std::false_type, std::true_type>
: VariantStorageImpl<Ts...>
{
VariantConstructorImpl() = default;
VTKM_EXEC_CONT VariantConstructorImpl() { this->Index = -1; }
// Any trivially copyable class is trivially destructable.
~VariantConstructorImpl() = default;
VariantConstructorImpl(const VariantConstructorImpl&) = default;
VariantConstructorImpl(VariantConstructorImpl&&) = default;
VariantConstructorImpl& operator=(const VariantConstructorImpl&) = default;
VariantConstructorImpl& operator=(VariantConstructorImpl&&) = default;
};
// Cannot trivially copy. We assume we cannot trivially construct/destruct.
template <typename construct_type, typename... Ts>
struct VariantConstructorImpl<vtkm::internal::Variant<Ts...>, construct_type, std::false_type>
: VariantStorageImpl<Ts...>
{
VTKM_EXEC_CONT VariantConstructorImpl() { this->Index = -1; }
VTKM_EXEC_CONT ~VariantConstructorImpl() { this->Reset(); }
VTKM_EXEC_CONT VariantConstructorImpl(const VariantConstructorImpl& src) noexcept
@ -239,16 +324,20 @@ class Variant : detail::VariantConstructorImpl<Variant<Ts...>>
public:
/// Returns the index of the type of object this variant is storing. If no object is currently
/// stored (i.e. the Variant is invalid), -1 is returned.
/// stored (i.e. the `Variant` is invalid), an invalid is returned.
///
VTKM_EXEC_CONT vtkm::IdComponent GetIndex() const noexcept
{
return this->Superclass::GetIndex();
}
/// Returns true if this Variant is storing an object from one of the types in the template
/// Returns true if this `Variant` is storing an object from one of the types in the template
/// list, false otherwise.
///
/// Note that if this `Variant` was not initialized with an object, the result of `IsValid`
/// is undefined. The `Variant` could report itself as validly containing an object that
/// is trivially constructed.
///
VTKM_EXEC_CONT bool IsValid() const noexcept { return this->Superclass::IsValid(); }
/// Type that converts to a std::integral_constant containing the index of the given type (or

@ -23,6 +23,12 @@ struct TypePlaceholder
{
};
// A class that is trivially copiable but not totally trivial.
struct TrivialCopy
{
vtkm::Id Value = 0;
};
void TestSize()
{
std::cout << "Test size" << std::endl;
@ -171,13 +177,42 @@ void TestTriviallyCopyable()
{
#ifndef VTKM_USING_GLIBCXX_4
// Make sure base types are behaving as expected
VTKM_STATIC_ASSERT(std::is_trivially_constructible<float>::value);
VTKM_STATIC_ASSERT(std::is_trivially_copyable<float>::value);
VTKM_STATIC_ASSERT(std::is_trivial<float>::value);
VTKM_STATIC_ASSERT(std::is_trivially_constructible<int>::value);
VTKM_STATIC_ASSERT(std::is_trivially_copyable<int>::value);
VTKM_STATIC_ASSERT(std::is_trivial<int>::value);
VTKM_STATIC_ASSERT(!std::is_trivially_constructible<std::shared_ptr<float>>::value);
VTKM_STATIC_ASSERT(!std::is_trivially_copyable<std::shared_ptr<float>>::value);
VTKM_STATIC_ASSERT(!std::is_trivial<std::shared_ptr<float>>::value);
VTKM_STATIC_ASSERT(!std::is_trivially_constructible<TrivialCopy>::value);
VTKM_STATIC_ASSERT(std::is_trivially_copyable<TrivialCopy>::value);
VTKM_STATIC_ASSERT(!std::is_trivial<TrivialCopy>::value);
// A variant of trivially constructable things should be trivially constructable
VTKM_STATIC_ASSERT((vtkm::internal::detail::AllTriviallyConstructible<float, int>::value));
VTKM_STATIC_ASSERT((std::is_trivially_constructible<vtkm::internal::Variant<float, int>>::value));
// A variant of trivially copyable things should be trivially copyable
VTKM_STATIC_ASSERT((vtkm::internal::detail::AllTriviallyCopyable<float, int>::value));
VTKM_STATIC_ASSERT((std::is_trivially_copyable<vtkm::internal::Variant<float, int>>::value));
VTKM_STATIC_ASSERT(
(vtkm::internal::detail::AllTriviallyCopyable<float, int, TrivialCopy>::value));
VTKM_STATIC_ASSERT(
(std::is_trivially_copyable<vtkm::internal::Variant<float, int, TrivialCopy>>::value));
// A variant of any non-trivially constructable things is not trivially copyable
VTKM_STATIC_ASSERT((
!vtkm::internal::detail::AllTriviallyConstructible<std::shared_ptr<float>, float, int>::value));
VTKM_STATIC_ASSERT((
!vtkm::internal::detail::AllTriviallyConstructible<float, std::shared_ptr<float>, int>::value));
VTKM_STATIC_ASSERT((
!vtkm::internal::detail::AllTriviallyConstructible<float, int, std::shared_ptr<float>>::value));
VTKM_STATIC_ASSERT((!std::is_trivially_constructible<
vtkm::internal::Variant<std::shared_ptr<float>, float, int>>::value));
VTKM_STATIC_ASSERT((!std::is_trivially_constructible<
vtkm::internal::Variant<float, std::shared_ptr<float>, int>>::value));
VTKM_STATIC_ASSERT((!std::is_trivially_constructible<
vtkm::internal::Variant<float, int, std::shared_ptr<float>>>::value));
// A variant of any non-trivially copyable things is not trivially copyable
VTKM_STATIC_ASSERT(
@ -192,6 +227,12 @@ void TestTriviallyCopyable()
vtkm::internal::Variant<float, std::shared_ptr<float>, int>>::value));
VTKM_STATIC_ASSERT((!std::is_trivially_copyable<
vtkm::internal::Variant<float, int, std::shared_ptr<float>>>::value));
// A variant of trivial things should be trivial
VTKM_STATIC_ASSERT((std::is_trivial<vtkm::internal::Variant<float, int>>::value));
VTKM_STATIC_ASSERT((!std::is_trivial<vtkm::internal::Variant<float, int, TrivialCopy>>::value));
VTKM_STATIC_ASSERT(
(!std::is_trivial<vtkm::internal::Variant<float, int, std::shared_ptr<float>>>::value));
#endif // !VTKM_USING_GLIBCXX_4
}

@ -11,7 +11,7 @@
set(headers
aligned_union.h
integer_sequence.h
is_trivially_copyable.h
is_trivial.h
void_t.h
)

85
vtkmstd/is_trivial.h Normal file

@ -0,0 +1,85 @@
//============================================================================
// Copyright (c) Kitware, Inc.
// All rights reserved.
// See LICENSE.txt for details.
//
// This software is distributed WITHOUT ANY WARRANTY; without even
// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
// PURPOSE. See the above copyright notice for more information.
//============================================================================
#ifndef vtk_m_std_is_trivial_h
#define vtk_m_std_is_trivial_h
#include <vtkm/StaticAssert.h>
#include <type_traits>
#if defined(VTKM_USING_GLIBCXX_4)
namespace vtkmstd
{
// GCC 4.8 and 4.9 standard library does not support std::is_trivially_copyable.
// There is no relyable way to get this information (since it has to come special from
// the compiler). For our purposes, we will report as nothing being trivially copyable,
// which causes us to call the constructors with everything. This should be fine unless
// some other part of the compiler is trying to check for trivial copies (perhaps nvcc
// on top of GCC 4.8).
template <typename>
struct is_trivially_copyable : std::false_type
{
};
// I haven't tried the other forms of is_trivial, but let's just assume they don't
// work as expected.
template <typename...>
struct is_trivially_constructible : std::false_type
{
};
template <typename>
struct is_trivially_destructible : std::false_type
{
};
template <typename>
struct is_trivial : std::false_type
{
};
// A common exception to reporting nothing as trivially copyable is assertions that
// a class is trivially copyable. If we have code that _only_ works with trivially
// copyable classes, we don't want to report nothing as trivially copyably, because
// that will error out for everything. For this case, we define the macro
// `VTKM_IS_TRIVIALLY_COPYABLE`, which will always pass on compilers that don't
// support is_trivially_copyable, but will do the correct check on compilers that
// do support it.
#define VTKM_IS_TRIVIALLY_COPYABLE(type) VTKM_STATIC_ASSERT(true)
#define VTKM_IS_TRIVIALLY_CONSTRUCTIBLE(...) VTKM_STATIC_ASSERT(true)
#define VTKM_IS_TRIVIALLY_DESTRUCTIBLE(...) VTKM_STATIC_ASSERT(true)
#define VTKM_IS_TRIVIAL(type) VTKM_STATIC_ASSERT(true)
} // namespace vtkmstd
#else // NOT VTKM_USING_GLIBCXX_4
namespace vtkmstd
{
using std::is_trivial;
using std::is_trivially_constructible;
using std::is_trivially_copyable;
using std::is_trivially_destructible;
#define VTKM_IS_TRIVIALLY_COPYABLE(type) \
VTKM_STATIC_ASSERT_MSG(::vtkmstd::is_trivially_copyable<type>::value, \
"Type must be trivially copyable to be used here.")
#define VTKM_IS_TRIVIALLY_CONSTRUCTIBLE(...) \
VTKM_STATIC_ASSERT_MSG(::vtkmstd::is_trivially_constructible<__VA_ARGS__>::value, \
"Type must be trivially constructible to be used here.")
#define VTKM_IS_TRIVIALLY_DESTRUCTIBLE(...) \
VTKM_STATIC_ASSERT_MSG(::vtkmstd::is_trivially_destructible<__VA_ARGS__>::value, \
"Type must be trivially constructible to be used here.")
#define VTKM_IS_TRIVIAL(type) \
VTKM_STATIC_ASSERT_MSG(::vtkmstd::is_trivial<type>::value, \
"Type must be trivial to be used here.")
} // namespace vtkmstd
#endif
#endif //vtk_m_std_is_trivial_h

@ -1,41 +0,0 @@
//============================================================================
// Copyright (c) Kitware, Inc.
// All rights reserved.
// See LICENSE.txt for details.
//
// This software is distributed WITHOUT ANY WARRANTY; without even
// the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
// PURPOSE. See the above copyright notice for more information.
//============================================================================
#ifndef vtk_m_std_is_trivially_copyable_h
#define vtk_m_std_is_trivially_copyable_h
#include <type_traits>
#if defined(VTKM_USING_GLIBCXX_4)
namespace vtkmstd
{
// GCC 4.8 and 4.9 standard library does not support std::is_trivially_copyable.
// There is no relyable way to get this information (since it has to come special from
// the compiler). For our purposes, we will report as nothing being trivially copyable,
// which causes us to call the constructors with everything. This should be fine unless
// some other part of the compiler is trying to check for trivial copies (perhaps nvcc
// on top of GCC 4.8).
template <typename>
struct is_trivially_copyable : std::false_type
{
};
} // namespace vtkmstd
#else // NOT VTKM_USING_GLIBCXX_4
namespace vtkmstd
{
using std::is_trivially_copyable;
} // namespace vtkmstd
#endif
#endif //vtk_m_std_is_trivially_copyable_h