[PATCH] D82477: [lldb-vscode] Add Support for Module Event

[Greg Clayton via Phabricator via cfe-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So looks like you didn't use "git commit --amend -a" again here. These changes are incremental changes on your patch which hasn't been submitted yet?



================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:21
+        program_basename = "a.out.stripped"
+        program= self.getBuildArtifact(program_basename)
         self.build_and_launch(program)
----------------
add a space after "program"


================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:33-34
+        program_module = active_modules[program_basename]
+        self.assertIn('name', program_module, 'make sure name is in module')
+        self.assertEqual(program_basename, program_module['name'])
+        self.assertTrue('symbolFilePath' not in program_module, 'Make sure a.out.stripped has no debug info')
----------------
Do a similar check for ='path'
```
self.assertIn('name', program_module, 'make sure "name" is in module')
self.assertEqual(program_basename, program_module['name'])
self.assertIn('path', program_module, 'make sure "path" is in module')
self.assertEqual(program, program_module['path'])
```
In general, make sure you test every key that you have added support for.


================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:35
+        self.assertEqual(program_basename, program_module['name'])
+        self.assertTrue('symbolFilePath' not in program_module, 'Make sure a.out.stripped has no debug info')
+        symbol_path = self.getBuildArtifact("a.out")
----------------
Check for the symbol status here:
```
self.assertEqual('Symbols not found.', program_module['symbolStatus'])
```


================
Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:40
+        program_module = active_modules[program_basename]
+        self.assertEqual(program_basename, program_module['name'])
+        self.assertEqual('Symbols loaded.', program_module['symbolStatus'])
----------------
verify 'path' again


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:346
+    object.try_emplace("symbolFilePath", symbol_path);
   }
   std::string loaded_addr = std::to_string(
----------------
Add an else clause here:

```
} else {
  object.try_emplace("symbolStatus", "Symbols not found.");
}
```


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:352-357
+  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
+  for (uint32_t i=0; i<num_versions; ++i) {
+    if (!version_str.empty())
+      version_str += ".";
+    version_str += std::to_string(version_nums[i]);
+  }
----------------
fix indentation.


================
Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:358-360
+if (!version_str.empty()){
+  object.try_emplace("version", version_str);
+}
----------------
remove {} here for single line if statement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82477