0

Is this valid/portable/legal code:

class Vector3
{
public:

  float& operator [] ( const size_t i )
  {
    assert( i < 3 );

    return *(&x+i);
  }

  float x, y, z;
};

There have been quite a few instances where I wanted to use the [] operator and ended up putting the elements in an array (to avoid if/switch statements). I never did what is being done in this particular method. I can tell why it works (x,y and z are contiguous) but is it good (or at least ok) practice?

Also, does the code require #pragma pack 1 to guarantee no-pad packing or can it work without it? Reason I ask is because the snippet is actually taken from Ogre3D vector class and I don't see #pragma pack 1 anywhere.

3 Answers 3

4

No. This is not correct and is not good practice. It is likely that there won't be padding between the three elements (even without instructing the compiler to pack the structure) and thus it is likely that you would be able to access the members as if they were elements of an array. That doesn't make the code right, though: the code as written yields undefined behavior.

A correct way to do this would be to use accessor functions for the components, e.g.

struct Vector 
{
    float& operator[](std::size_t i)       { return data_[i]; }
    float  operator[](std::size_t i) const { return data_[i]; }

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

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

private:
    float data_[3];
};
Sign up to request clarification or add additional context in comments.

Comments

0

I believe that it does actually work because the C++ compiler is not allowed to reorder members of a class/struct within a single access specifier - so they always have to be laid out x, y, z. This is there for backwards compatibility with C in which the compiler isn't allowed to reorder members at all.

However: I would argue that it's a little bit too clever for its own good. A couple of ifs or a switch inside the operator would seem cleaner to me, although I guess it is possible that they considered it performance-critical if it's an Ogre vector so that may be why it's done this way.

I don't think the packing pragma would be needed since all three members are the same size so you'd expect them to abut one another. I'm not 100% sure of that in all cases though.

Comments

0

A 64-bit machine which aligns structure elements to 64-bit boundaries would fail, likely without warning, if the code assumes 32-bit floats are contiguous.

Just because it works on all 32-bit and 16-bit architectures in all implementations that I know about doesn't make it generally useful. At best, it is a special case that works on multiple implementations. Presumably the future will bring 128-bit and 256-bit architectures where it will probably fail just as bizarrely.

Such hidden assumptions needlessly create pain and suffering for those who inherit such code—maybe even you if you forget about it. It has such a questionable expression that it is really hard to understand any meager benefit being justified as a trade for what benefit. Please, learn about the KISS principle. Know it. Live it. Love it.

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.