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

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere requested changes to 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.

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