Invalid values returned by ArrayProperty::get with std::vector<bool>
AntAgna opened this issue · 5 comments
I am trying to use std::vector<> with Ponder.
If I have a std::vector of bool and I use ArrayProperty::get(const UserObject& object, std::size_t index), I receive invalid values.
The same thing with a std::vector of int works correctly.
I attach example code.
Another note : The compiler generates warning C4172 (returning address of local variable or temporary) when a std::vector is declared.
It is as if the vector was returned by value at some point instead of by reference.
I am wondering if it may be due to a missing template specialization or to a bug in the compiler.
I am building using Visual Studio 2015.
I will try another compiler tomorrow.
The problem also happens with gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
The warning message is more explicit :
In file included from include/ponder/valuemapper.hpp:38:0,
from include/ponder/value.hpp:37,
from include/ponder/tagholder.hpp:36,
from include/ponder/property.hpp:35,
from include/ponder/arrayproperty.hpp:35,
from main.cpp:4:
include/ponder/arraymapper.hpp: In instantiation of ‘static const T& ponder_ext::ArrayMapper<std::vector<_RealType> >::get(const std::vector<_RealType>&, std::size_t) [with T = bool; std::size_t = long unsigned int]’:
include/ponder/detail/arraypropertyimpl.inl:68:23: required from ‘ponder::Value ponder::detail::ArrayPropertyImpl<A>::getElement(const ponder::UserObject&, std::size_t) const [with A = ponder::detail::Accessor1<ExampleStruct, std::vector<bool>&, void>; std::size_t = long unsigned int]’
main.cpp:77:1: required from here
include/ponder/arraymapper.hpp:258:25: warning: returning reference to temporary [-Wreturn-local-addr]
return arr[index];
^
When running the program built by gcc, the result is : Segmentation fault (core dumped)
Aha, I found the cause.
std::vector<bool>
is a specialization of std::vector<T>.
In this specialization, array subscript does not return a reference : typedef bool const_reference;
This is documented here : http://en.cppreference.com/w/cpp/container/vector_bool
We need to return bool instead of bool&
in ArrayMapper::get()
#include <ponder/classget.hpp>
#include <ponder/errors.hpp>
#include <ponder/arrayproperty.hpp>
#include <ponder/class.hpp>
#include <ponder/classbuilder.hpp>
struct ExampleStruct
{
static void declare(); // For introspection
std::vector<int> Ints = { 1, 2, 3, 4 };
std::vector<bool> Bools = { false, true, false, true };
};
PONDER_AUTO_TYPE(ExampleStruct, &ExampleStruct::declare) // Will automatically call declare()
inline void ExampleStruct::declare()
{
// Declare structure members
ponder::Class::declare< ExampleStruct >("ExampleStruct")
.property("Ints", &ExampleStruct::Ints)
.property("Bools", &ExampleStruct::Bools) // This causes the following warning: C4172: returning address of local variable or temporary
;
}
int main()
{
ExampleStruct Instance;
const ponder::Class& Class = ponder::classByType<ExampleStruct>();
ponder::UserObject Object(Instance);
{
// Test with int array
const ponder::Property& Prop = Class.property("Ints");
const ponder::ArrayProperty& ArrayProp = dynamic_cast<const ponder::ArrayProperty&>(Prop);
std::vector<int> Copy;
for (size_t i = 0; i < ArrayProp.size(Object); i++)
{
ponder::Value value = ArrayProp.get(Object, i);
int v = value.to<int>();
Copy.push_back(v);
}
assert(Copy == ExampleStruct().Ints); // Works correctly
}
{
// Test with bool array
const ponder::Property& Prop = Class.property("Bools");
const ponder::ArrayProperty& ArrayProp = dynamic_cast<const ponder::ArrayProperty&>(Prop);
std::vector<bool> Copy;
for (size_t i = 0; i < ArrayProp.size(Object); i++)
{
ponder::Value value = ArrayProp.get(Object, i);
bool v = value.to<bool>();
Copy.push_back(v);
}
assert(Copy == ExampleStruct().Bools); // Fails here (Copy is all true)
}
return 0;
}
Good, work. Thanks for looking into this.
Specialization for bool
The Standard Library defines a specialization of the vector template for bool. The description of this specialization indicates that the implementation should pack the elements so that every bool only uses one bit of memory.[8] This is widely considered a mistake.[9][10] vector does not meet the requirements for a C++ Standard Library container. For instance, a container::reference must be a true lvalue of type T. This is not the case with vector::reference, which is a proxy class convertible to bool.[11] Similarly, the vector::iterator does not yield a bool& when dereferenced. There is a general consensus among the C++ Standard Committee and the Library Working Group that vector should be deprecated and subsequently removed from the standard library, while the functionality will be reintroduced under a different name.[12]
from wikipedia.