jimingham wrote:

The reason that we use ValueObjectSP() returns rather than {} in a lot of this 
code is because the `return {}` construct came along with C++11, which was not 
available when most of this code was written.  I don't think anyone would mind 
switching over to {}.  I usually like to make it easy to see the types of 
things while reading, which `return {}` slightly obscures.  But it's usually 
pretty easy to figure out the return type of the function you are in, so in 
this case I don't know that the explicit type helps all that much.

Jim


> On Dec 19, 2023, at 11:40 AM, Pete Lawrence ***@***.***> wrote:
> 
> 
> @PortalPete commented on this pull request.
> 
> In lldb/source/Core/ValueObject.cpp 
> <https://github.com/llvm/llvm-project/pull/75865#discussion_r1431865534>:
> 
> > @@ -1700,13 +1706,13 @@ ValueObjectSP 
> > ValueObject::GetSyntheticChildAtOffset(
>      return synthetic_child_sp;
>  
>    if (!can_create)
> -    return {};
> +    return ValueObjectSP();
> This one is more about consistency because the vast majority of the remaining 
> code (in this file, anyway) use the ValueObjectSP() form.
> 
> I also have an ulterior motive in that a lot of the changes that (might) come 
> from a later PR will change MANY of these ValueObjectSP() instances to {} 
> because I convert them to empty optionals, aka std::optional<ValueObjectSP> 
> (instead of creating ValueObjectSP instances with nullptr).
> 
> Therefore, when that subsequent PR comes around, I want that changes to 
> reflect all applicable changes as possible. If I leave this line as return 
> {};, then the line won't change in the other PR, which reduces the attention 
> that method will get at that time. Not only that, this line would be 
> initializing something completely different, an optional instead of a shared 
> pointer, in that future PR.
> 
> I'd rather bring more attention to things that are functionally changing to 
> prevent accidental bugs and/or behavioral changes, specifically because the 
> compiler quietly accepts it either way.
> 
> —
> Reply to this email directly, view it on GitHub 
> <https://github.com/llvm/llvm-project/pull/75865#discussion_r1431865534>, or 
> unsubscribe 
> <https://github.com/notifications/unsubscribe-auth/ADUPVW5HTJVNU5D4OG2SNODYKHUUHAVCNFSM6AAAAABA2HRRV2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOBZGYYDCOBQGI>.
> You are receiving this because you are on a team that was mentioned.
> 



https://github.com/llvm/llvm-project/pull/75865
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to