[Lldb-commits] [lldb] Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected (PR #84219)

[via lldb-commits]

jimingham wrote:

I would say this differently.  Many clients of the GetNumChildren API are planning to respond the same way to an error and a number of children == 0.  Having to embed this "i'm not actually checking the error, I'm just converting it to num-children = 0" snippet looks odd, like "Why are you bothering with errors when you just discard them immediately for "value is 0".

A better title for this use of GetNumChildren would be something like GetNumChildrenErrorsAreZero.  That has the virtue of saying what's going on explicitly.

Jim

> On Mar 7, 2024, at 10:45 AM, Jonas Devlieghere ***@***.***> wrote:
> 
> 
> @JDevlieghere requested changes on this pull request.
> 
> I'm worried about making it too easy to ignore errors. The whole point of the llvm::Error class is that you have to check it. There might be a legitimate reason you don't about the error the call site, but it should very explicit and easily auditable. If none of the callers care about the error, then it shouldn't be an Error/Expected in the first place. We have plenty of examples of that in LLVM.
> 
> I totally understand the need for this as a transitional tool, but I want to make sure nobody writes new code using this. I can see two ways to make discourage that:
> 
> Mark the helper as deprecated. This might generate a lot off noise though.
> Make it really obvious in the name that this is a temporary or that we're purposely dropping the error. Either something like FIX_ERROR_HANDLING or LLDB_DROP_ERROR or something. Logging to the verbose log is better than nothing, but it doesn't constitute error handling.
> PS: The current implementation wraps the LLDB_LOG_* which uses #line and #file information. Now all those errors will get attributed to this helper function.
> 
> —
> Reply to this email directly, view it on GitHub <https://github.com/llvm/llvm-project/pull/84219#pullrequestreview-1923251331>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW6WTDMOLUAJSNVWUILYXCYVNAVCNFSM6AAAAABEJS3ASKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMRTGI2TCMZTGE>.
> You are receiving this because you are on a team that was mentioned.
> 



https://github.com/llvm/llvm-project/pull/84219