[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a subscriber: clayborg. zturner added a comment. I know when I stepped through it with the Pi example, it was returning all matches, but not filtering down the results based on the template parameter first, so you’d get back every instantiation but the template parameter would be

Re: [Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Zachary Turner via lldb-commits
I know when I stepped through it with the Pi example, it was returning all matches, but not filtering down the results based on the template parameter first, so you’d get back every instantiation but the template parameter would be treated as a subexpression. I think the SymbolFile plugin should

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In https://reviews.llvm.org/D54454#1296392, @zturner wrote: > BTW, I will have to see if it's possible to write a test for this. Even when > I compiled and built a program with DWARF on Linux, the `target variable > Pi` example didn't "just work" for me, because

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. A dotest test can be xfailed if the debug format is not PDB, right? At least we can xfail them for all the DWARF variants so it should be possible to do that for PDB as well. So you should be able to write a test for this and then just xfail it till the DWARF parser

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. BTW, I will have to see if it's possible to write a test for this. Even when I compiled and built a program with DWARF on Linux, the `target variable Pi` example didn't "just work" for me, because `FindGlobalVariables` wasn't returning the variable. So I think this

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. In https://reviews.llvm.org/D54454#1296377, @jingham wrote: > Those seem legit things to try to capture, though a little esoteric. Since > "frame variable" and "target variable" didn't support these constructs before > you should certainly add some tests for that. > >

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. We do also handle [], though it isn't obvious to me after a quick glance where that gets done. This is a little cheesy because the name of the child that we are finding with [0] is actually "[0]", so you just have to be careful not to consume that when you consume the

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. Since we only handle . and -> this general idea seems ok to me. Do we even need a regex then? Maybe just search for first of ".-"? https://reviews.llvm.org/D54454 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Those seem legit things to try to capture, though a little esoteric. Since "frame variable" and "target variable" didn't support these constructs before you should certainly add some tests for that. The frame variable parser also supports: (lldb) frame variable

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. In https://reviews.llvm.org/D54454#1296358, @jingham wrote: > You were probably speaking loosely, but to be clear, the code you are > changing doesn't get used for expressions - i.e. the expression command - > unless I'm missing something. > > This little mini-parser

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. You were probably speaking loosely, but to be clear, the code you are changing doesn't get used for expressions - i.e. the expression command - unless I'm missing something. This little mini-parser is for doing things like: (lldb) frame variable foo.bar We don't use

[Lldb-commits] [PATCH] D54454: Be more permissive in what we consider a variable name.

2018-11-12 Thread Zachary Turner via Phabricator via lldb-commits
zturner created this revision. zturner added reviewers: clayborg, jingham, labath. Herald added subscribers: JDevlieghere, aprantl. When we evaluate a variable name as part of an expression, we run a regex against it so break it apart. The intent seems to be that we want to get the main