[PATCH] D83731: Add Debug Info Size to Symbol Status

This revision was automatically updated to reflect the committed changes. Closed by commit rG986e3af53bfe: Add Debug Info Size to Symbol Status (authored by aelitashen, committed by Walter Erquinigo wall...@fb.com). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 280261. aelitashen added a comment. Fix run_test to cover every system case and attributes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731 Files:

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg added inline comments. Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61 +active_modules = self.vscode.get_active_modules() +program_module = active_modules[program_basename] +

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen marked an inline comment as done. aelitashen added inline comments. Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61 +active_modules = self.vscode.get_active_modules() +program_module =

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Looks like there is an indentation issue in the test. See inline comments. Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61 +

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 279936. aelitashen added a comment. Fix the accidentally removed test contents Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731 Files:

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 279351. aelitashen added a comment. Add a white space in Symbols loaded info. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731 Files:

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Just a space before the '(' of the debug info size. Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:357 + std::ostringstream oss; + oss << "("; + oss <<

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 279303. aelitashen added a comment. Remove white space for test_module_event Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731 Files:

[PATCH] D83731: Add Debug Info Size to Symbol Status

wallace accepted this revision. wallace added inline comments. Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:65 +@skipIfWindows +@ skipUnlessDarwin +@skipIfRemote remove this whitespace Repository: rG LLVM Github

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 278922. aelitashen added a comment. Add TODO comment for Linux, Use ostringstream for Debug Info Size message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731

[PATCH] D83731: Add Debug Info Size to Symbol Status

wallace requested changes to this revision. wallace added a comment. This revision now requires changes to proceed. The logic looks very good, just some final comments and the code will be high quality Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:66

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 278900. aelitashen added a comment. Merge if statements in GetDebugInfoSizeInSection() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731 Files:

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 278898. aelitashen added a comment. Merge Codes and Improve Readability Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/ https://reviews.llvm.org/D83731 Files:

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 278894. aelitashen added a comment. Create help function in tests for code re-use. Take care of both macOS and Linux system. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83731/new/

[PATCH] D83731: Add Debug Info Size to Symbol Status

wallace added a comment. @clayborg, the tests will fail on linux because the Makefile is expecting a .dsym file. I think it's okay if she does it just for Darwin and then I update the test to cover also linux, as I have my linux already well set up. Repository: rG LLVM Github Monorepo

[PATCH] D83731: Add Debug Info Size to Symbol Status

clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. So you should revert any clang format changes that are not in modified lines of source, mostly revert a lot of lines in lldb-vscode.cpp. Comment at:

[PATCH] D83731: Add Debug Info Size to Symbol Status

wallace added inline comments. Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:333-335 + if (section_name.startswith(".debug") || section_name.startswith("__debug")) +debug_info_size += section.GetFileByteSize(); + if (section_name.startswith(".apple") ||

[PATCH] D83731: Add Debug Info Size to Symbol Status

aelitashen updated this revision to Diff 278631. aelitashen added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. The tests added are only for macOS, Walter will help me write tests on Linux. This update optimize the code and apply git clang format.