[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

António Afonso via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu May 2 14:19:50 PDT 2019


aadsm marked an inline comment as done.
aadsm added inline comments.


================
Comment at: lldb/include/lldb/Symbol/Declaration.h:112-113
+  ///
+  /// Compares the specification from \a rhs. If the file specifications are
+  /// equal, then continue to compare the line.
+  ///
----------------
jingham wrote:
> The argument is not called "rhs" anymore.
> 
> I'm not at all sure of the value of returning > < here.  If you get > or < you have no way of knowing if that's because the files don't match, in which case so far as I can see the ordering is not useful, or if they are in the same file but with greater or lesser line number.  To actually use the < or > you would have to go back and compare the files again anyway, so for all practical purposes you're really just returning a bool...  And certainly that's how you are using it.
> 
> You were probably just copying the behavior from the function above, but that one doesn't make much sense either...
Oh, originally it was rhs but then I renamed and forgot to update the documentation.
Just to give some context: the reason I used the same interface was to provide a consistent API in the methods that do comparisons, but I see what you mean, I'll change it then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61292





More information about the lldb-commits mailing list