5

I have a class to represent a 3D vector of floats:

class Vector3D
{
    public:

    float x, y, z;
    float * const data;

    Vector3D() : x(0.0), y(0.0), z(0.0), data(&x) {}
}

My question is: are x, y, and z going to be allocated sequentially in memory such that I can assign the address of x to data and later use the subscript operator on data to access the vector components as an array?

For example, sometimes I may want to access the vector components directly:

Vector3D vec;
vec.x = 42.0;
vec.y = 42.0;
vec.z = 42.0;

And sometimes I may want to access them by offset:

Vector3D vec;
for (int i = 3; i--; )
    vec.data[i] = 42.0;

Will the second example have the same effect as the first one, or do I run the risk of overwriting memory other than the x, y, and z floats?

Improve this question
11
  • An interesting idea. I'm don;t think it would be a good idea to actually do, but a very interesting question. Commented Jun 7, 2011 at 23:39
  • Instead of getting the address of x explicitly, have you tried offsetof(Vector3D, x) + this? You'll have to make sure all members of the class are aligned properly. Commented Jun 7, 2011 at 23:47
  • 1
    Just curious, why have data at all? You could just implement an operator[] on Vector3D and/or could cast the Vector3D * 'this' pointer to a "float *". You achieve the same affect, with less memory usage and syntax. Commented Jun 8, 2011 at 0:15
  • 1
    Or implement a union of a double[3] array with an x,y,z structure Commented Jun 8, 2011 at 0:18
  • @MerickOWA: You make a good point about memory savings (I will have 1000's of these objects). Overloading operator[] is something I will consider. Commented Jun 8, 2011 at 0:31

6 Answers 6

Reset to default
6

No, this is undefined behaviour, for two reasons:

  • Firstly for the padding issues that everyone else has mentioned.
  • Secondly, even if things are padded correctly, it is not valid to dereference a pointer with an offset that would take it beyond the bounds of what it's pointing to. The compiler is free to assume this, and make optimisations that would lead to undefined behaviour if you violate it.

However, the following would be valid:

class Vector3D
{
public:
    std::array<float,3> data;
    float &x, &y, &z;

    Vector3D() : data(), x(data[0]), y(data[1]), z(data[2]) { }
    Vector3D& operator =(Vector3D const& rhs) { data = rhs.data; return *this; }
};

std::array is new to C++0x, and is basically equivalent to boost::array. If you don't want C++0x or Boost, you could use a std::vector (and change the initializer to data(3)), although that's a much more heavyweight solution, its size could be modified from the outside world, and if it is, then the result would result be UB.

Improve this answer
Sign up to request clarification or add additional context in comments.

24 Comments

This would be valid but would force the user to write a custom assignment operator since one won't be defined automatically. And adding data() to the constructor's initialization list will zero-initialize its contents.
@ildjarn: You are correct. I've now modified my suggestion; vector isn't great, but array should be better!
@OliCharlesworth : I wasn't suggesting that you switch away from using a C-array, only that you should add data to the initialization list and write an appropriate copy-assignment operator. That said, I agree that std::array<>/boost::array<> is ideal here.
@ildjarn: I know. But the idea of having to write a copy-assignment operator for such a simple class bugs me! I'll put both options in...
The potential issue with this is that the class will be a lot larger than a simple float[3], as you'll be storing 3 pointers in addition to the data. This may or may not matter depending on what you're using the vectors for.
|
2

Yes. This class is layout-compatible standard-layout, because:

  • You have no virtual functions.
  • All data members are in a single access specifier block (the public:)

Because of this, it's guaranteed to be laid out sequentially just like a C structure. This is what allows you to read and write file headers as structures.

Improve this answer

32 Comments

Is padding not a potential issue? I.e., I don't see what layout-compatibility has to do with whether x, y, and z are stored contiguously in memory.
Everything you've said sounds correct. But the OP's mechanism is still UB.
@Michael : Every x64 compiler I've used will not lay those out in the same way by default. That said, agreed that nothing in the standard says those are layout-compatible.
@ildjarn: Thanks. Yeah in C++03 any user-defined constructor exempted a type from the POD layout requirements, even there is no reason for a constructor to affect layout. I believe this is why POD was split into two classifications in C++0x: trivial and standard-layout.
@Ben: This is, by definition an alias (en.wikipedia.org/wiki/Aliasing_(computing)). The question is, is it one that the compiler has to consider when it's generating optimised code? My understanding is that it's not, because you're not supposed to dereference "out-of-bounds".
|
1

or you can have an operator[] overload

float operator[](int idx)
{
 switch (idx)
{
case 0:
  return x;
case 1:
  return y;
case 2:
 return z;
}
assert (false);
}
Improve this answer

2 Comments

That should be float &. And you'd need to provide a const version as well...
Thanks, this a reasonable option.
1

The compiler has some flexibility in how it lays out the memory within a struct. The struct will never overlap another data structure, but it can inject unused space between elements. In the struct you give, some compilers might choose to add 4 bytes of extra space between z and data so that the data pointer can be aligned. Most compilers provide a way of packing everything tightly.

EDIT: There's no guarantee that the compiler will choose to pack x, y, and z tightly, but in practice they will be packed well because they are the first elements of the struct and because they're a power of two in size.

Improve this answer

1 Comment

The problem is not the gap between z and data, it's the gaps between x, y and z. There are other issues as well.
1

Your solution is not valid, but if you can ensure (or know) that your compiler will "do the right thing" (in particular by controlling padding between the x, y and z elements) you will be ok. In this case though I'd remove the data member altogether and use operator[].

I've seen something like this used on occasion. It runs into exactly the same issues, but does save you storing that data pointer, and allows for a nicer v[0] syntax rather than v.data[0].

class Vector3D
{
    public:

    float x, y, z;
    float& operator[](int i) { return *(&x+i); }
    const float& operator[](int i) const { return *(&x+i); }

    Vector3D() : x(0.0), y(0.0), z(0.0) {}
}

EDIT: Prompted by ildjam heres a compliant version using accessors rather than members, that is similar.

class Vector3D
{
    public:
      float& operator[](int i) { return v[i]; }
      const float& operator[](int i) const { return v[i]; }

      float& x() { return v[0]; }
      float  x() const { return v[0]; }
      float& y() { return v[1]; }
      float  y() const { return v[1]; }
      float& z() { return v[2]; }
      float  z() const { return v[2]; }

      Vector3D() : v() {}
    private:    
      float v[3];
};
Improve this answer

10 Comments

This suffers from the exact same UB as the OP's code -- there is no guarantee that x, y, and z are stored contiguously in memory.
Indeed, hence the "runs into exactly the same issues."
You posted an answer to demonstrate a nicer syntax for invoking UB? I.e., why post this instead of a valid, conformant implementation of Vector3D that demonstrates how to write an operator[]?
I'm posting something that I've seen used in production code, where performance, size, readability and conformance issues have been considered (and documented!). If you know what your compiler is doing with padding, or can control it using pragmas etc, then this solution may be appropriate. For this code accessing a Vector3D through v[0] and v.x produce the same machine code, and each Vector3D is not carrying around extra "cruft" that enlarges its size. Both of these are critical if you're using Vector3D inside core loops.
But you're right, thats not an answer to the explicit question. I'll update slightly.
|
-1

Do something like this:

float data[3];
float& x, y, z;

    Vector3D() : x(data[0]), y (data[1]), z(data[2]) { data [0] = data [1] = data [2] = 0;}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.