[Lldb-commits] [PATCH] D140358: [lldb-vscode] Add support for disassembly view

Fri Jan 6 02:05:06 PST 2023

eloparco added inline comments.

================
Comment at: lldb/tools/lldb-vscode/lldb-vscode.cpp:2194
+  if (index == sb_instructions.size() + 1) {
+    fprintf(stderr, "current line not found in disassembled instructions\n");
+    return response_instructions;
----------------
eloparco wrote:
> clayborg wrote:
> > eloparco wrote:
> > > clayborg wrote:
> > > > Remove any and all printf, or fprintf statements. You can't print anything to stderr or stdout as this is where the DAP packets are get emitted to. We do make it so this won't affect lldb-vscode by doing some magic with the STDOUT/STDERR file handles, but this output will be sent to /dev/null most likely. You can print something to a console (using "g_vsc.SendOutput(...)" is one way).
> > > I suppose I have to replace `llvm::errs()` too, right?
> > yes! the main issue is, will the user expect to see this output in the console and will it make sense to the user. I don't know what the user will think if they see "current line not found in disassembled instructions" in the debug console. That goes for all output to the console. It will have to make sense to the user. I don't know if the user will care and or be able to do anything about this message. It also isn't prefixed with a "warning:" or "error:". I would vote to remove it.
> > 
> Right, it is just a print to debug, is there anything else I can use for that purpose?
> Otherwise I'll just get rid of it as you were saying.
Probably better to remove it rather than prefixing the message with "debug"

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140358/new/

https://reviews.llvm.org/D140358