labath marked 5 inline comments as done.
labath added inline comments.

================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:393
+  llvm::StringRef LHS, RHS;
+  while (std::tie(Str, Line) = getToken(Line), !Str.empty()) {
+    if (Str.back() == ':') { // regN
----------------
clayborg wrote:
> clayborg wrote:
> > Do we really need to pull the content apart into separate strings for each 
> > register? Seems like a lot of work and 99% of these we will never accessed. 
> > Maybe just store the entire string for all registers and be done? 
> You can add an iterator method to the StackCFIRecord record maybe for when 
> you do want to parse each register?
That's a good point. I'll remove the register parsing code from this patch and 
just store the entire expression as one string(ref). I'll see what's the best 
api for splitting these up when I get to connecting this with the postfix 
expression parser.


================
Comment at: source/Plugins/ObjectFile/Breakpad/BreakpadRecords.cpp:394
+  while (std::tie(Str, Line) = getToken(Line), !Str.empty()) {
+    if (Str.back() == ':') { // regN
+      // Flush the previous expression, if there is one.
----------------
clayborg wrote:
> Does the format specify no space between the register name and the colon?
That is how I interpret the format "spec" 
<https://chromium.googlesource.com/breakpad/breakpad/+/master/docs/symbol_files.md>
```
Each registeri is the name of a register or pseudoregister. Each expression is 
a Breakpad postfix expression, which may contain spaces, but never ends with a 
colon.
```
That's not a very good definition because there are many ways one can split up 
these strings such they "don't end with a colon", but I chose this 
interpretation, as that's what made most sense to me (and it works for the 
example files I have lying around). We can always change that if we find a 
producer doing things slightly differently.


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

https://reviews.llvm.org/D60268



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to