[Lldb-commits] [lldb] [lldb] Store the return SBValueList in the CommandReturnObject (PR #127566)

[Pavel Labath via lldb-commits]

labath wrote:

> > That sort of makes sense, but it doesn't sound like you're planning to do that soon, so I'm asking about what should we do until (and if) that happens. The reason I suggested stderr is because that made sense for the commands I'm thinking of. For example, the expression command can produce a result and some warnings and fixit-applied notes. I don't know if these currently go to stderr, but I think it would be okay if they did...
> 
> I don't think it's a good idea for commands to put any of their output anywhere but the command result. Anything that goes to stderr is lost, and particularly if is trying to run their own command loop, so that they are by hand printing the command result, there will be no way to associate that output with the command that produced it.

I see that there's been a misunderstanding. By "stderr" I meant the "stderr *of* the CommandReturnObject" (i.e., `CommandReturnObject::m_err_stream`, returned by `SBCommandReturnObject::GetError`), not the actual stderr of the debugger. So the convention would be that if CommandReturnObject contains a variable, then its output (`GetOutput`) would contain a rendering of rendering of that variable (and it can be ignored when doing your own printing), but the error output (`GetError`) would contain any additional data (like the fixits) that can/should be anyway.

I'm sorry about the confusion. Does that sound better?

> We don't dump warnings on successful expression evaluation, though we do print fixits. I would suggest for the near term that if a command that produces a ValueObject returns successfully but with "notes" we should put those notes in the Error object - but still have the state be Success. That would make sense for the warnings and fixits since Adrian has been working on getting the error output into a structured form to better hold compiler warnings and errors.

I sort of like that, particularly as it lets you obtain the diagnostics in a structured form, but it also feels kinda wrong because -- if I'm not mistaken -- we are currently only setting the error field if we failed to retrieve the value. It sounds like it could confuse some code which is not expecting it.

I should also add that I don't really have a horse in this race. I have no plan on using this feature any time soon. I'm just thinking about the questions I would have if I was trying to implement it (either from the consumer or producer end). So it's possible this isn't something that's going to matter in practice...

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