[Lldb-commits] [lldb] [lldb] Improve maintainability and readability for ValueObject methods (PR #75865)

[via lldb-commits]

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