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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
