[Lldb-commits] [PATCH] D15593: Enhance "target modules dump line <file>" and use it to fix MI's -symbol-list-lines.

[Ilia K via lldb-commits]

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

clang-format your changes please (there are many deviations from the coding style)

I'll check it on Linux and say if everything is OK.


================
Comment at: source/Commands/CommandObjectTarget.cpp:1510
@@ -1503,2 +1509,3 @@
 
-        for (uint32_t i=0; i<num_matches; ++i)
+            for (uint32_t i=0; i<num_matches; ++i)
+            {
----------------
nit: fix indentation

================
Comment at: source/Commands/CommandObjectTarget.cpp:1536-1537
@@ -1508,1 +1535,4 @@
+            bool has_path = (file_spec.GetDirectory().AsCString() != 0);
+            int ncus = module->GetNumCompileUnits();
+            for (int i = 0; i < ncus; i++)
             {
----------------
use size_t

================
Comment at: source/Commands/CommandObjectTarget.cpp:1541-1542
@@ +1540,4 @@
+                CompUnitSP cu_sp(module->GetCompileUnitAtIndex(i));
+                if (!cu_sp)
+                    continue;
+                CompileUnit *cu = cu_sp.get();
----------------
Isn't it always false?

================
Comment at: source/Commands/CommandObjectTarget.cpp:1543
@@ +1542,3 @@
+                    continue;
+                CompileUnit *cu = cu_sp.get();
+                const FileSpecList &cu_file_list = cu->GetSupportFiles();
----------------
You don't need a raw pointer here, just use cu_sp.get() on line #1587

================
Comment at: source/Commands/CommandObjectTarget.cpp:1576
@@ +1575,3 @@
+                        assert(lldb_private::FileSpec::Equal(cu_file_spec, line_entry.file, has_path));
+                        if (cu_header_printed == false)
+                        {
----------------
!cu_header_printed

================
Comment at: source/Commands/CommandObjectTarget.cpp:1594-1596
@@ +1593,5 @@
+                        // Anymore after this one?
+                        start_idx++;
+                        start_idx = cu->FindLineEntry(start_idx, line, &cu_file_spec,
+                                                      /*exact=*/true, &line_entry);
+                    } while (start_idx != UINT32_MAX);
----------------
combine it together:
```
cu->FindLineEntry(start_idx + 1, ...)
```

================
Comment at: source/Commands/CommandObjectTarget.cpp:2689
@@ +2688,3 @@
+                default:
+                    error.SetErrorStringWithFormat ("unrecognized option %c.", short_option);
+                    break;
----------------
Please use the most popular pattern:
```
error.SetErrorStringWithFormat ("unrecognized option '%c'", short_option);
```

================
Comment at: tools/lldb-mi/MICmdCmdSymbol.cpp:85
@@ -84,7 +84,3 @@
     const CMIUtilString &strFilePath(pArgFile->GetValue());
-    // FIXME: this won't work for header files!  To try and use existing
-    // commands to get this to work for header files would be too slow.
-    // Instead, this code should be rewritten to use APIs and/or support
-    // should be added to lldb which would work for header files.
-    const CMIUtilString strCmd(CMIUtilString::Format("target modules dump line-table \"%s\"", strFilePath.AddSlashes().c_str()));
+    const CMIUtilString strCmd(CMIUtilString::Format("target modules dump lines -u false -e true -r false \"%s\"", strFilePath.AddSlashes().c_str()));
 
----------------
Could you use long-options here?


Repository:
  rL LLVM

http://reviews.llvm.org/D15593