It isn’t. A Region should not outlive cache.

> On Sep 19, 2017, at 10:54 AM, David Kimura <dkim...@pivotal.io> wrote:
> 
> Oh, man. I hope I didn't just kill the original idea.  Unless we are
> somehow using shared_from_this, it shouldn't be a divergence from existing
> behavior?
> 
> Thanks,
> David
> 
>> On Tue, Sep 19, 2017 at 10:47 AM, Jacob Barrett <jbarr...@pivotal.io> wrote:
>> 
>> Region returned by this would no longer be valid. It’s “references” to
>> interns cache/region would be invalid. The behavior is undefined.
>> 
>>> On Sep 19, 2017, at 10:24 AM, David Kimura <dkim...@pivotal.io> wrote:
>>> 
>>> Err... I missed a return in example above.
>>> 
>>> Region r = [](){ return CacheFactory::create().getRegion("myregion"); }
>> ();
>>> 
>>>> On Tue, Sep 19, 2017 at 10:19 AM, David Kimura <dkim...@pivotal.io>
>> wrote:
>>>> 
>>>> I favor values, but we should probably be diligent.
>>>> 
>>>> Do any of the objects returned from Cache depend on Cache to out-live
>> the
>>>> returned object?  A quick scan suggested no, but we still have a
>>>> std::enable_shared_from_this<Cache>.  Maybe dead code.  In code
>> example,
>>>> if this happens (for any cache.get*), could we be in trouble or is this
>>>> user error?
>>>> 
>>>> Region r = [](){ CacheFactory::create().getRegion("myregion"); }();
>>>> // Cache is out of scope.
>>>> // What is expected behavior?
>>>> r.put("key", "value");
>>>> 
>>>> Thanks,
>>>> David
>>>> 
>>>> On Mon, Sep 18, 2017 at 7:44 PM, Jacob Barrett <jbarr...@pivotal.io>
>>>> wrote:
>>>> 
>>>>> 
>>>>> 
>>>>>> On Sep 18, 2017, at 7:34 PM, Kirk Lund <kl...@apache.org> wrote:
>>>>>> 
>>>>>> I would vote +1 for a more attractive, professional and user-friendly
>>>>> API.
>>>>>> I'm not sure if there's a perf or memory-usage reason for using
>>>>>> "std::shared_ptr<?>" to types instead of returning values, but the end
>>>>>> result does not look like a professional API to me.
>>>>> 
>>>>> There really isn’t, especially if you look at what we did dirty
>>>>> CacheFactory::getCache by returning a value that can be moved into the
>> heap
>>>>> and a shared point of one wants but not being forced into it. RVO
>> tricks
>>>>> can event make that move a noop.
>>>>> 
>>>>> auto r = cache.getRegion(...);
>>>>> Where decltype(r) == Region
>>>>> and
>>>>> auto rp = std::make_shared<Region>(cache->getRegion());
>>>>> Where decltype(rp) == shared_ptr<Region>
>>>>> 
>>>>> Would both be valid.
>>>>> 
>>>>> 
>>>>> 
>>>> 
>> 

Reply via email to