[Lldb-commits] [PATCH] D50365: Add a new tool named "lldb-vscode" that implements the Visual Studio Code Debug Adaptor Protocol

[Pavel Labath via Phabricator via lldb-commits]

labath edited reviewers, added: zturner, davide; removed: labath.
labath added a comment.

I am not sure I'll have the resources to see this review through, so I'd prefer to leave this to someone else.

The thoughts I have had so far are:

- the patch is very big and probably runs afoul of the "you shall develop incrementally" section in the LLVM developer policy. At least the JSON parts should be split off into a separate patch and tested independently.
- however, the choice of the JSON library is also an open question. We currently have at least three options to choose from:
  - llvm/Support/JSON.h
  - lldb/Utility/JSON.h
  - debugserver/source/JSON.h

    Of these, the third one is the one I'd least expect to be used here.

- Since this is essentially starting a new-subproject, I think it's in place to discuss various conventions. E.g., right now, this seems to use a mixture of UpperCamel and snake_case, and so isn't very consistent with neither llvm nor lldb naming conventions.



================
Comment at: packages/Python/lldbsuite/test/tools/lldb-vscode/attach/TestVSCode_attach.py:47
+
+    @no_debug_info_test
+    def test_by_pid(self):
----------------
You might want to set `NO_DEBUG_INFO_TESTCASE = True` in the base `VSCodeTestCaseBase` to avoid these. I guess none of these tests should be debug-info dependent, right?


================
Comment at: tools/lldb-vscode/lldb-vscode.cpp:1688
+
+void __attribute__((format(printf, 2, 3)))
+send_formatted_output(OutputType o, const char *format, ...) {
----------------
gcc-specific attribute


https://reviews.llvm.org/D50365