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