clayborg added inline comments.
================
Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.cpp:29-48
+Token breakpad::toToken(llvm::StringRef str) {
return llvm::StringSwitch<Token>(str)
.Case("MODULE", Token::Module)
.Case("INFO", Token::Info)
.Case("FILE", Token::File)
.Case("FUNC", Token::Func)
.Case("PUBLIC", Token::Public)
----------------
Move to BreakpadLine.cpp/.h?
================
Comment at: source/Plugins/ObjectFile/Breakpad/ObjectFileBreakpad.h:20-23
+enum class Token { Unknown, Module, Info, File, Func, Public, Stack };
+
+Token toToken(llvm::StringRef str);
+llvm::StringRef toString(Token t);
----------------
Should we have a "Line" class in lldb_private::breakpad::Line? All functions
that parse a breakpad line could be in this new class? Seeing as we have symbol
files, object files and the process plug-in might need to parse these lines,
seems like it would be cleaner? Maybe in BreakpadLine.cpp/.h?
```
namespace lldb_private {
namespace breakpad {
class Line {
enum class Token { Unknown, Module, Info, File, Func, Public, Stack };
Line(llvm::StringRef s);
Token parseToken();
static llvm::StringRef tokenToString(Token t);
};
```
================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:26
namespace {
class LineIterator {
public:
----------------
Move this functionality into llvm::breakpad::Line?
================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:187
+
+ auto parse = [&](llvm::StringRef line, bool is_func) {
+ // [m] address {size} param_size name
----------------
change "llvm::StringRef line" to "lldb_private::breakpad::Line" and remove
second parameter as the "Line" class can know its token type?
================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:233-240
+ // Here we can get either FUNC records (starting with FUNC), or line
records
+ // (starting with a hex number).
+ llvm::StringRef token_str;
+ std::tie(token_str, line) = getToken(line);
+ if (toToken(token_str) != Token::Func)
+ continue; // Skip line records.
+
----------------
If we make a Line class this code would become:
```
lldb_private::breakpad::Line bp_line(line);
if (bp_line.GetToken() != Token::Func)
continue;
parse(bp_line);
```
================
Comment at: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:246
+ // skip PUBLIC keyword
+ parse(getToken(line).second, false);
+ }
----------------
If we make a Line class this code would become:
```
parse(bp_line);
```
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
