dawn marked 6 inline comments as done.
dawn added a comment.
> clang-format your changes please (there are many deviations from the coding
> style)
I've made some fixes even though the code is no longer consistent with the rest
of the file. Alas. I would love to run all of lldb through clang-format, but
as we've seen, there are several options which still need to be added before we
can do that (mostly relating to the formatting function declarations).
================
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)
+ {
----------------
ki.stfu wrote:
> nit: fix indentation
This is the original code, and is consistent with the coding style of the rest
of the file. It bothers me too that lldb is so schizophrenic about its coding
style, but unless we fix all of lldb, I think it's best to just try and follow
the style of the code around you.
Does anyone else have an opinion about this? I'll go ahead and change it, only
because I see I that the new code I added didn't follow this style (oops).
================
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();
----------------
ki.stfu wrote:
> Isn't it always false?
Code elsewhere checks for it, so I assume there are cases when cu_sp can be
null. Example in source/Core/SearchFilter.cpp:
for (size_t i = 0; i < num_comp_units; i++)
{
CompUnitSP cu_sp (module_sp->GetCompileUnitAtIndex (i));
if (cu_sp)
{
Better safe than sorry.
================
Comment at: source/Commands/CommandObjectTarget.cpp:1543
@@ +1542,3 @@
+ continue;
+ CompileUnit *cu = cu_sp.get();
+ const FileSpecList &cu_file_list = cu->GetSupportFiles();
----------------
ki.stfu wrote:
> You don't need a raw pointer here, just use cu_sp.get() on line #1587
There are 5 uses of cu in this code, so I think it's cleaner to have a variable.
================
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);
----------------
ki.stfu wrote:
> combine it together:
> ```
> cu->FindLineEntry(start_idx + 1, ...)
> ```
I'd love to have a guideline as to when to wrap lines - lldb is all over the
place about this. I've tended to try to keep lines to 100.
Repository:
rL LLVM
http://reviews.llvm.org/D15593
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
