mib marked 2 inline comments as done.
mib added inline comments.

================
Comment at: lldb/examples/python/crashlog.py:669
+                             r'(0x[0-9a-fA-F]{4,}) +'               # addr (4 
chars or more)
+                             r'((.*)(?:(?: +\+ +)([0-9]+))|[^\s]+)' # symbol + 
offset
                             )
----------------
kastiglione wrote:
> kastiglione wrote:
> > mib wrote:
> > > kastiglione wrote:
> > > > mib wrote:
> > > > > @kastiglione may be you have a better idea how to handle `symbol + 
> > > > > offset` where ` + offset` might be optional.
> > > > symbol is always present?
> > > Technically, in the past the `offs` (symbol + offset) was optional:
> > > ```
> > > r'(?: +(.*))?'
> > > ```
> > > So I guess `symbol` could be missing.
> > Breaking it down, there is:
> > 
> > 1. a symbol (maybe)
> > 2. the literal text " + " followed by a number (also maybe)
> > 
> > I'll start start with 2:
> > 
> > ```
> >  \+ (\d+)
> > ```
> > 
> > For the symbol, `.*`, it should instead have at least a length of 1. I'd 
> > use `+` instead of `*`. And, it shouldn't be _any_ character. At the very 
> > least it should be non-space, which is `\S`.
> > 
> > To combine these at a high level it looks like:
> > 
> > ```
> > (?:(<symbol>)(?:<offset>)?)?
> > ```
> > 
> > Filling in these two with symbol='\S+' and offset=' \+ (\d+)', it becomes:
> > 
> > ```
> > (?:(\S+)(?: \+ (\d+))?)?
> > ```
> > 
> > Here's some python real session that minimally exercises this code:
> > 
> > ```
> > % python
> > >>> import re
> > >>> pat = re.compile(r"(?:(\S+)(?: \+ (\d+))?)?")
> > >>> pat.match("func + 123").groups()
> > ('func', '123')
> > >>> pat.match("func").groups()
> > ('func', None)
> > >>> pat.match("").groups()
> > (None, None)
> > >>> 
> > ```
> ```
> (?:(?: +\+ +)([0-9]+))
> ```
> 
> Non-capturing groups are only needed when you use a quantifier, like `?`, 
> `*`, or `+`. These regex has two non-capturing groups that don't use a 
> quantifier, which means you can remove the groups:
> 
> ```
>  +\+ +([0-9]+)
> ```
Thanks for the tips! Regarding replacing the `.+` by `\S+` doesn't work because 
the symbol name could have a space in it (for objc symbols).

I'll try to break down these regex in smaller ones to make it easier to debug 
in the future. 


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

https://reviews.llvm.org/D146765

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

Reply via email to