codeJack
codeJack

Reputation: 2503

RVO: Return value passed by value even if explicitly assigned to a const reference

I have a settings framework, which eventually caches values storing them into an std::map of boost::any.

Since I don't want the client to deal with exceptions, it provides a default value the settings framework would fallback in case the setting retrieval fails : this forces me to return the setting value by copy.

class SettingsMgr
{

    public:
        template<class T>
        T getSetting(const std::string& settingName, const T& settingDefValue)
        {
            try
            {
                if(cache.find(settingName) != cache.end)
                {
                     return any_cast<const T&>(cache.find(settingName)->second);
                }
                else
                {
                    cache[settingName] = someDbRetrievalFunction<T>(settingName);    
                    return any_cast<const T&>(cache.find(settingName)->second);
                }
             }
             catch(...)
             {
                 return settingDefValue;
             }
          }
        // This won't work in case the default value needs to be returned 
        // because it would be a reference to a value the client - and not the SettingsMgr - 
        // owns (which might be temporary etc etc)
        template<class T>
        const T& getSettingByRef(const std::string& settingName, const T& settingDefValue);

     private:
        std::map<std::string, boost::any> cache;
}

Now, I wasn't expecting this to be a big deal since I thought that thanks to RVO magic a reference to the cached value owned by the settings framework would have been retured - especially when the client explicitly encapsulates the return value in a const reference!

According to my tests it does not seem to be the case.

void main() {
    SettingsMgr sm;
    // Assuming everything goes fine, SNAME is cached
    const std::string& asettingvalue1 = sm.getSetting<std::string>("SNAME", "DEF_VALUE"); 

    // Assuming everything goes fine, cached version is returned (no DB lookup)
    const std::string& asettingvalue2 = sm.getSetting<std::string>("SNAME", "DEF_VALUE");

    ASSERT_TRUE(&asettingvalue1 == &asettingvalue2);  // Fails

    const std::string& awrongsettingname = sm.getSettingByRef<std::string>("WRONGSETTINGNAME", "DEF_VALUE");
    ASSERT_TRUE(awrongsettingname  == "DEF_VALUE"); // Fails, awrongsettingname is random memory
}

Upvotes: 2

Views: 141

Answers (1)

x squared
x squared

Reputation: 3354

You can go with the getSettingByRef version and prevent the possibility to pass any rvalue references:

    template<class T>
    const T & getSetting(const std::string& settingName, T&& settingDefValue) {
         static_assert(false, "No rvalue references allowed!");
    }

Upvotes: 1

Related Questions