Fixing overloads without breaking clients

Question

Here is a toy example of something I am currently working on

class Foo
{
   private:
     std::string s;
     std::vector<std::string> v;
   public:

      // Foo constructors and other big 3

      std::string const& strmember() const;
      std::vector<std::string> const& vecmember() const;
      std::vector<std::string>& vecmember();
};

All good up till this point. The requirement comes such that I have to add overloads and change the guts of the Foo as below

class Foo
{
   private:
     std::u16string ssname;
     std::vector<std::u16string> vvname;
   public:

      // Foo constructors and other big 3

      std::string const& strmember() const;
      std::vector<std::string> const& vecmember() const;
      std::vector<std::string>& vecmember();

      std::u16string const& strmember(int) const;
      std::vector<std::u16string> const& vecmember(int) const;
      std::vector<std::u16string>& vecmember(int);
};

I have to keep older members and add overloads for the new so that older clients wont break. Appropriate character conversions are performed so that older constructors are delegated to new one and the members supplied are changed as per the new design.

In implement these members as below

std::string const& Foo::strmember() const
{
    const auto& name = char_convert(ssname);
    return name; 
}
std::vector<std::string>& Foo::vecmember()
{
    return vecmember();
} 
std::vector<std::string>& Foo::vecmember()
{
    std::vector<std::string> result;
    std::transform(vvname.begin(),vvname.end(),std::back_inserter(result),[] (const std:u16string& in) -> std::string
                                                            {
                                                                return char_convert(in);
                                                            });
    return result;
}

std::u16string const& Foo::strmember(int) const
{
    return ssname;
}

std::vector<std::u16string> const& Foo::vecmember(int) const
{
    return vvname;
}

std::vector<std::u16string>& Foo::vecmember(int)
{
    return vvname;
}

When I tried to compile the changed code, compiler gives me following warning cum error

error: reference to local variable ‘result’ returned [-Werror=return-local-addr]
     std::vector<std::string> result;
                              ^~~~~~
foo.cpp: In member function ‘const string& foo::strmember() const’:
foo.cpp: error: function returns address of local variable [-Werror=return-local-addr]
     return name;
            ^~~~
foo.cpp: note: declared here
     const auto& name = char_convert(ssname);
                                                 ^
cc1plus: all warnings being treated as errors

How do I solve this problem ? I can not change the interface as it might break unittests and clients.

How do I provide the mutator and accessor version of vecmember() and strmember() functions ?

If your member functions are returning a reference then, as the error message says, you can't return a local (strictly speaking its allowed, but causes undefined behaviour, so you're lucky your compiler is reporting an error). One approach is to keep the old class members, and return a reference to them. If you insist on removing those members and returning by reference.... well, you can't have your cake and eat it too. Another option is to change the functions so they return by value instead. — Peter, Mar 12 '20 at 09:08
Depending on how your clients use your accessors, a proxy object may be a wrokaround. Highly doubtful though. — Fureeish, Mar 12 '20 at 09:10

Fareanor · Accepted Answer · 2020-03-12T14:22:39.783

Variables local to a function are destroyed when the function goes out of scope. Returning a reference to such a variable is undefined behaviour (the reference become dangling when the function returns).

I want to highlight another issue of the same type. When you write:

const auto& name = char_convert(ssname); // Bad (undefined behaviour)

You are binding a reference to a rvalue. The temporary std::string returned by char_convert() will be destroyed after that statement. So name is dangling even before being returned by the strmember() function.
You should probably write instead (copy initialization):

auto name = char_convert(ssname); // Better

Now, if you don't want to break compatibility, I think you would need to use a proxy class (as suggested by @Fureeish) to handle the conversions under the hood.
_{Here you can see an example of how to write a proxy class (if needed).}

EDIT:

As @Jarod42 mentioned in comments, const auto& name = char_convert(ssame); is indeed correct because when a temporary is bound to a reference, its lifetime will be extended to the reference's lifetime itself (until name goes out of scope in our case).

But keep in mind that name does not refer to ssname but to the temporary returned by char_convert(ssname) (so it is syntactically correct but it still does not do what you wanted it to do).

Anyway, the undefined behaviour will still occur after the function returns though.

`const auto& name = char_convert(ssname);` is correct with life time extension (which ends with variable `name`, so at end of function and so "cannot" be returned as (const) reference). — Jarod42, Mar 12 '20 at 13:10
@Jarod42 Oh I missed this point, you're right, I'll edit my answer (but please, don't remove your comment). — Fareanor, Mar 12 '20 at 14:13

Fixing overloads without breaking clients

1 Answers1