I have this variable definition: float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} }; I want to scale all elements of vertices, by some constant, for now I do it like this: BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7]))) { f *= scale; } But maybe something more beautiful could be done with Boost.Range?
On 12/27/2009 4:33 PM, anony wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7])))
Yikes! You can't do that. The begin and end iterators point into different arrays.
{ f *= scale; }
But maybe something more beautiful could be done with Boost.Range?
Why not just use BOOST_FOREACH correctly? BOOST_FOREACH(float (&rgf)[3], vertices) BOOST_FOREACH(float& f, rgf) f *= scale; HTH, -- Eric Niebler BoostPro Computing http://www.boostpro.com
Eric Niebler pravi:
On 12/27/2009 4:33 PM, anony wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7])))
Yikes! You can't do that. The begin and end iterators point into different arrays.
{ f *= scale; }
But maybe something more beautiful could be done with Boost.Range?
Are you certain I am wrong? The array is multidimensional and contiguous in memory. The boost::begin() and boost::end() just return pointers. I have checked if all the floats scale correctly and they do.
Why not just use BOOST_FOREACH correctly?
BOOST_FOREACH(float (&rgf)[3], vertices) BOOST_FOREACH(float& f, rgf) f *= scale;
Double loop, oops. Not fancy enough.
On 12/27/2009 7:01 PM, anony wrote:
Eric Niebler pravi:
On 12/27/2009 4:33 PM, anony wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7])))
Yikes! You can't do that. The begin and end iterators point into different arrays.
{ f *= scale; }
But maybe something more beautiful could be done with Boost.Range?
Are you certain I am wrong? The array is multidimensional and contiguous in memory. The boost::begin() and boost::end() just return pointers. I have checked if all the floats scale correctly and they do.
I'm pretty certain. You're straying into undefined behavior by making a range using iterators from different arrays.
Why not just use BOOST_FOREACH correctly?
BOOST_FOREACH(float (&rgf)[3], vertices) BOOST_FOREACH(float& f, rgf) f *= scale;
Double loop, oops. Not fancy enough.
Are you joking? -- Eric Niebler BoostPro Computing http://www.boostpro.com
Eric Niebler pravi:
Are you certain I am wrong? The array is multidimensional and contiguous in memory. The boost::begin() and boost::end() just return pointers. I have checked if all the floats scale correctly and they do. I'm pretty certain. You're straying into undefined behavior by making a range using iterators from different arrays.
The make_iterator_range() function sees only 2 pointers and these are all it cares about. Now, let's check the standard: ISO-IEC-14882 Section 8.3.4 Array, page 137: An object of array type contains a contiguously allocated non- empty set of N sub-objects of type T. The pointers I provide point to the beginning and the end of the vertices multidimensional array. The array must be contiguously allocated, as specified in the standard, hence I don't see which aspect of what I am doing in undefined, if the functions boost::begin() and boost::end() work as expected (again, they return pointers, not fancy iterator objects). Did I mention the debugger shows everything is just perfect.
Why not just use BOOST_FOREACH correctly?
BOOST_FOREACH(float (&rgf)[3], vertices) BOOST_FOREACH(float& f, rgf) f *= scale;
Double loop, oops. Not fancy enough.
Are you joking?
I was looking for fancy (i.e. elegant) code, yours is too complicated.
Patrick Horgan pravi:
anony wrote:
The make_iterator_range() function sees only 2 pointers and these are all it cares about. Now, let's check the standard:
ISO-IEC-14882 Section 8.3.4 Array, page 137:
An object of array type contains a contiguously allocated non- empty set of N sub-objects of type T.
That's referring to a dimension 1 array explicitly, so doesn't have much bearing on this. In particular a two dimension array data[2][2] is built by a compiler like this:
data[0] -=>[data[0][0]][data[0][1]] data[1] -=>[data[1][0]][data[1][1]]
It's required by the standard, as you quoted, for data[0][0] and data[0][1] to be contiguous, for data[1][0] and data[1][1] to be contiguous, and also required for the pointers data[0] and data[1] to be contiguous (although their existence might get optimized away), but not AT ALL required for data[0][1] and data[1][0] (i.e. the two rows) to be contiguous. i.e. data[0] could point at one chunk of memory for the first row, and data[1] can point at memory megabytes away and it will meet the requirements of the standard. The compiler you're using chooses to allocate the memory for all the rows in one contiguous chunk, (at least some of the time as you discovered), so you get lucky at taking advantage of implementation defined behavior. I wouldn't depend on it if I were you.
The pointers I provide point to the beginning and the end of the vertices multidimensional array. The array must be contiguously allocated, as specified in the standard, hence I don't see which aspect of what I am doing in undefined, if the functions boost::begin() and boost::end() work as expected (again, they return pointers, not fancy iterator objects).
Did I mention the debugger shows everything is just perfect.
Yes, it shows what your compiler implements in this circumstance, but tells you nothing about what will be done by other compilers, or by your own compiler in all circumstances. When you take advantage of implementation defined behavior, you're gambling with no guarantees except that if it fails it's your fault and you don't get to complain to the compiler implementor.
What I see in vertices[][3] = { ... }; is an a array object of array objects. Nowhere in the standard did I ever see multidimensional arrays specifically mentioned. Since the elements of this array are arrays, these arrays must be contiguous in memory. Anyone else wants to chime in?
anony wrote:
What I see in
vertices[][3] = { ... };
is an a array object of array objects. Nowhere in the standard did I ever see multidimensional arrays specifically mentioned. It's in: 5.3.4 New section 5 (which notes: the type of new int[i][10] is int (*)[10]), and 6 8.3.4 Arrays sections 2, 3, 4, 7, and 8 8.5.1 aggregates sections 10 and 11 on array initialization, 13.1 Overloadable Declarations section 3 which notes that Parameter declarations that differ only in a pointer * versus an array [] are equivalent. That is, the array declaration is adjusted to become a pointer declaration (8.3.5) 20.5.6.4 Array modifications, Table 39.
and on and on in many more places, all consistently saying the for a multidimensional array, all but the final dimensions are pointers.
Since the elements of this array are arrays, these arrays must be contiguous in memory. What you're missing, is that it's required in the standard (8.3.4 section 6), to consider an array as a pointer to it's first element, so an array of arrays is an array of pointers (well explained in 8.3.4 section 7 and 8). For example in gcc, this code:
#include <iostream> int main() { int array[2][2]; std::cout << std::hex << array[0] << std::endl; std::cout << array[1] << std::endl; std::cout << &array[0][0] << std::endl; std::cout << &array[1][0] << std::endl; std::cout << std::dec << sizeof(array[1]) << std::endl; std::cout << sizeof(array[1][1]) << std::endl; } prints out 0xbff3519c 0xbff351a4 0xbff3519c 0xbff351a4 8 4 on one particular run, showing that array[0], and array[1] are just the addresses of their first elements, their elements are 8 byte pointers to 4 byte ints. The pointers are the elements of the array, and as you pointed out, have to be contiguous. There's nothing in the standard that requires that the rows pointed to have to be contiguous, because they aren't the elements of the array, the pointers to them are. So, the contents of the rows have to be contiguous, and the pointers to the rows have to be contiguous, but the rows themselves don't have to be. There's nothing in the standard that requires it. You get away with it with one set of data on one compiler, because it's easy for the compiler with this data to allocate memory for all the rows at once, but nothing guarantees that it will work. Liking it, or wishing that it was the way the spec has it, doesn't help. You're relying on undefined behavior. Patrick
On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7]))) { f *= scale; }
Are you sure that is correct, you are only multiplying the first 7 floats (out of 8*3=24)... On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
But maybe something more beautiful could be done with Boost.Range?
Actually you could make it more 'beautiful' (as in shorter code) using fusion+phoenix if it really is a constant sized array like you have. This is probably not correct, but I am at work right now so cannot check: using namespace boost::fusion; using namespace boost::phoenix; using boost::array; array<array<float,3>,8> vertices = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} }; for_each(vertices,arg0*=val(scale)); // one line of code Else, if it is not a static size, then yeah, do what Eric said, even then his way is short enough that you may as well use it regardless.
On Sat, Dec 26, 2009 at 10:45 PM, OvermindDL1 <overminddl1@gmail.com> wrote:
On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7]))) { f *= scale; }
Are you sure that is correct, you are only multiplying the first 7 floats (out of 8*3=24)...
On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
But maybe something more beautiful could be done with Boost.Range?
Actually you could make it more 'beautiful' (as in shorter code) using fusion+phoenix if it really is a constant sized array like you have.
This is probably not correct, but I am at work right now so cannot check:
using namespace boost::fusion; using namespace boost::phoenix; using boost::array;
array<array<float,3>,8> vertices = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
for_each(vertices,arg0*=val(scale)); // one line of code
Else, if it is not a static size, then yeah, do what Eric said, even then his way is short enough that you may as well use it regardless.
Er, you will need to joint view the sub array parts or something. It is easy to do, I am just a bit brain-dead right now, sleep...
OvermindDL1 pravi:
On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7]))) { f *= scale; }
Are you sure that is correct, you are only multiplying the first 7 floats (out of 8*3=24)...
Nope, I am multiplying all the 24 of them (I've checked!). boost::begin() and boost::end() only return pointers (i.e., vertices[0] is an array in itself, not a single float).
OvermindDL1 pravi:
On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
I have this variable definition:
float vertices[][3] = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
I want to scale all elements of vertices, by some constant, for now I do it like this:
BOOST_FOREACH(float& f, boost::make_iterator_range( boost::begin(vertices[0]), boost::end(vertices[7]))) { f *= scale; }
Are you sure that is correct, you are only multiplying the first 7 floats (out of 8*3=24)...
On Sat, Dec 26, 2009 at 8:33 PM, anony <janezz55@gmail.com> wrote:
But maybe something more beautiful could be done with Boost.Range?
Actually you could make it more 'beautiful' (as in shorter code) using fusion+phoenix if it really is a constant sized array like you have.
This is probably not correct, but I am at work right now so cannot check:
using namespace boost::fusion; using namespace boost::phoenix; using boost::array;
array<array<float,3>,8> vertices = { {-1, -1, 1}, {1, -1, 1}, {1, 1, 1}, {-1, 1, 1}, {-1, -1, -1}, {1, -1, -1}, {1, 1, -1}, {-1, 1, -1} };
for_each(vertices,arg0*=val(scale)); // one line of code
I guess Boost.Lambda would work also in this case :) Good idea.
participants (4)
-
anony -
Eric Niebler -
OvermindDL1 -
Patrick Horgan