labath added inline comments.
================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:26
namespace {
class LineIterator {
public:
----------------
clayborg wrote:
> Move this functionality into llvm::breakpad::Line?
I haven't moved this part to the new file because (unlike everything else in
that file) this depends on the ObjectFile class. Theoretically it can be moved
to ObjectFileBreapad.h (or a new file) if needed, but ideally I'd like to avoid
anyone else needing to parse the breakpad file contents. If the minidump
process plugin for instance needed to access some of this information, I would
have it go through the SymbolFile interface, which can present it in a nicer
fashion.
================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:232
+ };
+ for (llvm::StringRef line: lines(*m_obj_file, Token::Func)) {
+ // Here we can get either FUNC records (starting with FUNC), or line
records
----------------
clayborg wrote:
> Should the iterator for Token::Func just return "FUNC" objects only? Maybe we
> add a Token::Line to the Token enumeration and then add an optional second
> parameter to the iterator? So any code that would want a "FUNC" and its
> lines, would do:
>
> ```
> for (llvm::StringRef line: lines(*m_obj_file, Token::Func, Token::Line)) {
> }
> ```
With the new parse functions this should not be necessary as you can just say
"try to parse this line as a FUNC record", and that will automatically reject
LINE record as well as any other malformed lines. I think that would make sense
if I made the iterator perform the parsing internally and return already parsed
records.
I didn't do that (for now) because returning parse errors from an iterator gets
weird, and it didn't seem worth the small amount of code it would save.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56590/new/
https://reviews.llvm.org/D56590
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
