[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range
Jim Ingham via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu May 2 14:07:01 PDT 2019
jingham added inline comments.
Comment at: lldb/include/lldb/Symbol/Declaration.h:113
+ /// Compares the specification from \a rhs. If the file specifications are
+ /// equal, then continue to compare the line.
> How about just returning a bool:
> bool FileAndLineEqual(const Declaration &rhs);
That would be great. The whole "Compare" thing for Declarations doesn't make any sense to me. If you were trying to put Declarations in a container that required an ordering??? But that's not what this is for.
Comment at: lldb/include/lldb/Symbol/LineEntry.h:140
+ AddressRange GetSameLineContiguousAddressRange(
+ bool include_inlined_functions = false) const;
> jingham wrote:
> > We try to avoid default arguments unless there's a good reason, and in this case it looks like you pass the argument explicitly in almost all the uses, so I would make this a regular argument.
> We tried that but the patch becomes a lot bigger if so. Happy to add all the other changes back, or we can make a new fucntion?
Besides the declaration and definition, I only see 5 uses of GetSameLineContiguousAddressRange in lldb, and you've touched three of them. Am I missing something? If not, might as well change the other two.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the lldb-commits