overload array operator with templates and pointers

Question

The following code compiles (without warnings) on both clang++-2.9 and g++-4.6. However, the g++ binary Seg Faults, while the clang++ binary runs as intended.

What is the proper way to access template class data members through pointers when overloading []?

Here's the code:

#include <iostream>
template <typename T>
    class A {
    private:
    T val1;
    T val2;

    public:
    T& getVal1() { return val1; }
    void setVal1(T aVal) { val1 = aVal; }
    T& getVal2() { return val2; }
    void setVal2(T aVal) { val2 = aVal; }
};

template <typename T>
    class B {
    private:
    A<T>* aPtr;

    public:
    A<T>* getAPtr() { return aPtr; }
    T& operator[](const int& key) {
    if(key == 0) { T& res = getAPtr()->getVal1();
        return res; }
    else { T& res = getAPtr()->getVal2();
        return res; }
    }
};

int main()
{
    B<int> foo;
    foo[0] = 1;
    int x = foo[0];
    std::cout << foo[0] << " " << x << std::endl; // 1 1
}

possible duplicate of [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) — Flexo, Oct 03 '11 at 08:11
`aPtr` is uninitialized, therefore dereferencing it is undefined behavior. — Philipp, Oct 03 '11 at 08:37

sehe · Accepted Answer · 2011-10-03T08:26:05.870

0

You are returning a reference to a local variable (res). The reference won't be valid after returning from operator[]. It could be overwritten by other stuff. What really happens is Undefined: that is why compilers are allowed to eat your children or grow a moustache: Undefined Behaviour

You probably want to return by value.

Edit

Since you have a setter, you don't need the reference: See the solution live at http://ideone.com/oxslQ

Note: there was another problem with aPtr not being initialized. I proposed a simple constructor for that. _You might want to initialize this from elsewhere OR you need

assignment and copy constructors
or use a shared_ptr for aPtr

.

#include <iostream>

template <typename T>
class A
{
private:
    T val1;
    T val2;

public:
    T getVal1()
    {
        return val1;
    }
    void setVal1(T aVal)
    {
        val1 = aVal;
    }
    T getVal2()
    {
        return val2;
    }
    void setVal2(T aVal)
    {
        val2 = aVal;
    }
};

template <typename T>
class B
{
private:
    A<T>* aPtr;
    B(const B&);            // TODO , disallow for now
    B& operator=(const B&); // TODO , disallow for now

public:
    B() : aPtr(new A<T>()) {}
    ~B() { delete aPtr; }

    A<T>* getAPtr()
    {
        return aPtr;
    }
    T operator[](const int& key)
    {
        if(key == 0)
        {
            T res = getAPtr()->getVal1();
            return res;
        }
        else
        {
            T res = getAPtr()->getVal2();
            return res;
        }
    }
};

int main()
{
    B<int> foo;
    foo.getAPtr()->setVal1(1);
    int x = foo[0];
    std::cout << foo[0] << " " << x << std::endl; // 1 1
}

edited Oct 03 '11 at 08:26

answered Oct 03 '11 at 08:05

sehe

374,641
47
450
633

That's not quite enough for the reference version as `getVal1` and `getVal2` return values, not references, so those have to be changed as well. – Luc Danton Oct 03 '11 at 08:08
The first solution gives `simplerpriv.cc:33:14: error: lvalue required as left operand of assignment'` and the second gives `simplerpriv.cc:23:45: error: invalid initialization of non-const reference of type ‘int&’ from an rvalue of type ‘int’` – Jeremiah Oct 03 '11 at 08:12
@Jeremiah: If you wanted to return by reference you need to return references from getVal1() etc. Currently, that is returning values. – sehe Oct 03 '11 at 08:17
@LucDanton: went all the way and fixed the problem using the setter. See http://ideone.com/oxslQ – sehe Oct 03 '11 at 08:26
@sehe Thanks. `T& getVal1()` gets rid of the compiler warnings, but the g++ binary still seg faults. I will update the question. – Jeremiah Oct 03 '11 at 08:31
@Jeremiah: Oops! the answer has been extended several times, 4 minutes _before your comment_ and you still haven't even seen it? I do think you should read it now, before updating your question. Also note I posted working code here: http://ideone.com/oxslQ ?! – sehe Oct 03 '11 at 08:32
@sehe Sorry! I didn't see the part you added with B() and ~B(). That did it. Thank you! – Jeremiah Oct 03 '11 at 08:37

score 0 · Answer 2 · answered Oct 03 '11 at 08:26

If you want to return by ref, then your A::getValX() functions should also return by ref, and your res variable inside B::operator should also be T& instead of T:

#include <iostream>
template <typename T>
    class A {
    private:
    T val1;
    T val2;

    public:
    T& getVal1() { return val1; }
    void setVal1(T aVal) { val1 = aVal; }
    T& getVal2() { return val2; }
    void setVal2(T aVal) { val2 = aVal; }
};

template <typename T>
    class B {
    private:
    A<T>* aPtr;

    public:
    A<T>* getAPtr() { return aPtr; }
    T& operator[](const int& key) {
        if(key == 0) { T& res = getAPtr()->getVal1();
            return res; }
        else { T& res = getAPtr()->getVal2();
            return res; }
    }
};

int main()
{
    B<int> foo;
    foo[0] = 1;
    int x = foo[0];
    std::cout << foo[0] << " " << x << std::endl; // 1 1
}

(Note that it will still crash at runtime, since aPtr isn't initialized anywhere.)

Your original code returns a reference to the local variable res, not to A::val1 / A::val2 as you probably intended. If res is a non-reference variable, then it will be a simple copy of the val1 / val2 value, that is only valid for inside the scope (in this case the function) where it was declared. So you need a reference here.

overload array operator with templates and pointers

2 Answers2

Edit