[Lldb-commits] [lldb] r282135 - Fix parsing expressions to evaluate with spaces and optional args (MI)

[Ilia K via lldb-commits]

Author: ki.stfu
Date: Thu Sep 22 00:08:41 2016
New Revision: 282135

URL: http://llvm.org/viewvc/llvm-project?rev=282135=rev
Log:
Fix parsing expressions to evaluate with spaces and optional args (MI)

Summary:
When extracting options for long options (starting with `--`), the use of
`MIUtilString::SplitConsiderQuotes` to split all the arguments was being
conditioned on the option type to be expected. This was wrong as this caused
other options to be parsed incorrectly since it was not taking into account the
presence of quotes.

Patch by Ed Munoz 

Reviewers: edmunoz, ki.stfu

Subscribers: ki.stfu, lldb-commits

Projects: #lldb

Differential Revision: https://reviews.llvm.org/D24202

Modified:

lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py?rev=282135=282134=282135=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py 
Thu Sep 22 00:08:41 2016
@@ -61,7 +61,7 @@ class MiVarTestCase(lldbmi_testcase.MiTe
 self.expect("\^done,value=\"30\"")
 self.runCmd("-var-update --all-values var2")
 # 
self.expect("\^done,changelist=\[\{name=\"var2\",value=\"30\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var2")
 self.expect("\^done")
 self.runCmd("-var-create var2 * g_MyVar")
@@ -84,7 +84,7 @@ class MiVarTestCase(lldbmi_testcase.MiTe
 self.expect("\^done,value=\"3\"")
 self.runCmd("-var-update --all-values var3")
 # 
self.expect("\^done,changelist=\[\{name=\"var3\",value=\"3\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var3")
 self.expect("\^done")
 self.runCmd("-var-create var3 * s_MyVar")
@@ -107,7 +107,7 @@ class MiVarTestCase(lldbmi_testcase.MiTe
 self.expect("\^done,value=\"2\"")
 self.runCmd("-var-update --all-values var4")
 # 
self.expect("\^done,changelist=\[\{name=\"var4\",value=\"2\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var4")
 self.expect("\^done")
 self.runCmd("-var-create var4 * b")
@@ -148,6 +148,13 @@ class MiVarTestCase(lldbmi_testcase.MiTe
 self.expect(
 
"\^done,numchild=\"1\",children=\[child=\{name=\"var6\.\*\$[0-9]+\",exp=\"\*\$[0-9]+\",numchild=\"0\",type=\"const
 char\",thread-id=\"4294967295\",value=\"47 
'/'\",has_more=\"0\"\}\],has_more=\"0\"")
 
+# Print an expression with spaces and optional arguments
+self.runCmd("-data-evaluate-expression \"a + b\"")
+self.expect("\^done,value=\"12\"")
+self.runCmd("-var-create var7 * \"a + b\" --thread 1 --frame 0")
+self.expect(
+
"\^done,name=\"var7\",numchild=\"0\",value=\"12\",type=\"int\",thread-id=\"1\",has_more=\"0\"")
+
 @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races
 @skipIfLinux  # llvm.org/pr22841: lldb-mi tests fail on all Linux buildbots
@@ -315,12 +322,14 @@ class MiVarTestCase(lldbmi_testcase.MiTe
 # Test that -var-list-children lists all children with their values
 # (and that from and to are optional)
 self.runCmd("-var-list-children --all-values var_complx")
-
self.expect("\^done,numchild=\"3\",children=\[child=\{name=\"var_complx\.i\",exp=\"i\",numchild=\"0\",type=\"int\",thread-id=\"1\",value=\"3\",has_more=\"0\"\},child=\{name=\"var_complx\.inner\",exp=\"inner\",numchild=\"1\",type=\"complex_type::\(anonymous
 
struct\)\",thread-id=\"1\",value=\"\{\.\.\.\}\",has_more=\"0\"\},child=\{name=\"var_complx\.complex_ptr\",exp=\"complex_ptr\",numchild=\"3\",type=\"complex_type
 \*\",thread-id=\"1\",value=\"0x[0-9a-f]+\",has_more=\"0\"\}\],has_more=\"0\"")
+self.expect(
+
"\^done,numchild=\"3\",children=\[child=\{name=\"var_complx\.i\",exp=\"i\",numchild=\"0\",type=\"int\",thread-id=\"1\",value=\"3\",has_more=\"0\"\},child=\{name=\"var_complx\.inner\",exp=\"inner\",numchild=\"1\",type=\"complex_type::\(anonymous
 

Re: [Lldb-commits] [PATCH] D24202: Fix parsing expressions to evaluate with spaces and optional args (MI)

[Ilia K via lldb-commits]

This revision was automatically updated to reflect the committed changes.
ki.stfu marked an inline comment as done.
Closed by commit rL282135: Fix parsing expressions to evaluate with spaces and 
optional args (MI) (authored by ki.stfu).

Changed prior to commit:
  https://reviews.llvm.org/D24202?vs=72136=72137#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24202

Files:
  lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
  lldb/trunk/tools/lldb-mi/MICmdArgValOptionLong.cpp

Index: 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
===
--- 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
+++ 
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
@@ -61,7 +61,7 @@
 self.expect("\^done,value=\"30\"")
 self.runCmd("-var-update --all-values var2")
 # 
self.expect("\^done,changelist=\[\{name=\"var2\",value=\"30\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var2")
 self.expect("\^done")
 self.runCmd("-var-create var2 * g_MyVar")
@@ -84,7 +84,7 @@
 self.expect("\^done,value=\"3\"")
 self.runCmd("-var-update --all-values var3")
 # 
self.expect("\^done,changelist=\[\{name=\"var3\",value=\"3\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var3")
 self.expect("\^done")
 self.runCmd("-var-create var3 * s_MyVar")
@@ -107,7 +107,7 @@
 self.expect("\^done,value=\"2\"")
 self.runCmd("-var-update --all-values var4")
 # 
self.expect("\^done,changelist=\[\{name=\"var4\",value=\"2\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var4")
 self.expect("\^done")
 self.runCmd("-var-create var4 * b")
@@ -148,6 +148,13 @@
 self.expect(
 
"\^done,numchild=\"1\",children=\[child=\{name=\"var6\.\*\$[0-9]+\",exp=\"\*\$[0-9]+\",numchild=\"0\",type=\"const
 char\",thread-id=\"4294967295\",value=\"47 
'/'\",has_more=\"0\"\}\],has_more=\"0\"")
 
+# Print an expression with spaces and optional arguments
+self.runCmd("-data-evaluate-expression \"a + b\"")
+self.expect("\^done,value=\"12\"")
+self.runCmd("-var-create var7 * \"a + b\" --thread 1 --frame 0")
+self.expect(
+
"\^done,name=\"var7\",numchild=\"0\",value=\"12\",type=\"int\",thread-id=\"1\",has_more=\"0\"")
+
 @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races
 @skipIfLinux  # llvm.org/pr22841: lldb-mi tests fail on all Linux buildbots
@@ -315,12 +322,14 @@
 # Test that -var-list-children lists all children with their values
 # (and that from and to are optional)
 self.runCmd("-var-list-children --all-values var_complx")
-
self.expect("\^done,numchild=\"3\",children=\[child=\{name=\"var_complx\.i\",exp=\"i\",numchild=\"0\",type=\"int\",thread-id=\"1\",value=\"3\",has_more=\"0\"\},child=\{name=\"var_complx\.inner\",exp=\"inner\",numchild=\"1\",type=\"complex_type::\(anonymous
 
struct\)\",thread-id=\"1\",value=\"\{\.\.\.\}\",has_more=\"0\"\},child=\{name=\"var_complx\.complex_ptr\",exp=\"complex_ptr\",numchild=\"3\",type=\"complex_type
 \*\",thread-id=\"1\",value=\"0x[0-9a-f]+\",has_more=\"0\"\}\],has_more=\"0\"")
+self.expect(
+
"\^done,numchild=\"3\",children=\[child=\{name=\"var_complx\.i\",exp=\"i\",numchild=\"0\",type=\"int\",thread-id=\"1\",value=\"3\",has_more=\"0\"\},child=\{name=\"var_complx\.inner\",exp=\"inner\",numchild=\"1\",type=\"complex_type::\(anonymous
 
struct\)\",thread-id=\"1\",value=\"\{\.\.\.\}\",has_more=\"0\"\},child=\{name=\"var_complx\.complex_ptr\",exp=\"complex_ptr\",numchild=\"3\",type=\"complex_type
 \*\",thread-id=\"1\",value=\"0x[0-9a-f]+\",has_more=\"0\"\}\],has_more=\"0\"")
 self.runCmd("-var-list-children --simple-values var_complx_array")
 self.expect(
 
"\^done,numchild=\"2\",children=\[child=\{name=\"var_complx_array\.\[0\]\",exp=\"\[0\]\",numchild=\"3\",type=\"complex_type\",thread-id=\"1\",has_more=\"0\"\},child=\{name=\"var_complx_array\.\[1\]\",exp=\"\[1\]\",numchild=\"3\",type=\"complex_type\",thread-id=\"1\",has_more=\"0\"\}\],has_more=\"0\"")
 self.runCmd("-var-list-children 0 var_pcomplx")
-

Re: [Lldb-commits] [PATCH] D24202: Fix parsing expressions to evaluate with spaces and optional args (MI)

[Ilia K via lldb-commits]

ki.stfu marked an inline comment as done.


Comment at: tools/lldb-mi/MICmdArgValOptionLong.cpp:187-188
@@ -186,12 +186,4 @@
 const MIuint nArgIndex) {
-  CMIUtilString::VecString_t vecOptions;
-  MIuint nOptionsPresent = 0;
-  if ((m_eExpectingOptionType != eArgValType_StringQuoted) &&
-  (m_eExpectingOptionType != eArgValType_StringQuotedNumber) &&
-  (m_eExpectingOptionType != eArgValType_StringQuotedNumberPath))
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().Split(" ", vecOptions);
-  else
-nOptionsPresent =
-vrwTxt.GetArgsLeftToParse().SplitConsiderQuotes(" ", vecOptions);
-  if (nOptionsPresent == 0)
+  CMIUtilString::VecString_t vecOptions = vrwTxt.GetArgs();
+  if (vecOptions.size() == 0)
 return MIstatus::failure;

@edmunoz: I did it as you suggested.


https://reviews.llvm.org/D24202



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

Re: [Lldb-commits] [PATCH] D24202: Fix parsing expressions to evaluate with spaces and optional args (MI)

[Ilia K via lldb-commits]

ki.stfu updated this revision to Diff 72136.
ki.stfu added a comment.

Rebase againt ToT; Apply clang-format & autopep; Minor fix in condition


https://reviews.llvm.org/D24202

Files:
  packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
  tools/lldb-mi/MICmdArgValOptionLong.cpp

Index: tools/lldb-mi/MICmdArgValOptionLong.cpp
===
--- tools/lldb-mi/MICmdArgValOptionLong.cpp
+++ tools/lldb-mi/MICmdArgValOptionLong.cpp
@@ -184,16 +184,8 @@
 //--
 bool CMICmdArgValOptionLong::ExtractExpectedOptions(CMICmdArgContext ,
 const MIuint nArgIndex) {
-  CMIUtilString::VecString_t vecOptions;
-  MIuint nOptionsPresent = 0;
-  if ((m_eExpectingOptionType != eArgValType_StringQuoted) &&
-  (m_eExpectingOptionType != eArgValType_StringQuotedNumber) &&
-  (m_eExpectingOptionType != eArgValType_StringQuotedNumberPath))
-nOptionsPresent = vrwTxt.GetArgsLeftToParse().Split(" ", vecOptions);
-  else
-nOptionsPresent =
-vrwTxt.GetArgsLeftToParse().SplitConsiderQuotes(" ", vecOptions);
-  if (nOptionsPresent == 0)
+  CMIUtilString::VecString_t vecOptions = vrwTxt.GetArgs();
+  if (vecOptions.size() == 0)
 return MIstatus::failure;
 
   MIuint nArgIndexCnt = 0;
Index: packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
===
--- packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
+++ packages/Python/lldbsuite/test/tools/lldb-mi/variable/TestMiVar.py
@@ -61,7 +61,7 @@
 self.expect("\^done,value=\"30\"")
 self.runCmd("-var-update --all-values var2")
 # 
self.expect("\^done,changelist=\[\{name=\"var2\",value=\"30\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var2")
 self.expect("\^done")
 self.runCmd("-var-create var2 * g_MyVar")
@@ -84,7 +84,7 @@
 self.expect("\^done,value=\"3\"")
 self.runCmd("-var-update --all-values var3")
 # 
self.expect("\^done,changelist=\[\{name=\"var3\",value=\"3\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var3")
 self.expect("\^done")
 self.runCmd("-var-create var3 * s_MyVar")
@@ -107,7 +107,7 @@
 self.expect("\^done,value=\"2\"")
 self.runCmd("-var-update --all-values var4")
 # 
self.expect("\^done,changelist=\[\{name=\"var4\",value=\"2\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"\}\]")
-# #FIXME -var-update doesn't work
+# FIXME -var-update doesn't work
 self.runCmd("-var-delete var4")
 self.expect("\^done")
 self.runCmd("-var-create var4 * b")
@@ -148,6 +148,13 @@
 self.expect(
 
"\^done,numchild=\"1\",children=\[child=\{name=\"var6\.\*\$[0-9]+\",exp=\"\*\$[0-9]+\",numchild=\"0\",type=\"const
 char\",thread-id=\"4294967295\",value=\"47 
'/'\",has_more=\"0\"\}\],has_more=\"0\"")
 
+# Print an expression with spaces and optional arguments
+self.runCmd("-data-evaluate-expression \"a + b\"")
+self.expect("\^done,value=\"12\"")
+self.runCmd("-var-create var7 * \"a + b\" --thread 1 --frame 0")
+self.expect(
+
"\^done,name=\"var7\",numchild=\"0\",value=\"12\",type=\"int\",thread-id=\"1\",has_more=\"0\"")
+
 @skipIfWindows  # llvm.org/pr24452: Get lldb-mi tests working on Windows
 @skipIfFreeBSD  # llvm.org/pr22411: Failure presumably due to known thread 
races
 @skipIfLinux  # llvm.org/pr22841: lldb-mi tests fail on all Linux buildbots
@@ -315,12 +322,14 @@
 # Test that -var-list-children lists all children with their values
 # (and that from and to are optional)
 self.runCmd("-var-list-children --all-values var_complx")
-
self.expect("\^done,numchild=\"3\",children=\[child=\{name=\"var_complx\.i\",exp=\"i\",numchild=\"0\",type=\"int\",thread-id=\"1\",value=\"3\",has_more=\"0\"\},child=\{name=\"var_complx\.inner\",exp=\"inner\",numchild=\"1\",type=\"complex_type::\(anonymous
 
struct\)\",thread-id=\"1\",value=\"\{\.\.\.\}\",has_more=\"0\"\},child=\{name=\"var_complx\.complex_ptr\",exp=\"complex_ptr\",numchild=\"3\",type=\"complex_type
 \*\",thread-id=\"1\",value=\"0x[0-9a-f]+\",has_more=\"0\"\}\],has_more=\"0\"")
+self.expect(
+
"\^done,numchild=\"3\",children=\[child=\{name=\"var_complx\.i\",exp=\"i\",numchild=\"0\",type=\"int\",thread-id=\"1\",value=\"3\",has_more=\"0\"\},child=\{name=\"var_complx\.inner\",exp=\"inner\",numchild=\"1\",type=\"complex_type::\(anonymous
 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

I imagine Greg also thought the llvm classes were just a wrapper around the 
system reg* functions.  I wouldn't have had a problem with that, though it 
wouldn't have made any noticeable difference to lldb's performance.  But I 
actually had to deal with a bug in the OS X copy of the regex engine that 
crashed gdb back in the day, so I know such things are possible, and are much 
more likely to be found and fixed in the system implementation which is quite 
heavily used than in a private copy.  And I really don't want to be in the 
business of maintaining one of these beasts.  So let's hold off on this till 
llvm gets around to using an implementation that is battle tested, like the 
std::regex one.

Jim


> On Sep 21, 2016, at 6:47 PM, Zachary Turner  wrote:
> 
> Fair enough, greg seemed to be rather disturbed by the copy, so this was 
> really just to appease him, I'm also not too concerned . I do think it's 
> worth sharing code with llvm here, but I'm happy to put it off unless someone 
> insists (I wasn't planning to do it anyway until greg expressed concern)
> On Wed, Sep 21, 2016 at 6:42 PM Jim Ingham  wrote:
> First off, I bet we could decide for giggles to make 5 or 6 random copies of 
> strings before we pass them to regcomp in lldb and none of our users would 
> notice the difference.  It's just not something we do as a hot code path.  So 
> avoiding a copy or two is not worth even one new bug in regex handling IMO.
> 
> Secondly, maybe people who work on compilers find the notion of maintaining a 
> regular expression engine something they can get excited about (though I 
> doubt it) but that is certainly not something I want to take on unless I'm 
> getting a very strong benefit from it.
> 
> Jim
> 
> > On Sep 21, 2016, at 6:36 PM, Zachary Turner  wrote:
> >
> > I suppose, but at the same time if it's good enough for llvm it seems good 
> > enough for us.
> >
> > By using it we contribute to its testing, and if we find something it helps 
> > a much larger group of people than just us.
> >
> > Anyway it's you guys' call, but this string copy isn't going to go away 
> > otherwise and I don't really think reverting this change is in the projects 
> > best interest
> > On Wed, Sep 21, 2016 at 6:31 PM Jim Ingham  wrote:
> > I was being facetious about the enhancements.  I am more serious about 
> > bugs.  I would really rather use a maintained rather than a "I got this 
> > source at some point and use it for some things, but don't rigorously test 
> > it" version of regcomp & friends.  Can we hold off on that change till we 
> > have nothing else better to do?
> >
> > Jim
> > > On Sep 21, 2016, at 6:27 PM, Zachary Turner  wrote:
> > >
> > > I don't believe so. For the same reason we don't want enhanced on Apple 
> > > and extended everywhere else. Better if all platforms do the same thing.
> > >
> > > There may be a case to be made for standardizing on std::regex though, it 
> > > has many different flavors and has been standardized for some time.
> > >
> > > Maybe llvm::Regex could be rewritten in terms of std::regex, that would 
> > > enable all the cool flavors for everyone (but I imagine it would tickle 
> > > some strange compatibility bugs and not be entirely smooth)
> > >
> > >
> > > On Wed, Sep 21, 2016 at 6:20 PM Jim Ingham  wrote:
> > > So does the build machinery use the one that is supported on that 
> > > platform if it is available?  They are going to get much wider testing, 
> > > fixes and even "ENHANCE"ments...
> > >
> > > Jim
> > >
> > > > On Sep 21, 2016, at 6:07 PM, Zachary Turner  wrote:
> > > >
> > > > Windows :)
> > > >
> > > > But that was before std::regex. In theory std::regex would work 
> > > > everywhere (although idk how it performs)
> > > > On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:
> > > > It seems a little odd that llvm has its own forked copy of Henry 
> > > > Spencer's old regular expression engine?  Are there platforms we care 
> > > > about that don't have a maintained version of exactly the same code?
> > > >
> > > > Jim
> > > >
> > > >
> > > > > On Sep 21, 2016, at 5:42 PM, Zachary Turner  
> > > > > wrote:
> > > > >
> > > > > I'll try to address the Regex issue this week (by making everything 
> > > > > use llvm extended mode regexes).  If someone feels up to the 
> > > > > challenge, adding support for \d \s etc etc to llvm's regex 
> > > > > implementation would make a lot of people very happy.
> > > > >
> > > > > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> > > > >
> > > > > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits 
> > > > > >  wrote:
> > > > > >
> > > > > > Yep, and we can't have any regex objects in LLDB using those 
> > > > > > because they will only work on Apple and we don't want code like:
> 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Fair enough, greg seemed to be rather disturbed by the copy, so this was
really just to appease him, I'm also not too concerned . I do think it's
worth sharing code with llvm here, but I'm happy to put it off unless
someone insists (I wasn't planning to do it anyway until greg expressed
concern)
On Wed, Sep 21, 2016 at 6:42 PM Jim Ingham  wrote:

> First off, I bet we could decide for giggles to make 5 or 6 random copies
> of strings before we pass them to regcomp in lldb and none of our users
> would notice the difference.  It's just not something we do as a hot code
> path.  So avoiding a copy or two is not worth even one new bug in regex
> handling IMO.
>
> Secondly, maybe people who work on compilers find the notion of
> maintaining a regular expression engine something they can get excited
> about (though I doubt it) but that is certainly not something I want to
> take on unless I'm getting a very strong benefit from it.
>
> Jim
>
> > On Sep 21, 2016, at 6:36 PM, Zachary Turner  wrote:
> >
> > I suppose, but at the same time if it's good enough for llvm it seems
> good enough for us.
> >
> > By using it we contribute to its testing, and if we find something it
> helps a much larger group of people than just us.
> >
> > Anyway it's you guys' call, but this string copy isn't going to go away
> otherwise and I don't really think reverting this change is in the projects
> best interest
> > On Wed, Sep 21, 2016 at 6:31 PM Jim Ingham  wrote:
> > I was being facetious about the enhancements.  I am more serious about
> bugs.  I would really rather use a maintained rather than a "I got this
> source at some point and use it for some things, but don't rigorously test
> it" version of regcomp & friends.  Can we hold off on that change till we
> have nothing else better to do?
> >
> > Jim
> > > On Sep 21, 2016, at 6:27 PM, Zachary Turner 
> wrote:
> > >
> > > I don't believe so. For the same reason we don't want enhanced on
> Apple and extended everywhere else. Better if all platforms do the same
> thing.
> > >
> > > There may be a case to be made for standardizing on std::regex though,
> it has many different flavors and has been standardized for some time.
> > >
> > > Maybe llvm::Regex could be rewritten in terms of std::regex, that
> would enable all the cool flavors for everyone (but I imagine it would
> tickle some strange compatibility bugs and not be entirely smooth)
> > >
> > >
> > > On Wed, Sep 21, 2016 at 6:20 PM Jim Ingham  wrote:
> > > So does the build machinery use the one that is supported on that
> platform if it is available?  They are going to get much wider testing,
> fixes and even "ENHANCE"ments...
> > >
> > > Jim
> > >
> > > > On Sep 21, 2016, at 6:07 PM, Zachary Turner 
> wrote:
> > > >
> > > > Windows :)
> > > >
> > > > But that was before std::regex. In theory std::regex would work
> everywhere (although idk how it performs)
> > > > On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham 
> wrote:
> > > > It seems a little odd that llvm has its own forked copy of Henry
> Spencer's old regular expression engine?  Are there platforms we care about
> that don't have a maintained version of exactly the same code?
> > > >
> > > > Jim
> > > >
> > > >
> > > > > On Sep 21, 2016, at 5:42 PM, Zachary Turner 
> wrote:
> > > > >
> > > > > I'll try to address the Regex issue this week (by making
> everything use llvm extended mode regexes).  If someone feels up to the
> challenge, adding support for \d \s etc etc to llvm's regex implementation
> would make a lot of people very happy.
> > > > >
> > > > > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham 
> wrote:
> > > > >
> > > > > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > > > > >
> > > > > > Yep, and we can't have any regex objects in LLDB using those
> because they will only work on Apple and we don't want code like:
> > > > > >
> > > > > > #if defined(__APPLE__)
> > > > > >   RegularExpression e("\s+");
> > > > > > #else
> > > > > >   RegularExpression e("[ \t]+");
> > > > > > #endif
> > > > > >
> > > > > > I know "\s" is probably extended, so this was a bad example, but
> you get my drift.
> > > > >
> > > > > Nope, sadly for extended you have to say [[:space:]].  To avoid
> the #define problem, we could require only extended regular expressions in
> lldb code, but let users type the more convenient enhanced one wherever the
> command line uses them.  But beyond these convenient shortcuts there
> probably aren't that many additions that would be useful for the kind of
> regular expressions our users are likely to write.  If we ever provide "-r"
> option to "memory find" the byte literal extension might come in handy.
> But I don't think that justifies making MacOS builds different.
> > > > >
> > > > > If it really bothered us we 

Re: [Lldb-commits] [PATCH] D12363: Read module list from mini dump

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r246302.


https://reviews.llvm.org/D12363



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

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

First off, I bet we could decide for giggles to make 5 or 6 random copies of 
strings before we pass them to regcomp in lldb and none of our users would 
notice the difference.  It's just not something we do as a hot code path.  So 
avoiding a copy or two is not worth even one new bug in regex handling IMO.

Secondly, maybe people who work on compilers find the notion of maintaining a 
regular expression engine something they can get excited about (though I doubt 
it) but that is certainly not something I want to take on unless I'm getting a 
very strong benefit from it.

Jim

> On Sep 21, 2016, at 6:36 PM, Zachary Turner  wrote:
> 
> I suppose, but at the same time if it's good enough for llvm it seems good 
> enough for us. 
> 
> By using it we contribute to its testing, and if we find something it helps a 
> much larger group of people than just us.
> 
> Anyway it's you guys' call, but this string copy isn't going to go away 
> otherwise and I don't really think reverting this change is in the projects 
> best interest 
> On Wed, Sep 21, 2016 at 6:31 PM Jim Ingham  wrote:
> I was being facetious about the enhancements.  I am more serious about bugs.  
> I would really rather use a maintained rather than a "I got this source at 
> some point and use it for some things, but don't rigorously test it" version 
> of regcomp & friends.  Can we hold off on that change till we have nothing 
> else better to do?
> 
> Jim
> > On Sep 21, 2016, at 6:27 PM, Zachary Turner  wrote:
> >
> > I don't believe so. For the same reason we don't want enhanced on Apple and 
> > extended everywhere else. Better if all platforms do the same thing.
> >
> > There may be a case to be made for standardizing on std::regex though, it 
> > has many different flavors and has been standardized for some time.
> >
> > Maybe llvm::Regex could be rewritten in terms of std::regex, that would 
> > enable all the cool flavors for everyone (but I imagine it would tickle 
> > some strange compatibility bugs and not be entirely smooth)
> >
> >
> > On Wed, Sep 21, 2016 at 6:20 PM Jim Ingham  wrote:
> > So does the build machinery use the one that is supported on that platform 
> > if it is available?  They are going to get much wider testing, fixes and 
> > even "ENHANCE"ments...
> >
> > Jim
> >
> > > On Sep 21, 2016, at 6:07 PM, Zachary Turner  wrote:
> > >
> > > Windows :)
> > >
> > > But that was before std::regex. In theory std::regex would work 
> > > everywhere (although idk how it performs)
> > > On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:
> > > It seems a little odd that llvm has its own forked copy of Henry 
> > > Spencer's old regular expression engine?  Are there platforms we care 
> > > about that don't have a maintained version of exactly the same code?
> > >
> > > Jim
> > >
> > >
> > > > On Sep 21, 2016, at 5:42 PM, Zachary Turner  wrote:
> > > >
> > > > I'll try to address the Regex issue this week (by making everything use 
> > > > llvm extended mode regexes).  If someone feels up to the challenge, 
> > > > adding support for \d \s etc etc to llvm's regex implementation would 
> > > > make a lot of people very happy.
> > > >
> > > > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> > > >
> > > > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits 
> > > > >  wrote:
> > > > >
> > > > > Yep, and we can't have any regex objects in LLDB using those because 
> > > > > they will only work on Apple and we don't want code like:
> > > > >
> > > > > #if defined(__APPLE__)
> > > > >   RegularExpression e("\s+");
> > > > > #else
> > > > >   RegularExpression e("[ \t]+");
> > > > > #endif
> > > > >
> > > > > I know "\s" is probably extended, so this was a bad example, but you 
> > > > > get my drift.
> > > >
> > > > Nope, sadly for extended you have to say [[:space:]].  To avoid the 
> > > > #define problem, we could require only extended regular expressions in 
> > > > lldb code, but let users type the more convenient enhanced one wherever 
> > > > the command line uses them.  But beyond these convenient shortcuts 
> > > > there probably aren't that many additions that would be useful for the 
> > > > kind of regular expressions our users are likely to write.  If we ever 
> > > > provide "-r" option to "memory find" the byte literal extension might 
> > > > come in handy.  But I don't think that justifies making MacOS builds 
> > > > different.
> > > >
> > > > If it really bothered us we could go get a more modern regex engine 
> > > > from somewhere (rip it out of Tcl or something like that...)
> > > >
> > > > Jim
> > > >
> > > >
> > > > >
> > > > > Greg
> > > > >
> > > > >> On Sep 21, 2016, at 5:19 PM, Zachary Turner  
> > > > >> wrote:
> > > > >>
> > > > >> That sounds like a plan.  BTW, extended is the one that everyone 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

I suppose, but at the same time if it's good enough for llvm it seems good
enough for us.

By using it we contribute to its testing, and if we find something it helps
a much larger group of people than just us.

Anyway it's you guys' call, but this string copy isn't going to go away
otherwise and I don't really think reverting this change is in the projects
best interest
On Wed, Sep 21, 2016 at 6:31 PM Jim Ingham  wrote:

> I was being facetious about the enhancements.  I am more serious about
> bugs.  I would really rather use a maintained rather than a "I got this
> source at some point and use it for some things, but don't rigorously test
> it" version of regcomp & friends.  Can we hold off on that change till we
> have nothing else better to do?
>
> Jim
> > On Sep 21, 2016, at 6:27 PM, Zachary Turner  wrote:
> >
> > I don't believe so. For the same reason we don't want enhanced on Apple
> and extended everywhere else. Better if all platforms do the same thing.
> >
> > There may be a case to be made for standardizing on std::regex though,
> it has many different flavors and has been standardized for some time.
> >
> > Maybe llvm::Regex could be rewritten in terms of std::regex, that would
> enable all the cool flavors for everyone (but I imagine it would tickle
> some strange compatibility bugs and not be entirely smooth)
> >
> >
> > On Wed, Sep 21, 2016 at 6:20 PM Jim Ingham  wrote:
> > So does the build machinery use the one that is supported on that
> platform if it is available?  They are going to get much wider testing,
> fixes and even "ENHANCE"ments...
> >
> > Jim
> >
> > > On Sep 21, 2016, at 6:07 PM, Zachary Turner 
> wrote:
> > >
> > > Windows :)
> > >
> > > But that was before std::regex. In theory std::regex would work
> everywhere (although idk how it performs)
> > > On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:
> > > It seems a little odd that llvm has its own forked copy of Henry
> Spencer's old regular expression engine?  Are there platforms we care about
> that don't have a maintained version of exactly the same code?
> > >
> > > Jim
> > >
> > >
> > > > On Sep 21, 2016, at 5:42 PM, Zachary Turner 
> wrote:
> > > >
> > > > I'll try to address the Regex issue this week (by making everything
> use llvm extended mode regexes).  If someone feels up to the challenge,
> adding support for \d \s etc etc to llvm's regex implementation would make
> a lot of people very happy.
> > > >
> > > > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham 
> wrote:
> > > >
> > > > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > > > >
> > > > > Yep, and we can't have any regex objects in LLDB using those
> because they will only work on Apple and we don't want code like:
> > > > >
> > > > > #if defined(__APPLE__)
> > > > >   RegularExpression e("\s+");
> > > > > #else
> > > > >   RegularExpression e("[ \t]+");
> > > > > #endif
> > > > >
> > > > > I know "\s" is probably extended, so this was a bad example, but
> you get my drift.
> > > >
> > > > Nope, sadly for extended you have to say [[:space:]].  To avoid the
> #define problem, we could require only extended regular expressions in lldb
> code, but let users type the more convenient enhanced one wherever the
> command line uses them.  But beyond these convenient shortcuts there
> probably aren't that many additions that would be useful for the kind of
> regular expressions our users are likely to write.  If we ever provide "-r"
> option to "memory find" the byte literal extension might come in handy.
> But I don't think that justifies making MacOS builds different.
> > > >
> > > > If it really bothered us we could go get a more modern regex engine
> from somewhere (rip it out of Tcl or something like that...)
> > > >
> > > > Jim
> > > >
> > > >
> > > > >
> > > > > Greg
> > > > >
> > > > >> On Sep 21, 2016, at 5:19 PM, Zachary Turner 
> wrote:
> > > > >>
> > > > >> That sounds like a plan.  BTW, extended is the one that everyone
> supports, enhanced is the one that only apple supports.
> > > > >>
> > > > >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton 
> wrote:
> > > > >> And we should check for any "extended" mode regex stuff and get
> rid of it since as you said they are not portable. They tend to be
> shortcuts for classes of objects and we can just fix the regex to be more
> explicit about it. For now we would keep the
> lldb_private::RegularExpression class, have it have a llvm::Regex member
> and then lldbassert if the regex fails to compile so we can catch any
> extended cruft that we might miss so we can fix it...
> > > > >>
> > > > >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton 
> wrote:
> > > > >>>
> > > > >>> To be clear: if we can make StringRef work efficiently, I am
> fine with that. We can just cut 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

I was being facetious about the enhancements.  I am more serious about bugs.  I 
would really rather use a maintained rather than a "I got this source at some 
point and use it for some things, but don't rigorously test it" version of 
regcomp & friends.  Can we hold off on that change till we have nothing else 
better to do?

Jim
> On Sep 21, 2016, at 6:27 PM, Zachary Turner  wrote:
> 
> I don't believe so. For the same reason we don't want enhanced on Apple and 
> extended everywhere else. Better if all platforms do the same thing.
> 
> There may be a case to be made for standardizing on std::regex though, it has 
> many different flavors and has been standardized for some time.
> 
> Maybe llvm::Regex could be rewritten in terms of std::regex, that would 
> enable all the cool flavors for everyone (but I imagine it would tickle some 
> strange compatibility bugs and not be entirely smooth)
> 
> 
> On Wed, Sep 21, 2016 at 6:20 PM Jim Ingham  wrote:
> So does the build machinery use the one that is supported on that platform if 
> it is available?  They are going to get much wider testing, fixes and even 
> "ENHANCE"ments...
> 
> Jim
> 
> > On Sep 21, 2016, at 6:07 PM, Zachary Turner  wrote:
> >
> > Windows :)
> >
> > But that was before std::regex. In theory std::regex would work everywhere 
> > (although idk how it performs)
> > On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:
> > It seems a little odd that llvm has its own forked copy of Henry Spencer's 
> > old regular expression engine?  Are there platforms we care about that 
> > don't have a maintained version of exactly the same code?
> >
> > Jim
> >
> >
> > > On Sep 21, 2016, at 5:42 PM, Zachary Turner  wrote:
> > >
> > > I'll try to address the Regex issue this week (by making everything use 
> > > llvm extended mode regexes).  If someone feels up to the challenge, 
> > > adding support for \d \s etc etc to llvm's regex implementation would 
> > > make a lot of people very happy.
> > >
> > > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> > >
> > > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits 
> > > >  wrote:
> > > >
> > > > Yep, and we can't have any regex objects in LLDB using those because 
> > > > they will only work on Apple and we don't want code like:
> > > >
> > > > #if defined(__APPLE__)
> > > >   RegularExpression e("\s+");
> > > > #else
> > > >   RegularExpression e("[ \t]+");
> > > > #endif
> > > >
> > > > I know "\s" is probably extended, so this was a bad example, but you 
> > > > get my drift.
> > >
> > > Nope, sadly for extended you have to say [[:space:]].  To avoid the 
> > > #define problem, we could require only extended regular expressions in 
> > > lldb code, but let users type the more convenient enhanced one wherever 
> > > the command line uses them.  But beyond these convenient shortcuts there 
> > > probably aren't that many additions that would be useful for the kind of 
> > > regular expressions our users are likely to write.  If we ever provide 
> > > "-r" option to "memory find" the byte literal extension might come in 
> > > handy.  But I don't think that justifies making MacOS builds different.
> > >
> > > If it really bothered us we could go get a more modern regex engine from 
> > > somewhere (rip it out of Tcl or something like that...)
> > >
> > > Jim
> > >
> > >
> > > >
> > > > Greg
> > > >
> > > >> On Sep 21, 2016, at 5:19 PM, Zachary Turner  wrote:
> > > >>
> > > >> That sounds like a plan.  BTW, extended is the one that everyone 
> > > >> supports, enhanced is the one that only apple supports.
> > > >>
> > > >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton  
> > > >> wrote:
> > > >> And we should check for any "extended" mode regex stuff and get rid of 
> > > >> it since as you said they are not portable. They tend to be shortcuts 
> > > >> for classes of objects and we can just fix the regex to be more 
> > > >> explicit about it. For now we would keep the 
> > > >> lldb_private::RegularExpression class, have it have a llvm::Regex 
> > > >> member and then lldbassert if the regex fails to compile so we can 
> > > >> catch any extended cruft that we might miss so we can fix it...
> > > >>
> > > >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> > > >>>
> > > >>> To be clear: if we can make StringRef work efficiently, I am fine 
> > > >>> with that. We can just cut over to using llvm::Regex where it uses 
> > > >>> the start and end pointer. I would just like to avoid making string 
> > > >>> copies for no reason. I don't have anything against using StringRef 
> > > >>> as long as we do it efficiently.
> > > >>>
> > > >>> Greg
> > > >>>
> > > >>>
> > >  On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits 
> > >   wrote:
> > > 
> > 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

I don't believe so. For the same reason we don't want enhanced on Apple and
extended everywhere else. Better if all platforms do the same thing.

There may be a case to be made for standardizing on std::regex though, it
has many different flavors and has been standardized for some time.

Maybe llvm::Regex could be rewritten in terms of std::regex, that would
enable all the cool flavors for everyone (but I imagine it would tickle
some strange compatibility bugs and not be entirely smooth)


On Wed, Sep 21, 2016 at 6:20 PM Jim Ingham  wrote:

> So does the build machinery use the one that is supported on that platform
> if it is available?  They are going to get much wider testing, fixes and
> even "ENHANCE"ments...
>
> Jim
>
> > On Sep 21, 2016, at 6:07 PM, Zachary Turner  wrote:
> >
> > Windows :)
> >
> > But that was before std::regex. In theory std::regex would work
> everywhere (although idk how it performs)
> > On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:
> > It seems a little odd that llvm has its own forked copy of Henry
> Spencer's old regular expression engine?  Are there platforms we care about
> that don't have a maintained version of exactly the same code?
> >
> > Jim
> >
> >
> > > On Sep 21, 2016, at 5:42 PM, Zachary Turner 
> wrote:
> > >
> > > I'll try to address the Regex issue this week (by making everything
> use llvm extended mode regexes).  If someone feels up to the challenge,
> adding support for \d \s etc etc to llvm's regex implementation would make
> a lot of people very happy.
> > >
> > > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> > >
> > > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > > >
> > > > Yep, and we can't have any regex objects in LLDB using those because
> they will only work on Apple and we don't want code like:
> > > >
> > > > #if defined(__APPLE__)
> > > >   RegularExpression e("\s+");
> > > > #else
> > > >   RegularExpression e("[ \t]+");
> > > > #endif
> > > >
> > > > I know "\s" is probably extended, so this was a bad example, but you
> get my drift.
> > >
> > > Nope, sadly for extended you have to say [[:space:]].  To avoid the
> #define problem, we could require only extended regular expressions in lldb
> code, but let users type the more convenient enhanced one wherever the
> command line uses them.  But beyond these convenient shortcuts there
> probably aren't that many additions that would be useful for the kind of
> regular expressions our users are likely to write.  If we ever provide "-r"
> option to "memory find" the byte literal extension might come in handy.
> But I don't think that justifies making MacOS builds different.
> > >
> > > If it really bothered us we could go get a more modern regex engine
> from somewhere (rip it out of Tcl or something like that...)
> > >
> > > Jim
> > >
> > >
> > > >
> > > > Greg
> > > >
> > > >> On Sep 21, 2016, at 5:19 PM, Zachary Turner 
> wrote:
> > > >>
> > > >> That sounds like a plan.  BTW, extended is the one that everyone
> supports, enhanced is the one that only apple supports.
> > > >>
> > > >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton 
> wrote:
> > > >> And we should check for any "extended" mode regex stuff and get rid
> of it since as you said they are not portable. They tend to be shortcuts
> for classes of objects and we can just fix the regex to be more explicit
> about it. For now we would keep the lldb_private::RegularExpression class,
> have it have a llvm::Regex member and then lldbassert if the regex fails to
> compile so we can catch any extended cruft that we might miss so we can fix
> it...
> > > >>
> > > >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton 
> wrote:
> > > >>>
> > > >>> To be clear: if we can make StringRef work efficiently, I am fine
> with that. We can just cut over to using llvm::Regex where it uses the
> start and end pointer. I would just like to avoid making string copies for
> no reason. I don't have anything against using StringRef as long as we do
> it efficiently.
> > > >>>
> > > >>> Greg
> > > >>>
> > > >>>
> > >  On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > > 
> > > >
> > > > On Sep 21, 2016, at 4:43 PM, Zachary Turner 
> wrote:
> > > >
> > > > You need to duplicate something on the heap once when you
> execute the regex.  And in turn you save tens or hundreds or copies on the
> way there because of inefficient string usage.
> > > 
> > >  Where? please show this.
> > > 
> > >  I see the following callers:
> > > 
> > > const char *class_name =
> > > 
> iterator->second->GetClassName().AsCString("");
> > > if (regex_up && class_name &&
> > > 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

So does the build machinery use the one that is supported on that platform if 
it is available?  They are going to get much wider testing, fixes and even 
"ENHANCE"ments...

Jim

> On Sep 21, 2016, at 6:07 PM, Zachary Turner  wrote:
> 
> Windows :)
> 
> But that was before std::regex. In theory std::regex would work everywhere 
> (although idk how it performs)
> On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:
> It seems a little odd that llvm has its own forked copy of Henry Spencer's 
> old regular expression engine?  Are there platforms we care about that don't 
> have a maintained version of exactly the same code?
> 
> Jim
> 
> 
> > On Sep 21, 2016, at 5:42 PM, Zachary Turner  wrote:
> >
> > I'll try to address the Regex issue this week (by making everything use 
> > llvm extended mode regexes).  If someone feels up to the challenge, adding 
> > support for \d \s etc etc to llvm's regex implementation would make a lot 
> > of people very happy.
> >
> > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> >
> > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits 
> > >  wrote:
> > >
> > > Yep, and we can't have any regex objects in LLDB using those because they 
> > > will only work on Apple and we don't want code like:
> > >
> > > #if defined(__APPLE__)
> > >   RegularExpression e("\s+");
> > > #else
> > >   RegularExpression e("[ \t]+");
> > > #endif
> > >
> > > I know "\s" is probably extended, so this was a bad example, but you get 
> > > my drift.
> >
> > Nope, sadly for extended you have to say [[:space:]].  To avoid the #define 
> > problem, we could require only extended regular expressions in lldb code, 
> > but let users type the more convenient enhanced one wherever the command 
> > line uses them.  But beyond these convenient shortcuts there probably 
> > aren't that many additions that would be useful for the kind of regular 
> > expressions our users are likely to write.  If we ever provide "-r" option 
> > to "memory find" the byte literal extension might come in handy.  But I 
> > don't think that justifies making MacOS builds different.
> >
> > If it really bothered us we could go get a more modern regex engine from 
> > somewhere (rip it out of Tcl or something like that...)
> >
> > Jim
> >
> >
> > >
> > > Greg
> > >
> > >> On Sep 21, 2016, at 5:19 PM, Zachary Turner  wrote:
> > >>
> > >> That sounds like a plan.  BTW, extended is the one that everyone 
> > >> supports, enhanced is the one that only apple supports.
> > >>
> > >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton  wrote:
> > >> And we should check for any "extended" mode regex stuff and get rid of 
> > >> it since as you said they are not portable. They tend to be shortcuts 
> > >> for classes of objects and we can just fix the regex to be more explicit 
> > >> about it. For now we would keep the lldb_private::RegularExpression 
> > >> class, have it have a llvm::Regex member and then lldbassert if the 
> > >> regex fails to compile so we can catch any extended cruft that we might 
> > >> miss so we can fix it...
> > >>
> > >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> > >>>
> > >>> To be clear: if we can make StringRef work efficiently, I am fine with 
> > >>> that. We can just cut over to using llvm::Regex where it uses the start 
> > >>> and end pointer. I would just like to avoid making string copies for no 
> > >>> reason. I don't have anything against using StringRef as long as we do 
> > >>> it efficiently.
> > >>>
> > >>> Greg
> > >>>
> > >>>
> >  On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits 
> >   wrote:
> > 
> > >
> > > On Sep 21, 2016, at 4:43 PM, Zachary Turner  
> > > wrote:
> > >
> > > You need to duplicate something on the heap once when you execute the 
> > > regex.  And in turn you save tens or hundreds or copies on the way 
> > > there because of inefficient string usage.
> > 
> >  Where? please show this.
> > 
> >  I see the following callers:
> > 
> > const char *class_name =
> > iterator->second->GetClassName().AsCString("");
> > if (regex_up && class_name &&
> > !regex_up->Execute(llvm::StringRef(class_name)))
> > 
> >  You are adding a strlen() call here to construct the StringRef, not 
> >  saving anything.
> > 
> > 
> >  bool CommandObjectRegexCommand::DoExecute(const char *command,
> > CommandReturnObject ) {
> >  if (command) {
> >   EntryCollection::const_iterator pos, end = m_entries.end();
> >   for (pos = m_entries.begin(); pos != end; ++pos) {
> > RegularExpression::Match regex_match(m_max_matches);
> > 
> > if 

Re: [Lldb-commits] [PATCH] D6042: Fix junk content handling within GDBRemoteCommunication::CheckForPacket.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r220983.


https://reviews.llvm.org/D6042



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

Re: [Lldb-commits] [PATCH] D6105: Redirect stdin, stdout and stderr to /dev/null when launching LLGS process.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r221324.


https://reviews.llvm.org/D6105



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

Re: [Lldb-commits] [PATCH] D6158: Fix error handling in NativeProcessLinux::AttachToInferior

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r221647.


https://reviews.llvm.org/D6158



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

Re: [Lldb-commits] [PATCH] D6204: Apply SOCK_CLOEXEC flag to socket API functions in order to avoid handle leakage to a forked child process.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r222004.


https://reviews.llvm.org/D6204



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

Re: [Lldb-commits] [PATCH] D6490: Use timeout when reading debugserver's port from a named pipe.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r223251.


https://reviews.llvm.org/D6490



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

Re: [Lldb-commits] [PATCH] D6740: Make DynamicLoaderPOSIXDYLD::DidAttach to deduce a target executable by pid if no executable hasn't been assigned to a target so far.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r225332.


https://reviews.llvm.org/D6740



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

[Lldb-commits] [lldb] r282128 - fix Args function broken in r281942

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Wed Sep 21 19:59:23 2016
New Revision: 282128

URL: http://llvm.org/viewvc/llvm-project?rev=282128=rev
Log:
fix Args function broken in r281942

The method was hard-coded to check only the 0th element of the array.
This manifested as NSLog messages behaving incorrectly on macOS.
(This is independent of the broken DarwinLog feature).

Modified:
lldb/trunk/source/Interpreter/Args.cpp

Modified: lldb/trunk/source/Interpreter/Args.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=282128=282127=282128=diff
==
--- lldb/trunk/source/Interpreter/Args.cpp (original)
+++ lldb/trunk/source/Interpreter/Args.cpp Wed Sep 21 19:59:23 2016
@@ -1002,7 +1002,7 @@ bool Args::ContainsEnvironmentVariable(l
 
   // Check each arg to see if it matches the env var name.
   for (size_t i = 0; i < GetArgumentCount(); ++i) {
-auto arg_value = llvm::StringRef::withNullAsEmpty(GetArgumentAtIndex(0));
+auto arg_value = llvm::StringRef::withNullAsEmpty(GetArgumentAtIndex(i));
 
 llvm::StringRef name, value;
 std::tie(name, value) = arg_value.split('=');


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

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Windows :)

But that was before std::regex. In theory std::regex would work everywhere
(although idk how it performs)
On Wed, Sep 21, 2016 at 6:04 PM Jim Ingham  wrote:

> It seems a little odd that llvm has its own forked copy of Henry Spencer's
> old regular expression engine?  Are there platforms we care about that
> don't have a maintained version of exactly the same code?
>
> Jim
>
>
> > On Sep 21, 2016, at 5:42 PM, Zachary Turner  wrote:
> >
> > I'll try to address the Regex issue this week (by making everything use
> llvm extended mode regexes).  If someone feels up to the challenge, adding
> support for \d \s etc etc to llvm's regex implementation would make a lot
> of people very happy.
> >
> > On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> >
> > > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > >
> > > Yep, and we can't have any regex objects in LLDB using those because
> they will only work on Apple and we don't want code like:
> > >
> > > #if defined(__APPLE__)
> > >   RegularExpression e("\s+");
> > > #else
> > >   RegularExpression e("[ \t]+");
> > > #endif
> > >
> > > I know "\s" is probably extended, so this was a bad example, but you
> get my drift.
> >
> > Nope, sadly for extended you have to say [[:space:]].  To avoid the
> #define problem, we could require only extended regular expressions in lldb
> code, but let users type the more convenient enhanced one wherever the
> command line uses them.  But beyond these convenient shortcuts there
> probably aren't that many additions that would be useful for the kind of
> regular expressions our users are likely to write.  If we ever provide "-r"
> option to "memory find" the byte literal extension might come in handy.
> But I don't think that justifies making MacOS builds different.
> >
> > If it really bothered us we could go get a more modern regex engine from
> somewhere (rip it out of Tcl or something like that...)
> >
> > Jim
> >
> >
> > >
> > > Greg
> > >
> > >> On Sep 21, 2016, at 5:19 PM, Zachary Turner 
> wrote:
> > >>
> > >> That sounds like a plan.  BTW, extended is the one that everyone
> supports, enhanced is the one that only apple supports.
> > >>
> > >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton 
> wrote:
> > >> And we should check for any "extended" mode regex stuff and get rid
> of it since as you said they are not portable. They tend to be shortcuts
> for classes of objects and we can just fix the regex to be more explicit
> about it. For now we would keep the lldb_private::RegularExpression class,
> have it have a llvm::Regex member and then lldbassert if the regex fails to
> compile so we can catch any extended cruft that we might miss so we can fix
> it...
> > >>
> > >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton 
> wrote:
> > >>>
> > >>> To be clear: if we can make StringRef work efficiently, I am fine
> with that. We can just cut over to using llvm::Regex where it uses the
> start and end pointer. I would just like to avoid making string copies for
> no reason. I don't have anything against using StringRef as long as we do
> it efficiently.
> > >>>
> > >>> Greg
> > >>>
> > >>>
> >  On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > 
> > >
> > > On Sep 21, 2016, at 4:43 PM, Zachary Turner 
> wrote:
> > >
> > > You need to duplicate something on the heap once when you execute
> the regex.  And in turn you save tens or hundreds or copies on the way
> there because of inefficient string usage.
> > 
> >  Where? please show this.
> > 
> >  I see the following callers:
> > 
> > const char *class_name =
> > iterator->second->GetClassName().AsCString("");
> > if (regex_up && class_name &&
> > !regex_up->Execute(llvm::StringRef(class_name)))
> > 
> >  You are adding a strlen() call here to construct the StringRef, not
> saving anything.
> > 
> > 
> >  bool CommandObjectRegexCommand::DoExecute(const char *command,
> > CommandReturnObject )
> {
> >  if (command) {
> >   EntryCollection::const_iterator pos, end = m_entries.end();
> >   for (pos = m_entries.begin(); pos != end; ++pos) {
> > RegularExpression::Match regex_match(m_max_matches);
> > 
> > if (pos->regex.Execute(command, _match)) {
> >   std::string new_command(pos->command);
> >   std::string match_str;
> > 
> >  No copy saved. Just wasted time with strlen in StringRef
> constructor.
> > 
> > 
> >   DataVisualization::Categories::ForEach(
> >   [, ](const lldb::TypeCategoryImplSP _sp)
> -> bool {
> > if (regex) {
> >   bool escape = true;
> >   if 

Re: [Lldb-commits] [PATCH] D6743: No need to call SetErrorToErrno when pipe2 succeeds.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r224652.


https://reviews.llvm.org/D6743



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

Re: [Lldb-commits] [PATCH] D6941: Fix XCode build on OSX - add OptionValueChar.cpp

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r225733.


https://reviews.llvm.org/D6941



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

Re: [Lldb-commits] [PATCH] D6954: Extend PipePosix with support for named pipes/timeout-based IO and integrate it with GDBRemoteCommunication / lldb-gdbserver.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r225849.


https://reviews.llvm.org/D6954



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

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

It seems a little odd that llvm has its own forked copy of Henry Spencer's old 
regular expression engine?  Are there platforms we care about that don't have a 
maintained version of exactly the same code?  

Jim


> On Sep 21, 2016, at 5:42 PM, Zachary Turner  wrote:
> 
> I'll try to address the Regex issue this week (by making everything use llvm 
> extended mode regexes).  If someone feels up to the challenge, adding support 
> for \d \s etc etc to llvm's regex implementation would make a lot of people 
> very happy.
> 
> On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:
> 
> > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits 
> >  wrote:
> >
> > Yep, and we can't have any regex objects in LLDB using those because they 
> > will only work on Apple and we don't want code like:
> >
> > #if defined(__APPLE__)
> >   RegularExpression e("\s+");
> > #else
> >   RegularExpression e("[ \t]+");
> > #endif
> >
> > I know "\s" is probably extended, so this was a bad example, but you get my 
> > drift.
> 
> Nope, sadly for extended you have to say [[:space:]].  To avoid the #define 
> problem, we could require only extended regular expressions in lldb code, but 
> let users type the more convenient enhanced one wherever the command line 
> uses them.  But beyond these convenient shortcuts there probably aren't that 
> many additions that would be useful for the kind of regular expressions our 
> users are likely to write.  If we ever provide "-r" option to "memory find" 
> the byte literal extension might come in handy.  But I don't think that 
> justifies making MacOS builds different.
> 
> If it really bothered us we could go get a more modern regex engine from 
> somewhere (rip it out of Tcl or something like that...)
> 
> Jim
> 
> 
> >
> > Greg
> >
> >> On Sep 21, 2016, at 5:19 PM, Zachary Turner  wrote:
> >>
> >> That sounds like a plan.  BTW, extended is the one that everyone supports, 
> >> enhanced is the one that only apple supports.
> >>
> >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton  wrote:
> >> And we should check for any "extended" mode regex stuff and get rid of it 
> >> since as you said they are not portable. They tend to be shortcuts for 
> >> classes of objects and we can just fix the regex to be more explicit about 
> >> it. For now we would keep the lldb_private::RegularExpression class, have 
> >> it have a llvm::Regex member and then lldbassert if the regex fails to 
> >> compile so we can catch any extended cruft that we might miss so we can 
> >> fix it...
> >>
> >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> >>>
> >>> To be clear: if we can make StringRef work efficiently, I am fine with 
> >>> that. We can just cut over to using llvm::Regex where it uses the start 
> >>> and end pointer. I would just like to avoid making string copies for no 
> >>> reason. I don't have anything against using StringRef as long as we do it 
> >>> efficiently.
> >>>
> >>> Greg
> >>>
> >>>
>  On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits 
>   wrote:
> 
> >
> > On Sep 21, 2016, at 4:43 PM, Zachary Turner  wrote:
> >
> > You need to duplicate something on the heap once when you execute the 
> > regex.  And in turn you save tens or hundreds or copies on the way 
> > there because of inefficient string usage.
> 
>  Where? please show this.
> 
>  I see the following callers:
> 
> const char *class_name =
> iterator->second->GetClassName().AsCString("");
> if (regex_up && class_name &&
> !regex_up->Execute(llvm::StringRef(class_name)))
> 
>  You are adding a strlen() call here to construct the StringRef, not 
>  saving anything.
> 
> 
>  bool CommandObjectRegexCommand::DoExecute(const char *command,
> CommandReturnObject ) {
>  if (command) {
>   EntryCollection::const_iterator pos, end = m_entries.end();
>   for (pos = m_entries.begin(); pos != end; ++pos) {
> RegularExpression::Match regex_match(m_max_matches);
> 
> if (pos->regex.Execute(command, _match)) {
>   std::string new_command(pos->command);
>   std::string match_str;
> 
>  No copy saved. Just wasted time with strlen in StringRef constructor.
> 
> 
>   DataVisualization::Categories::ForEach(
>   [, ](const lldb::TypeCategoryImplSP _sp) -> 
>  bool {
> if (regex) {
>   bool escape = true;
>   if (regex->GetText() == category_sp->GetName()) {
> escape = false;
>   } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
>  category_sp->GetName( {
> escape = false;
>   

Re: [Lldb-commits] [PATCH] D7115: Make OSX test run firewall friendly.

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r226856.


https://reviews.llvm.org/D7115



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

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

I'll try to address the Regex issue this week (by making everything use
llvm extended mode regexes).  If someone feels up to the challenge, adding
support for \d \s etc etc to llvm's regex implementation would make a lot
of people very happy.

On Wed, Sep 21, 2016 at 5:38 PM Jim Ingham  wrote:

>
> > On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Yep, and we can't have any regex objects in LLDB using those because
> they will only work on Apple and we don't want code like:
> >
> > #if defined(__APPLE__)
> >   RegularExpression e("\s+");
> > #else
> >   RegularExpression e("[ \t]+");
> > #endif
> >
> > I know "\s" is probably extended, so this was a bad example, but you get
> my drift.
>
> Nope, sadly for extended you have to say [[:space:]].  To avoid the
> #define problem, we could require only extended regular expressions in lldb
> code, but let users type the more convenient enhanced one wherever the
> command line uses them.  But beyond these convenient shortcuts there
> probably aren't that many additions that would be useful for the kind of
> regular expressions our users are likely to write.  If we ever provide "-r"
> option to "memory find" the byte literal extension might come in handy.
> But I don't think that justifies making MacOS builds different.
>
> If it really bothered us we could go get a more modern regex engine from
> somewhere (rip it out of Tcl or something like that...)
>
> Jim
>
>
> >
> > Greg
> >
> >> On Sep 21, 2016, at 5:19 PM, Zachary Turner  wrote:
> >>
> >> That sounds like a plan.  BTW, extended is the one that everyone
> supports, enhanced is the one that only apple supports.
> >>
> >> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton 
> wrote:
> >> And we should check for any "extended" mode regex stuff and get rid of
> it since as you said they are not portable. They tend to be shortcuts for
> classes of objects and we can just fix the regex to be more explicit about
> it. For now we would keep the lldb_private::RegularExpression class, have
> it have a llvm::Regex member and then lldbassert if the regex fails to
> compile so we can catch any extended cruft that we might miss so we can fix
> it...
> >>
> >>> On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> >>>
> >>> To be clear: if we can make StringRef work efficiently, I am fine with
> that. We can just cut over to using llvm::Regex where it uses the start and
> end pointer. I would just like to avoid making string copies for no reason.
> I don't have anything against using StringRef as long as we do it
> efficiently.
> >>>
> >>> Greg
> >>>
> >>>
>  On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> 
> >
> > On Sep 21, 2016, at 4:43 PM, Zachary Turner 
> wrote:
> >
> > You need to duplicate something on the heap once when you execute
> the regex.  And in turn you save tens or hundreds or copies on the way
> there because of inefficient string usage.
> 
>  Where? please show this.
> 
>  I see the following callers:
> 
> const char *class_name =
> iterator->second->GetClassName().AsCString("");
> if (regex_up && class_name &&
> !regex_up->Execute(llvm::StringRef(class_name)))
> 
>  You are adding a strlen() call here to construct the StringRef, not
> saving anything.
> 
> 
>  bool CommandObjectRegexCommand::DoExecute(const char *command,
> CommandReturnObject ) {
>  if (command) {
>   EntryCollection::const_iterator pos, end = m_entries.end();
>   for (pos = m_entries.begin(); pos != end; ++pos) {
> RegularExpression::Match regex_match(m_max_matches);
> 
> if (pos->regex.Execute(command, _match)) {
>   std::string new_command(pos->command);
>   std::string match_str;
> 
>  No copy saved. Just wasted time with strlen in StringRef constructor.
> 
> 
>   DataVisualization::Categories::ForEach(
>   [, ](const lldb::TypeCategoryImplSP _sp)
> -> bool {
> if (regex) {
>   bool escape = true;
>   if (regex->GetText() == category_sp->GetName()) {
> escape = false;
>   } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
>  category_sp->GetName( {
> escape = false;
>   }
> 
>   if (escape)
> return true;
> }
> 
>  No copy saved. Just wasted time with strlen in StringRef constructor.
> 
> 
> TypeCategoryImpl::ForEachCallbacks foreach;
> foreach
>   .SetExact([, _regex, _printed](
> ConstString name,
> const FormatterSharedPointer _sp) 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

> On Sep 21, 2016, at 5:25 PM, Greg Clayton via lldb-commits 
>  wrote:
> 
> Yep, and we can't have any regex objects in LLDB using those because they 
> will only work on Apple and we don't want code like:
> 
> #if defined(__APPLE__)
>   RegularExpression e("\s+");
> #else
>   RegularExpression e("[ \t]+");
> #endif
> 
> I know "\s" is probably extended, so this was a bad example, but you get my 
> drift.

Nope, sadly for extended you have to say [[:space:]].  To avoid the #define 
problem, we could require only extended regular expressions in lldb code, but 
let users type the more convenient enhanced one wherever the command line uses 
them.  But beyond these convenient shortcuts there probably aren't that many 
additions that would be useful for the kind of regular expressions our users 
are likely to write.  If we ever provide "-r" option to "memory find" the byte 
literal extension might come in handy.  But I don't think that justifies making 
MacOS builds different.

If it really bothered us we could go get a more modern regex engine from 
somewhere (rip it out of Tcl or something like that...)

Jim


> 
> Greg
> 
>> On Sep 21, 2016, at 5:19 PM, Zachary Turner  wrote:
>> 
>> That sounds like a plan.  BTW, extended is the one that everyone supports, 
>> enhanced is the one that only apple supports.  
>> 
>> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton  wrote:
>> And we should check for any "extended" mode regex stuff and get rid of it 
>> since as you said they are not portable. They tend to be shortcuts for 
>> classes of objects and we can just fix the regex to be more explicit about 
>> it. For now we would keep the lldb_private::RegularExpression class, have it 
>> have a llvm::Regex member and then lldbassert if the regex fails to compile 
>> so we can catch any extended cruft that we might miss so we can fix it...
>> 
>>> On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
>>> 
>>> To be clear: if we can make StringRef work efficiently, I am fine with 
>>> that. We can just cut over to using llvm::Regex where it uses the start and 
>>> end pointer. I would just like to avoid making string copies for no reason. 
>>> I don't have anything against using StringRef as long as we do it 
>>> efficiently.
>>> 
>>> Greg
>>> 
>>> 
 On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits 
  wrote:
 
> 
> On Sep 21, 2016, at 4:43 PM, Zachary Turner  wrote:
> 
> You need to duplicate something on the heap once when you execute the 
> regex.  And in turn you save tens or hundreds or copies on the way there 
> because of inefficient string usage.
 
 Where? please show this.
 
 I see the following callers:
 
const char *class_name =
iterator->second->GetClassName().AsCString("");
if (regex_up && class_name &&
!regex_up->Execute(llvm::StringRef(class_name)))
 
 You are adding a strlen() call here to construct the StringRef, not saving 
 anything.
 
 
 bool CommandObjectRegexCommand::DoExecute(const char *command,
CommandReturnObject ) {
 if (command) {
  EntryCollection::const_iterator pos, end = m_entries.end();
  for (pos = m_entries.begin(); pos != end; ++pos) {
RegularExpression::Match regex_match(m_max_matches);
 
if (pos->regex.Execute(command, _match)) {
  std::string new_command(pos->command);
  std::string match_str;
 
 No copy saved. Just wasted time with strlen in StringRef constructor.
 
 
  DataVisualization::Categories::ForEach(
  [, ](const lldb::TypeCategoryImplSP _sp) -> 
 bool {
if (regex) {
  bool escape = true;
  if (regex->GetText() == category_sp->GetName()) {
escape = false;
  } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
 category_sp->GetName( {
escape = false;
  }
 
  if (escape)
return true;
}
 
 No copy saved. Just wasted time with strlen in StringRef constructor.
 
 
TypeCategoryImpl::ForEachCallbacks foreach;
foreach
  .SetExact([, _regex, _printed](
ConstString name,
const FormatterSharedPointer _sp) -> bool {
if (formatter_regex) {
  bool escape = true;
  if (name.GetStringRef() == formatter_regex->GetText()) {
escape = false;
  } else if (formatter_regex->Execute(name.GetStringRef())) {
escape = false;
  }
 
 No copy saved. Just wasted time with strlen in StringRef constructor.
 
  bool ParseCoordinate(const char 

Re: [Lldb-commits] [lldb] r282079 - Make lldb::Regex use StringRef.

[Zachary Turner via lldb-commits]

But the thing is, there were plenty of copies being made before.  People
just didn't see them.  StringRef provides functions like find_first,
substr, and all kinds of stuff.  So before, any time someone wanted to use
one of those functions, what did they do?  Put it in a std::string!
There's your copy.  Even worse, you might think that once you've got a
std::string, at least you don't have to make any more copies, but even
*that* isn't true.  If you want to call std::string::substr(), that
mandates a copy.  Even if you don't put it in a std::string, if you use
strchr or whatever to find the index and calculate your substring c-style,
and you want to pass that to a function taking a const char*, now you have
to put it in a std::string so you can null terminate it, so there's another
copy.

So yes, there's a few more temporary ones while the conversion is in
progress, but there's also some that are removed such as in the above
examples.  I'm not worried too much about the temporary ones because in the
long run the net effect will be a huge reduction in number of copies and
strlens.

On Wed, Sep 21, 2016 at 5:23 PM Greg Clayton  wrote:

>
> > On Sep 21, 2016, at 5:14 PM, Zachary Turner  wrote:
> >
> >
> >
> > On Wed, Sep 21, 2016 at 5:00 PM Greg Clayton  wrote:
> > Please submit a change requests when doing these kinds of things and let
> people comment on the changes before committing such things.
> >
> > We deleted functions that were correctly using "const char *" like:
> >
> >   bool Execute(llvm::StringRef string, Match *match = nullptr) const;
> >   bool Execute(const char *, Match * = nullptr) = delete;
> >
> > Yet inside these functions you do "string.str().c_str()" because these
> need to be null terminated? this indicates that StringRef is NOT the
> correct choice here. You will make heap copy of all strings (if they are
> long enough and regex matching string can be very long) that you compile or
> execute. We already make a copy of the string when we compile, so StringRef
> didn't really save us anything on compile and execute is always making a
> copy just to run the expression on. I am fine with both being there in case
> one might be more efficient, but taking them out just to use a less
> efficient version that uses llvm::StringRef is not the kind of changes we
> should be doing all over.
> >
> > We will make copies of all strings in all of the following changes:
> >
> > - Unneeded copy:
> >
> > -new TypeNameSpecifierImpl(regex->GetText(), true));
> > +new TypeNameSpecifierImpl(regex->GetText().str().c_str(),
> true));
> > All of these copies are only temporary.  As I've said over and over,
> once everything is StringRef all the way down, the copies all disappear.
> It's only a copy because TypeNameSpecifierImpl doesn't take a STringRef,
> and I didn't want to change the whole codebase all at once, but rather
> incrementally.
>
> As part of these changes we absolutely should add StringRef to these
> classes and we should just make it part of the changes. It know these
> strings are temporary, I am not worried about memory, I am worried about
> efficiency of the heap copy of the string that temporarily is created
> during this call. If things were taking "const char *" before, copies were
> not being made. If they were being made, then they were probably being made
> for good reason. I would love to see the places where all of these copies
> were removed, but if they were "const char *" there were no copies going
> in, unless they needed to. If they didn't need to make the copies, then we
> can fix that function.
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

On Wed, Sep 21, 2016 at 5:13 PM Greg Clayton  wrote:

>
> > On Sep 21, 2016, at 4:43 PM, Zachary Turner  wrote:
> >
> > You need to duplicate something on the heap once when you execute the
> regex.  And in turn you save tens or hundreds or copies on the way there
> because of inefficient string usage.
>
> Where? please show this.
>
> I see the following callers:
>
>   const char *class_name =
>   iterator->second->GetClassName().AsCString("");
>   if (regex_up && class_name &&
>   !regex_up->Execute(llvm::StringRef(class_name)))
>
> You are adding a strlen() call here to construct the StringRef, not saving
> anything.
>
Right, this is only this way because i deleted the const char * version of
the function.  Which as I explained in a previous email, I did because it
forces the compiler to tell you when you're using a raw string literal, so
you can manually examine it and make sure it's not null, since that would
cause it to crash.  So you're right, here it doesn't save you anything, but
it doesn't hurt anything either, and the explicit conversion helps catch
bugs in OTHER  places.  Which again, is only temporary until all the
conversions are done, and we can remove the =delete overloads.


>
>
> bool CommandObjectRegexCommand::DoExecute(const char *command,
>   CommandReturnObject ) {
>   if (command) {
> EntryCollection::const_iterator pos, end = m_entries.end();
> for (pos = m_entries.begin(); pos != end; ++pos) {
>   RegularExpression::Match regex_match(m_max_matches);
>
>   if (pos->regex.Execute(command, _match)) {
> std::string new_command(pos->command);
> std::string match_str;
>
> No copy saved. Just wasted time with strlen in StringRef constructor.
>
Right.  But that's because DoExecute takes a const char*, not a StringRef.
And again, doing the entire codebase all at once is infeasible.  Think
about how many places along the pipeline are using this command.  This
isn't the only place where we use it.  We use it later in the function, we
use it in the function above us on the callchain.  We use it lower down on
the callchain.  And every time we do anything with it, we calculate the
length.  So yes, there's an extra one here.  But again, it's only temporary
until in a subsequent patch I change DoExecute() to take a StringRef.
Can't do it all at once.


>
>
> DataVisualization::Categories::ForEach(
> [, ](const lldb::TypeCategoryImplSP _sp) ->
> bool {
>   if (regex) {
> bool escape = true;
> if (regex->GetText() == category_sp->GetName()) {
>   escape = false;
> } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
>category_sp->GetName( {
>   escape = false;
> }
>
> if (escape)
>   return true;
>   }
>
> No copy saved. Just wasted time with strlen in StringRef constructor.
>
>
>   TypeCategoryImpl::ForEachCallbacks foreach;
>   foreach
> .SetExact([, _regex, _printed](
>   ConstString name,
>   const FormatterSharedPointer _sp) -> bool {
>   if (formatter_regex) {
> bool escape = true;
> if (name.GetStringRef() == formatter_regex->GetText()) {
>   escape = false;
> } else if (formatter_regex->Execute(name.GetStringRef())) {
>   escape = false;
> }
>
> No copy saved. Just wasted time with strlen in StringRef constructor.
>
> bool ParseCoordinate(const char *id_cstr) {
>   RegularExpression regex;
>   RegularExpression::Match regex_match(3);
>
>   llvm::StringRef id_ref = llvm::StringRef::withNullAsEmpty(id_cstr);
>   bool matched = false;
>   if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
>   regex.Execute(id_ref, _match))
> matched = true;
>   else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
>regex.Execute(id_ref, _match))
> matched = true;
>   else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
>regex.Execute(id_ref, _match))
>
> No copy saved. Just wasted time with strlen in StringRef constructor.
>
Again, the solution is to have ParseCoordinate take a StringRef.  Problem
solved.


>
> void DWARFCompileUnit::ParseProducerInfo() {
>   m_producer_version_major = UINT32_MAX;
>   m_producer_version_minor = UINT32_MAX;
>   m_producer_version_update = UINT32_MAX;
>
>   const DWARFDebugInfoEntry *die = GetCompileUnitDIEPtrOnly();
>   if (die) {
>
> const char *producer_cstr = die->GetAttributeValueAsString(
> m_dwarf2Data, this, DW_AT_producer, NULL);
> if (producer_cstr) {
>   RegularExpression llvm_gcc_regex(
>   llvm::StringRef("^4\\.[012]\\.[01] \\(Based on Apple "
>   

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Greg Clayton via lldb-commits]

Yep, and we can't have any regex objects in LLDB using those because they will 
only work on Apple and we don't want code like:

#if defined(__APPLE__)
   RegularExpression e("\s+");
#else
   RegularExpression e("[ \t]+");
#endif

I know "\s" is probably extended, so this was a bad example, but you get my 
drift.

Greg

> On Sep 21, 2016, at 5:19 PM, Zachary Turner  wrote:
> 
> That sounds like a plan.  BTW, extended is the one that everyone supports, 
> enhanced is the one that only apple supports.  
> 
> On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton  wrote:
> And we should check for any "extended" mode regex stuff and get rid of it 
> since as you said they are not portable. They tend to be shortcuts for 
> classes of objects and we can just fix the regex to be more explicit about 
> it. For now we would keep the lldb_private::RegularExpression class, have it 
> have a llvm::Regex member and then lldbassert if the regex fails to compile 
> so we can catch any extended cruft that we might miss so we can fix it...
> 
> > On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> >
> > To be clear: if we can make StringRef work efficiently, I am fine with 
> > that. We can just cut over to using llvm::Regex where it uses the start and 
> > end pointer. I would just like to avoid making string copies for no reason. 
> > I don't have anything against using StringRef as long as we do it 
> > efficiently.
> >
> > Greg
> >
> >
> >> On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits 
> >>  wrote:
> >>
> >>>
> >>> On Sep 21, 2016, at 4:43 PM, Zachary Turner  wrote:
> >>>
> >>> You need to duplicate something on the heap once when you execute the 
> >>> regex.  And in turn you save tens or hundreds or copies on the way there 
> >>> because of inefficient string usage.
> >>
> >> Where? please show this.
> >>
> >> I see the following callers:
> >>
> >> const char *class_name =
> >> iterator->second->GetClassName().AsCString("");
> >> if (regex_up && class_name &&
> >> !regex_up->Execute(llvm::StringRef(class_name)))
> >>
> >> You are adding a strlen() call here to construct the StringRef, not saving 
> >> anything.
> >>
> >>
> >> bool CommandObjectRegexCommand::DoExecute(const char *command,
> >> CommandReturnObject ) {
> >> if (command) {
> >>   EntryCollection::const_iterator pos, end = m_entries.end();
> >>   for (pos = m_entries.begin(); pos != end; ++pos) {
> >> RegularExpression::Match regex_match(m_max_matches);
> >>
> >> if (pos->regex.Execute(command, _match)) {
> >>   std::string new_command(pos->command);
> >>   std::string match_str;
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >>
> >>   DataVisualization::Categories::ForEach(
> >>   [, ](const lldb::TypeCategoryImplSP _sp) -> 
> >> bool {
> >> if (regex) {
> >>   bool escape = true;
> >>   if (regex->GetText() == category_sp->GetName()) {
> >> escape = false;
> >>   } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
> >>  category_sp->GetName( {
> >> escape = false;
> >>   }
> >>
> >>   if (escape)
> >> return true;
> >> }
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >>
> >> TypeCategoryImpl::ForEachCallbacks foreach;
> >> foreach
> >>   .SetExact([, _regex, _printed](
> >> ConstString name,
> >> const FormatterSharedPointer _sp) -> bool {
> >> if (formatter_regex) {
> >>   bool escape = true;
> >>   if (name.GetStringRef() == formatter_regex->GetText()) {
> >> escape = false;
> >>   } else if (formatter_regex->Execute(name.GetStringRef())) {
> >> escape = false;
> >>   }
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >>   bool ParseCoordinate(const char *id_cstr) {
> >> RegularExpression regex;
> >> RegularExpression::Match regex_match(3);
> >>
> >> llvm::StringRef id_ref = llvm::StringRef::withNullAsEmpty(id_cstr);
> >> bool matched = false;
> >> if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
> >> regex.Execute(id_ref, _match))
> >>   matched = true;
> >> else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
> >>  regex.Execute(id_ref, _match))
> >>   matched = true;
> >> else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
> >>  regex.Execute(id_ref, _match))
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >> void DWARFCompileUnit::ParseProducerInfo() {
> >> m_producer_version_major = UINT32_MAX;
> >> m_producer_version_minor = UINT32_MAX;

Re: [Lldb-commits] [lldb] r282079 - Make lldb::Regex use StringRef.

[Greg Clayton via lldb-commits]

> On Sep 21, 2016, at 5:14 PM, Zachary Turner  wrote:
> 
> 
> 
> On Wed, Sep 21, 2016 at 5:00 PM Greg Clayton  wrote:
> Please submit a change requests when doing these kinds of things and let 
> people comment on the changes before committing such things.
> 
> We deleted functions that were correctly using "const char *" like:
> 
>   bool Execute(llvm::StringRef string, Match *match = nullptr) const;
>   bool Execute(const char *, Match * = nullptr) = delete;
> 
> Yet inside these functions you do "string.str().c_str()" because these need 
> to be null terminated? this indicates that StringRef is NOT the correct 
> choice here. You will make heap copy of all strings (if they are long enough 
> and regex matching string can be very long) that you compile or execute. We 
> already make a copy of the string when we compile, so StringRef didn't really 
> save us anything on compile and execute is always making a copy just to run 
> the expression on. I am fine with both being there in case one might be more 
> efficient, but taking them out just to use a less efficient version that uses 
> llvm::StringRef is not the kind of changes we should be doing all over.
> 
> We will make copies of all strings in all of the following changes:
> 
> - Unneeded copy:
> 
> -new TypeNameSpecifierImpl(regex->GetText(), true));
> +new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));
> All of these copies are only temporary.  As I've said over and over, once 
> everything is StringRef all the way down, the copies all disappear.  It's 
> only a copy because TypeNameSpecifierImpl doesn't take a STringRef, and I 
> didn't want to change the whole codebase all at once, but rather 
> incrementally.

As part of these changes we absolutely should add StringRef to these classes 
and we should just make it part of the changes. It know these strings are 
temporary, I am not worried about memory, I am worried about efficiency of the 
heap copy of the string that temporarily is created during this call. If things 
were taking "const char *" before, copies were not being made. If they were 
being made, then they were probably being made for good reason. I would love to 
see the places where all of these copies were removed, but if they were "const 
char *" there were no copies going in, unless they needed to. If they didn't 
need to make the copies, then we can fix that function.

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

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

That sounds like a plan.  BTW, extended is the one that everyone supports,
enhanced is the one that only apple supports.

On Wed, Sep 21, 2016 at 5:18 PM Greg Clayton  wrote:

> And we should check for any "extended" mode regex stuff and get rid of it
> since as you said they are not portable. They tend to be shortcuts for
> classes of objects and we can just fix the regex to be more explicit about
> it. For now we would keep the lldb_private::RegularExpression class, have
> it have a llvm::Regex member and then lldbassert if the regex fails to
> compile so we can catch any extended cruft that we might miss so we can fix
> it...
>
> > On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> >
> > To be clear: if we can make StringRef work efficiently, I am fine with
> that. We can just cut over to using llvm::Regex where it uses the start and
> end pointer. I would just like to avoid making string copies for no reason.
> I don't have anything against using StringRef as long as we do it
> efficiently.
> >
> > Greg
> >
> >
> >> On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >>
> >>>
> >>> On Sep 21, 2016, at 4:43 PM, Zachary Turner 
> wrote:
> >>>
> >>> You need to duplicate something on the heap once when you execute the
> regex.  And in turn you save tens or hundreds or copies on the way there
> because of inefficient string usage.
> >>
> >> Where? please show this.
> >>
> >> I see the following callers:
> >>
> >> const char *class_name =
> >> iterator->second->GetClassName().AsCString("");
> >> if (regex_up && class_name &&
> >> !regex_up->Execute(llvm::StringRef(class_name)))
> >>
> >> You are adding a strlen() call here to construct the StringRef, not
> saving anything.
> >>
> >>
> >> bool CommandObjectRegexCommand::DoExecute(const char *command,
> >> CommandReturnObject ) {
> >> if (command) {
> >>   EntryCollection::const_iterator pos, end = m_entries.end();
> >>   for (pos = m_entries.begin(); pos != end; ++pos) {
> >> RegularExpression::Match regex_match(m_max_matches);
> >>
> >> if (pos->regex.Execute(command, _match)) {
> >>   std::string new_command(pos->command);
> >>   std::string match_str;
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >>
> >>   DataVisualization::Categories::ForEach(
> >>   [, ](const lldb::TypeCategoryImplSP _sp) ->
> bool {
> >> if (regex) {
> >>   bool escape = true;
> >>   if (regex->GetText() == category_sp->GetName()) {
> >> escape = false;
> >>   } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
> >>  category_sp->GetName( {
> >> escape = false;
> >>   }
> >>
> >>   if (escape)
> >> return true;
> >> }
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >>
> >> TypeCategoryImpl::ForEachCallbacks foreach;
> >> foreach
> >>   .SetExact([, _regex, _printed](
> >> ConstString name,
> >> const FormatterSharedPointer _sp) -> bool {
> >> if (formatter_regex) {
> >>   bool escape = true;
> >>   if (name.GetStringRef() == formatter_regex->GetText()) {
> >> escape = false;
> >>   } else if (formatter_regex->Execute(name.GetStringRef())) {
> >> escape = false;
> >>   }
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >>   bool ParseCoordinate(const char *id_cstr) {
> >> RegularExpression regex;
> >> RegularExpression::Match regex_match(3);
> >>
> >> llvm::StringRef id_ref = llvm::StringRef::withNullAsEmpty(id_cstr);
> >> bool matched = false;
> >> if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$"))
> &&
> >> regex.Execute(id_ref, _match))
> >>   matched = true;
> >> else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
> >>  regex.Execute(id_ref, _match))
> >>   matched = true;
> >> else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
> >>  regex.Execute(id_ref, _match))
> >>
> >> No copy saved. Just wasted time with strlen in StringRef constructor.
> >>
> >> void DWARFCompileUnit::ParseProducerInfo() {
> >> m_producer_version_major = UINT32_MAX;
> >> m_producer_version_minor = UINT32_MAX;
> >> m_producer_version_update = UINT32_MAX;
> >>
> >> const DWARFDebugInfoEntry *die = GetCompileUnitDIEPtrOnly();
> >> if (die) {
> >>
> >>   const char *producer_cstr = die->GetAttributeValueAsString(
> >>   m_dwarf2Data, this, DW_AT_producer, NULL);
> >>   if (producer_cstr) {
> >> RegularExpression llvm_gcc_regex(
> >> llvm::StringRef("^4\\.[012]\\.[01] \\(Based on Apple "
> >> "Inc\\. build 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Greg Clayton via lldb-commits]

And we should check for any "extended" mode regex stuff and get rid of it since 
as you said they are not portable. They tend to be shortcuts for classes of 
objects and we can just fix the regex to be more explicit about it. For now we 
would keep the lldb_private::RegularExpression class, have it have a 
llvm::Regex member and then lldbassert if the regex fails to compile so we can 
catch any extended cruft that we might miss so we can fix it...

> On Sep 21, 2016, at 5:15 PM, Greg Clayton  wrote:
> 
> To be clear: if we can make StringRef work efficiently, I am fine with that. 
> We can just cut over to using llvm::Regex where it uses the start and end 
> pointer. I would just like to avoid making string copies for no reason. I 
> don't have anything against using StringRef as long as we do it efficiently.
> 
> Greg
> 
> 
>> On Sep 21, 2016, at 5:13 PM, Greg Clayton via lldb-commits 
>>  wrote:
>> 
>>> 
>>> On Sep 21, 2016, at 4:43 PM, Zachary Turner  wrote:
>>> 
>>> You need to duplicate something on the heap once when you execute the 
>>> regex.  And in turn you save tens or hundreds or copies on the way there 
>>> because of inefficient string usage.  
>> 
>> Where? please show this.
>> 
>> I see the following callers:
>> 
>> const char *class_name =
>> iterator->second->GetClassName().AsCString("");
>> if (regex_up && class_name &&
>> !regex_up->Execute(llvm::StringRef(class_name)))
>> 
>> You are adding a strlen() call here to construct the StringRef, not saving 
>> anything.
>> 
>> 
>> bool CommandObjectRegexCommand::DoExecute(const char *command,
>> CommandReturnObject ) {
>> if (command) {
>>   EntryCollection::const_iterator pos, end = m_entries.end();
>>   for (pos = m_entries.begin(); pos != end; ++pos) {
>> RegularExpression::Match regex_match(m_max_matches);
>> 
>> if (pos->regex.Execute(command, _match)) {
>>   std::string new_command(pos->command);
>>   std::string match_str;
>> 
>> No copy saved. Just wasted time with strlen in StringRef constructor.
>> 
>> 
>>   DataVisualization::Categories::ForEach(
>>   [, ](const lldb::TypeCategoryImplSP _sp) -> bool 
>> {
>> if (regex) {
>>   bool escape = true;
>>   if (regex->GetText() == category_sp->GetName()) {
>> escape = false;
>>   } else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
>>  category_sp->GetName( {
>> escape = false;
>>   }
>> 
>>   if (escape)
>> return true;
>> }
>> 
>> No copy saved. Just wasted time with strlen in StringRef constructor.
>> 
>> 
>> TypeCategoryImpl::ForEachCallbacks foreach;
>> foreach
>>   .SetExact([, _regex, _printed](
>> ConstString name,
>> const FormatterSharedPointer _sp) -> bool {
>> if (formatter_regex) {
>>   bool escape = true;
>>   if (name.GetStringRef() == formatter_regex->GetText()) {
>> escape = false;
>>   } else if (formatter_regex->Execute(name.GetStringRef())) {
>> escape = false;
>>   }
>> 
>> No copy saved. Just wasted time with strlen in StringRef constructor.
>> 
>>   bool ParseCoordinate(const char *id_cstr) {
>> RegularExpression regex;
>> RegularExpression::Match regex_match(3);
>> 
>> llvm::StringRef id_ref = llvm::StringRef::withNullAsEmpty(id_cstr);
>> bool matched = false;
>> if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
>> regex.Execute(id_ref, _match))
>>   matched = true;
>> else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
>>  regex.Execute(id_ref, _match))
>>   matched = true;
>> else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
>>  regex.Execute(id_ref, _match))
>> 
>> No copy saved. Just wasted time with strlen in StringRef constructor.
>> 
>> void DWARFCompileUnit::ParseProducerInfo() {
>> m_producer_version_major = UINT32_MAX;
>> m_producer_version_minor = UINT32_MAX;
>> m_producer_version_update = UINT32_MAX;
>> 
>> const DWARFDebugInfoEntry *die = GetCompileUnitDIEPtrOnly();
>> if (die) {
>> 
>>   const char *producer_cstr = die->GetAttributeValueAsString(
>>   m_dwarf2Data, this, DW_AT_producer, NULL);
>>   if (producer_cstr) {
>> RegularExpression llvm_gcc_regex(
>> llvm::StringRef("^4\\.[012]\\.[01] \\(Based on Apple "
>> "Inc\\. build [0-9]+\\) \\(LLVM build "
>> "[\\.0-9]+\\)$"));
>> if (llvm_gcc_regex.Execute(llvm::StringRef(producer_cstr))) {
>>   m_producer = eProducerLLVMGCC;
>> } else if (strstr(producer_cstr, "clang")) {
>>   static RegularExpression g_clang_version_regex(
>>   llvm::StringRef("clang-([0-9]+)\\.([0-9]+)\\.([0-9]+)"));
>>   

Re: [Lldb-commits] [lldb] r282079 - Make lldb::Regex use StringRef.

[Zachary Turner via lldb-commits]

On Wed, Sep 21, 2016 at 5:00 PM Greg Clayton  wrote:

> Please submit a change requests when doing these kinds of things and let
> people comment on the changes before committing such things.
>
> We deleted functions that were correctly using "const char *" like:
>
>   bool Execute(llvm::StringRef string, Match *match = nullptr) const;
>   bool Execute(const char *, Match * = nullptr) = delete;
>
> Yet inside these functions you do "string.str().c_str()" because these
> need to be null terminated? this indicates that StringRef is NOT the
> correct choice here. You will make heap copy of all strings (if they are
> long enough and regex matching string can be very long) that you compile or
> execute. We already make a copy of the string when we compile, so StringRef
> didn't really save us anything on compile and execute is always making a
> copy just to run the expression on. I am fine with both being there in case
> one might be more efficient, but taking them out just to use a less
> efficient version that uses llvm::StringRef is not the kind of changes we
> should be doing all over.
>
> We will make copies of all strings in all of the following changes:
>
> - Unneeded copy:
>
> -new TypeNameSpecifierImpl(regex->GetText(), true));
> +new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));
>
All of these copies are only temporary.  As I've said over and over, once
everything is StringRef all the way down, the copies all disappear.  It's
only a copy because TypeNameSpecifierImpl doesn't take a STringRef, and I
didn't want to change the whole codebase all at once, but rather
incrementally.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Greg Clayton via lldb-commits]

> On Sep 21, 2016, at 4:43 PM, Zachary Turner  wrote:
> 
> You need to duplicate something on the heap once when you execute the regex.  
> And in turn you save tens or hundreds or copies on the way there because of 
> inefficient string usage.  

Where? please show this.

I see the following callers:

  const char *class_name =
  iterator->second->GetClassName().AsCString("");
  if (regex_up && class_name &&
  !regex_up->Execute(llvm::StringRef(class_name)))

You are adding a strlen() call here to construct the StringRef, not saving 
anything.


bool CommandObjectRegexCommand::DoExecute(const char *command,
  CommandReturnObject ) {
  if (command) {
EntryCollection::const_iterator pos, end = m_entries.end();
for (pos = m_entries.begin(); pos != end; ++pos) {
  RegularExpression::Match regex_match(m_max_matches);

  if (pos->regex.Execute(command, _match)) {
std::string new_command(pos->command);
std::string match_str;

No copy saved. Just wasted time with strlen in StringRef constructor.


DataVisualization::Categories::ForEach(
[, ](const lldb::TypeCategoryImplSP _sp) -> bool {
  if (regex) {
bool escape = true;
if (regex->GetText() == category_sp->GetName()) {
  escape = false;
} else if (regex->Execute(llvm::StringRef::withNullAsEmpty(
   category_sp->GetName( {
  escape = false;
}

if (escape)
  return true;
  }

No copy saved. Just wasted time with strlen in StringRef constructor.


  TypeCategoryImpl::ForEachCallbacks foreach;
  foreach
.SetExact([, _regex, _printed](
  ConstString name,
  const FormatterSharedPointer _sp) -> bool {
  if (formatter_regex) {
bool escape = true;
if (name.GetStringRef() == formatter_regex->GetText()) {
  escape = false;
} else if (formatter_regex->Execute(name.GetStringRef())) {
  escape = false;
}

No copy saved. Just wasted time with strlen in StringRef constructor.

bool ParseCoordinate(const char *id_cstr) {
  RegularExpression regex;
  RegularExpression::Match regex_match(3);

  llvm::StringRef id_ref = llvm::StringRef::withNullAsEmpty(id_cstr);
  bool matched = false;
  if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+),([0-9]+)$")) &&
  regex.Execute(id_ref, _match))
matched = true;
  else if (regex.Compile(llvm::StringRef("^([0-9]+),([0-9]+)$")) &&
   regex.Execute(id_ref, _match))
matched = true;
  else if (regex.Compile(llvm::StringRef("^([0-9]+)$")) &&
   regex.Execute(id_ref, _match))

No copy saved. Just wasted time with strlen in StringRef constructor.

void DWARFCompileUnit::ParseProducerInfo() {
  m_producer_version_major = UINT32_MAX;
  m_producer_version_minor = UINT32_MAX;
  m_producer_version_update = UINT32_MAX;

  const DWARFDebugInfoEntry *die = GetCompileUnitDIEPtrOnly();
  if (die) {

const char *producer_cstr = die->GetAttributeValueAsString(
m_dwarf2Data, this, DW_AT_producer, NULL);
if (producer_cstr) {
  RegularExpression llvm_gcc_regex(
  llvm::StringRef("^4\\.[012]\\.[01] \\(Based on Apple "
  "Inc\\. build [0-9]+\\) \\(LLVM build "
  "[\\.0-9]+\\)$"));
  if (llvm_gcc_regex.Execute(llvm::StringRef(producer_cstr))) {
m_producer = eProducerLLVMGCC;
  } else if (strstr(producer_cstr, "clang")) {
static RegularExpression g_clang_version_regex(
llvm::StringRef("clang-([0-9]+)\\.([0-9]+)\\.([0-9]+)"));
RegularExpression::Match regex_match(3);
if (g_clang_version_regex.Execute(llvm::StringRef(producer_cstr),
  _match)) {

No copy saved. Just wasted time with strlen in StringRef constructor (2 of them 
mind you).

void DWARFDebugPubnamesSet::Find(
const RegularExpression ,
std::vector _offset_coll) const {
  DescriptorConstIter pos;
  DescriptorConstIter end = m_descriptors.end();
  for (pos = m_descriptors.begin(); pos != end; ++pos) {
if (regex.Execute(pos->name.c_str()))
  die_offset_coll.push_back(m_header.die_offset + pos->offset);
  }
}

No copy saved. Just wasted time with strlen in StringRef constructor (2 of them 
mind you).


  std::string slice_str;
  if (reg_info_dict->GetValueForKeyAsString("slice", slice_str, nullptr)) {
// Slices use the following format:
//  REGNAME[MSBIT:LSBIT]
// REGNAME - name of the register to grab a slice of
// MSBIT - the most significant bit at which the current register value
// starts at
// LSBIT - the least significant bit at which the current register value
// 

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Incidentally, it appears the version of regcomp() on apple supports REG_PEND

, although that is not a standard.  And apple is already the only platform
where enhanced regexes are a thing.  So if you want to keep enhanced
regexes, one thing we could do is:

a) Have all non Apple platforms delegate to the LLVM implementation.
b) Have Apple's implementation use the system library but set the end
pointer.

That said, if you don't care about enhanced mode, let's just have everyone
use LLVM's.

On Wed, Sep 21, 2016 at 5:03 PM Zachary Turner  wrote:

> It supports extended, just not *enhanced*.  Enhanced appears to be an
> apple specific thing, and even the developer documentation recommends not
> using it in portable code.
>
> On Wed, Sep 21, 2016 at 5:00 PM Jim Ingham  wrote:
>
> IIRC you said the llvm one doesn't support extended regular expressions.
> If that's true, please don't do this till that is fixed.  I think pretty
> much everybody who is using reg expressions expects to be able to use
> extended regular expressions.
>
> Jim
>
> > On Sep 21, 2016, at 4:58 PM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Incidentally even the STL regex implementation supports specifying the
> end pointer.  So I would call the system one deficient in this regard and
> advocate for replacing it sooner rather than later.
> >
> > On Wed, Sep 21, 2016 at 4:52 PM Zachary Turner 
> wrote:
> > Looks like llvm's regex is better than LLDB's in this regard, since it
> supports explicitly setting the end pointer.  I can see a couple options:
> >
> > 1) Check if it's null terminated by peeking one past the end, and
> copying if it's not.  This is pretty hackish, not crazy about this idea.
> > 2) Un-delete the const char * version of the function but leave the
> StringRef overload, find all places where I added the explicit conversion
> and remove them so they invoke the const char* overload.
> > 3) Change lldb::RegularExpression to just delegate to llvm under the
> hood and set the end pointer.
> >
> > On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner 
> wrote:
> > Actually wait, it doesn't.  It just explicitly sets the end pointer.
> >
> > On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner 
> wrote:
> > Worth noting that llvm::Regex has this constructor:
> >
> >
> > Regex::Regex(StringRef regex, unsigned Flags) {
> >   unsigned flags = 0;
> >   preg = new llvm_regex();
> >   preg->re_endp = regex.end();
> >   if (Flags & IgnoreCase)
> > flags |= REG_ICASE;
> >   if (Flags & Newline)
> > flags |= REG_NEWLINE;
> >   if (!(Flags & BasicRegex))
> > flags |= REG_EXTENDED;
> >   error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);
> > }
> >
> > So it assumes null termination even though you have a StringRef.
> >
> > On Wed, Sep 21, 2016 at 4:43 PM Zachary Turner 
> wrote:
> > You need to duplicate something on the heap once when you execute the
> regex.  And in turn you save tens or hundreds or copies on the way there
> because of inefficient string usage.
> >
> > We could also just un-delete the overload that takes a const char*, then
> the duplication would only ever happen when you explicitly use a StringRef.
> >
> > I don't agree this should be reverted.  In the process of doing this
> conversion I eliminated numerous careless string copies.
> >
> > On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton  wrote:
> > This it the perfect example of why not to use a StringRef since the
> string needs to be null terminated. Why did we change this? Now even if you
> call this function:
> >
> > RegularExpression r(...);
> >
> > r.Execute("...", ...)
> >
> > You will need to duplicate the string on the heap just to execute this.
> Please revert this. Anything that requires null terminate is not a
> candidate for converting to StringRef.
> >
> >
> > > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> > >
> > > Author: zturner
> > > Date: Wed Sep 21 12:13:51 2016
> > > New Revision: 282090
> > >
> > > URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> > > Log:
> > > Fix failing regex tests.
> > >
> > > r282079 converted the regular expression interface to accept
> > > and return StringRefs instead of char pointers.  In one case
> > > a null pointer check was converted to an empty string check,
> > > but this was an incorrect conversion because an empty string
> > > is a valid regular expression.  Removing this check should
> > > fix the test failures.
> > >
> > > Modified:
> > >lldb/trunk/source/Core/RegularExpression.cpp
> > >
> > > Modified: lldb/trunk/source/Core/RegularExpression.cpp
> > > URL:
> 

Re: [Lldb-commits] [lldb] r282079 - Make lldb::Regex use StringRef.

[Greg Clayton via lldb-commits]

Please submit a change requests when doing these kinds of things and let people 
comment on the changes before committing such things.

We deleted functions that were correctly using "const char *" like:

  bool Execute(llvm::StringRef string, Match *match = nullptr) const;
  bool Execute(const char *, Match * = nullptr) = delete;

Yet inside these functions you do "string.str().c_str()" because these need to 
be null terminated? this indicates that StringRef is NOT the correct choice 
here. You will make heap copy of all strings (if they are long enough and regex 
matching string can be very long) that you compile or execute. We already make 
a copy of the string when we compile, so StringRef didn't really save us 
anything on compile and execute is always making a copy just to run the 
expression on. I am fine with both being there in case one might be more 
efficient, but taking them out just to use a less efficient version that uses 
llvm::StringRef is not the kind of changes we should be doing all over.

We will make copies of all strings in all of the following changes:

- Unneeded copy:

-new TypeNameSpecifierImpl(regex->GetText(), true));
+new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));


- Unneeded copy just to print in print (there are tons of these):

void BreakpointResolverFileRegex::GetDescription(Stream *s) {
-  s->Printf("source regex = \"%s\", exact_match = %d", m_regex.GetText(),
-m_exact_match);
+  s->Printf("source regex = \"%s\", exact_match = %d",
+m_regex.GetText().str().c_str(), m_exact_match);
}

If you use a StringRef in printf, please use "%*s" and then put the count and 
data pointer into the printf, so the above line would become:

+  s->Printf("source regex = \"%*s\", exact_match = %d",
+(int)m_regex.GetText().size(), m_regex.GetText().data(), 
m_exact_match);


- Unneeded copy:

lldb::OptionValueSP OptionValueRegex::DeepCopy() const {
-  return OptionValueSP(new OptionValueRegex(m_regex.GetText()));
+  return OptionValueSP(new OptionValueRegex(m_regex.GetText().str().c_str()));
}

So please submit things for review before committing them so we can catch 
things like this.

Things I would like to see fixed:
- convert all Printf functions that are not in "if (log) log->Printf(...)" 
(which eliminates most of the changes you need to make) uses of StringRef over 
to use the "%*s" and be careful that the "*" requires an integer length, not a 
size_t length, so you will need to cast the size of the string to int.
- Revert changing RegularExpression::Execute over to using StringRef as it is 
the wrong thing to do
- Either convert functions over to use StringRef as parameters so we can avoid 
extra copies (anything that calls "str().c_str()" in a parameters to functions) 
or revert back to "const char *" and don't use StringRef

> On Sep 21, 2016, at 9:01 AM, Zachary Turner via lldb-commits 
>  wrote:
> 
> Author: zturner
> Date: Wed Sep 21 11:01:28 2016
> New Revision: 282079
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=282079=rev
> Log:
> Make lldb::Regex use StringRef.
> 
> This updates getters and setters to use StringRef instead of
> const char *.  I tested the build on Linux, Windows, and OSX
> and saw no build or test failures.  I cannot test any BSD
> or Android variants, however I expect the required changes
> to be minimal or non-existant.
> 
> Modified:
>lldb/trunk/include/lldb/Breakpoint/BreakpointResolver.h
>lldb/trunk/include/lldb/Core/RegularExpression.h
>lldb/trunk/include/lldb/Core/Stream.h
>lldb/trunk/include/lldb/DataFormatters/FormattersContainer.h
>lldb/trunk/include/lldb/Interpreter/OptionValueRegex.h
>lldb/trunk/include/lldb/Interpreter/OptionValueString.h
>lldb/trunk/source/API/SBTarget.cpp
>lldb/trunk/source/API/SBTypeCategory.cpp
>lldb/trunk/source/Breakpoint/BreakpointResolver.cpp
>lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
>lldb/trunk/source/Breakpoint/BreakpointResolverFileRegex.cpp
>lldb/trunk/source/Breakpoint/BreakpointResolverName.cpp
>lldb/trunk/source/Commands/CommandCompletions.cpp
>lldb/trunk/source/Commands/CommandObjectBreakpoint.cpp
>lldb/trunk/source/Commands/CommandObjectFrame.cpp
>lldb/trunk/source/Commands/CommandObjectTarget.cpp
>lldb/trunk/source/Commands/CommandObjectType.cpp
>lldb/trunk/source/Core/AddressResolverName.cpp
>lldb/trunk/source/Core/Disassembler.cpp
>lldb/trunk/source/Core/Module.cpp
>lldb/trunk/source/Core/RegularExpression.cpp
>lldb/trunk/source/Core/SourceManager.cpp
>lldb/trunk/source/DataFormatters/FormatManager.cpp
>lldb/trunk/source/DataFormatters/FormattersHelpers.cpp
>lldb/trunk/source/Host/common/FileSpec.cpp
>lldb/trunk/source/Host/common/Socket.cpp
>lldb/trunk/source/Interpreter/Args.cpp
>lldb/trunk/source/Interpreter/CommandObjectRegexCommand.cpp
>

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Jim Ingham via lldb-commits]

IIRC you said the llvm one doesn't support extended regular expressions.  If 
that's true, please don't do this till that is fixed.  I think pretty much 
everybody who is using reg expressions expects to be able to use extended 
regular expressions.

Jim

> On Sep 21, 2016, at 4:58 PM, Zachary Turner via lldb-commits 
>  wrote:
> 
> Incidentally even the STL regex implementation supports specifying the end 
> pointer.  So I would call the system one deficient in this regard and 
> advocate for replacing it sooner rather than later.
> 
> On Wed, Sep 21, 2016 at 4:52 PM Zachary Turner  wrote:
> Looks like llvm's regex is better than LLDB's in this regard, since it 
> supports explicitly setting the end pointer.  I can see a couple options:
> 
> 1) Check if it's null terminated by peeking one past the end, and copying if 
> it's not.  This is pretty hackish, not crazy about this idea.
> 2) Un-delete the const char * version of the function but leave the StringRef 
> overload, find all places where I added the explicit conversion and remove 
> them so they invoke the const char* overload.
> 3) Change lldb::RegularExpression to just delegate to llvm under the hood and 
> set the end pointer.
> 
> On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner  wrote:
> Actually wait, it doesn't.  It just explicitly sets the end pointer.
> 
> On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner  wrote:
> Worth noting that llvm::Regex has this constructor:
> 
> 
> Regex::Regex(StringRef regex, unsigned Flags) {
>   unsigned flags = 0;
>   preg = new llvm_regex();
>   preg->re_endp = regex.end();
>   if (Flags & IgnoreCase) 
> flags |= REG_ICASE;
>   if (Flags & Newline)
> flags |= REG_NEWLINE;
>   if (!(Flags & BasicRegex))
> flags |= REG_EXTENDED;
>   error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);
> }
> 
> So it assumes null termination even though you have a StringRef.
> 
> On Wed, Sep 21, 2016 at 4:43 PM Zachary Turner  wrote:
> You need to duplicate something on the heap once when you execute the regex.  
> And in turn you save tens or hundreds or copies on the way there because of 
> inefficient string usage.  
> 
> We could also just un-delete the overload that takes a const char*, then the 
> duplication would only ever happen when you explicitly use a StringRef.
> 
> I don't agree this should be reverted.  In the process of doing this 
> conversion I eliminated numerous careless string copies.
> 
> On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton  wrote:
> This it the perfect example of why not to use a StringRef since the string 
> needs to be null terminated. Why did we change this? Now even if you call 
> this function:
> 
> RegularExpression r(...);
> 
> r.Execute("...", ...)
> 
> You will need to duplicate the string on the heap just to execute this. 
> Please revert this. Anything that requires null terminate is not a candidate 
> for converting to StringRef.
> 
> 
> > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits 
> >  wrote:
> >
> > Author: zturner
> > Date: Wed Sep 21 12:13:51 2016
> > New Revision: 282090
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> > Log:
> > Fix failing regex tests.
> >
> > r282079 converted the regular expression interface to accept
> > and return StringRefs instead of char pointers.  In one case
> > a null pointer check was converted to an empty string check,
> > but this was an incorrect conversion because an empty string
> > is a valid regular expression.  Removing this check should
> > fix the test failures.
> >
> > Modified:
> >lldb/trunk/source/Core/RegularExpression.cpp
> >
> > Modified: lldb/trunk/source/Core/RegularExpression.cpp
> > URL: 
> > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
> > ==
> > --- lldb/trunk/source/Core/RegularExpression.cpp (original)
> > +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
> > @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
> > //-
> > bool RegularExpression::Execute(llvm::StringRef str, Match *match) const {
> >   int err = 1;
> > -  if (!str.empty() && m_comp_err == 0) {
> > +  if (m_comp_err == 0) {
> > // Argument to regexec must be null-terminated.
> > std::string reg_str = str;
> > if (match) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Incidentally even the STL regex implementation supports specifying the end
pointer.  So I would call the system one deficient in this regard and
advocate for replacing it sooner rather than later.

On Wed, Sep 21, 2016 at 4:52 PM Zachary Turner  wrote:

> Looks like llvm's regex is better than LLDB's in this regard, since it
> supports explicitly setting the end pointer.  I can see a couple options:
>
> 1) Check if it's null terminated by peeking one past the end, and copying
> if it's not.  This is pretty hackish, not crazy about this idea.
> 2) Un-delete the const char * version of the function but leave the
> StringRef overload, find all places where I added the explicit conversion
> and remove them so they invoke the const char* overload.
> 3) Change lldb::RegularExpression to just delegate to llvm under the hood
> and set the end pointer.
>
> On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner  wrote:
>
> Actually wait, it doesn't.  It just explicitly sets the end pointer.
>
> On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner  wrote:
>
> Worth noting that llvm::Regex has this constructor:
>
>
> Regex::Regex(StringRef regex, unsigned Flags) {
>   unsigned flags = 0;
>   preg = new llvm_regex();
>   preg->re_endp = regex.end();
>   if (Flags & IgnoreCase)
> flags |= REG_ICASE;
>   if (Flags & Newline)
> flags |= REG_NEWLINE;
>   if (!(Flags & BasicRegex))
> flags |= REG_EXTENDED;
>   error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);
> }
>
> So it assumes null termination even though you have a StringRef.
>
> On Wed, Sep 21, 2016 at 4:43 PM Zachary Turner  wrote:
>
> You need to duplicate something on the heap once when you execute the
> regex.  And in turn you save tens or hundreds or copies on the way there
> because of inefficient string usage.
>
> We could also just un-delete the overload that takes a const char*, then
> the duplication would only ever happen when you explicitly use a StringRef.
>
> I don't agree this should be reverted.  In the process of doing this
> conversion I eliminated numerous careless string copies.
>
> On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton  wrote:
>
> This it the perfect example of why not to use a StringRef since the string
> needs to be null terminated. Why did we change this? Now even if you call
> this function:
>
> RegularExpression r(...);
>
> r.Execute("...", ...)
>
> You will need to duplicate the string on the heap just to execute this.
> Please revert this. Anything that requires null terminate is not a
> candidate for converting to StringRef.
>
>
> > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Wed Sep 21 12:13:51 2016
> > New Revision: 282090
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> > Log:
> > Fix failing regex tests.
> >
> > r282079 converted the regular expression interface to accept
> > and return StringRefs instead of char pointers.  In one case
> > a null pointer check was converted to an empty string check,
> > but this was an incorrect conversion because an empty string
> > is a valid regular expression.  Removing this check should
> > fix the test failures.
> >
> > Modified:
> >lldb/trunk/source/Core/RegularExpression.cpp
> >
> > Modified: lldb/trunk/source/Core/RegularExpression.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
> >
> ==
> > --- lldb/trunk/source/Core/RegularExpression.cpp (original)
> > +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
> > @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
> > //-
> > bool RegularExpression::Execute(llvm::StringRef str, Match *match) const
> {
> >   int err = 1;
> > -  if (!str.empty() && m_comp_err == 0) {
> > +  if (m_comp_err == 0) {
> > // Argument to regexec must be null-terminated.
> > std::string reg_str = str;
> > if (match) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Actually wait, it doesn't.  It just explicitly sets the end pointer.

On Wed, Sep 21, 2016 at 4:44 PM Zachary Turner  wrote:

> Worth noting that llvm::Regex has this constructor:
>
>
> Regex::Regex(StringRef regex, unsigned Flags) {
>   unsigned flags = 0;
>   preg = new llvm_regex();
>   preg->re_endp = regex.end();
>   if (Flags & IgnoreCase)
> flags |= REG_ICASE;
>   if (Flags & Newline)
> flags |= REG_NEWLINE;
>   if (!(Flags & BasicRegex))
> flags |= REG_EXTENDED;
>   error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);
> }
>
> So it assumes null termination even though you have a StringRef.
>
> On Wed, Sep 21, 2016 at 4:43 PM Zachary Turner  wrote:
>
> You need to duplicate something on the heap once when you execute the
> regex.  And in turn you save tens or hundreds or copies on the way there
> because of inefficient string usage.
>
> We could also just un-delete the overload that takes a const char*, then
> the duplication would only ever happen when you explicitly use a StringRef.
>
> I don't agree this should be reverted.  In the process of doing this
> conversion I eliminated numerous careless string copies.
>
> On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton  wrote:
>
> This it the perfect example of why not to use a StringRef since the string
> needs to be null terminated. Why did we change this? Now even if you call
> this function:
>
> RegularExpression r(...);
>
> r.Execute("...", ...)
>
> You will need to duplicate the string on the heap just to execute this.
> Please revert this. Anything that requires null terminate is not a
> candidate for converting to StringRef.
>
>
> > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Wed Sep 21 12:13:51 2016
> > New Revision: 282090
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> > Log:
> > Fix failing regex tests.
> >
> > r282079 converted the regular expression interface to accept
> > and return StringRefs instead of char pointers.  In one case
> > a null pointer check was converted to an empty string check,
> > but this was an incorrect conversion because an empty string
> > is a valid regular expression.  Removing this check should
> > fix the test failures.
> >
> > Modified:
> >lldb/trunk/source/Core/RegularExpression.cpp
> >
> > Modified: lldb/trunk/source/Core/RegularExpression.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
> >
> ==
> > --- lldb/trunk/source/Core/RegularExpression.cpp (original)
> > +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
> > @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
> > //-
> > bool RegularExpression::Execute(llvm::StringRef str, Match *match) const
> {
> >   int err = 1;
> > -  if (!str.empty() && m_comp_err == 0) {
> > +  if (m_comp_err == 0) {
> > // Argument to regexec must be null-terminated.
> > std::string reg_str = str;
> > if (match) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Worth noting that llvm::Regex has this constructor:


Regex::Regex(StringRef regex, unsigned Flags) {
  unsigned flags = 0;
  preg = new llvm_regex();
  preg->re_endp = regex.end();
  if (Flags & IgnoreCase)
flags |= REG_ICASE;
  if (Flags & Newline)
flags |= REG_NEWLINE;
  if (!(Flags & BasicRegex))
flags |= REG_EXTENDED;
  error = llvm_regcomp(preg, regex.data(), flags|REG_PEND);
}

So it assumes null termination even though you have a StringRef.

On Wed, Sep 21, 2016 at 4:43 PM Zachary Turner  wrote:

> You need to duplicate something on the heap once when you execute the
> regex.  And in turn you save tens or hundreds or copies on the way there
> because of inefficient string usage.
>
> We could also just un-delete the overload that takes a const char*, then
> the duplication would only ever happen when you explicitly use a StringRef.
>
> I don't agree this should be reverted.  In the process of doing this
> conversion I eliminated numerous careless string copies.
>
> On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton  wrote:
>
> This it the perfect example of why not to use a StringRef since the string
> needs to be null terminated. Why did we change this? Now even if you call
> this function:
>
> RegularExpression r(...);
>
> r.Execute("...", ...)
>
> You will need to duplicate the string on the heap just to execute this.
> Please revert this. Anything that requires null terminate is not a
> candidate for converting to StringRef.
>
>
> > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Wed Sep 21 12:13:51 2016
> > New Revision: 282090
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> > Log:
> > Fix failing regex tests.
> >
> > r282079 converted the regular expression interface to accept
> > and return StringRefs instead of char pointers.  In one case
> > a null pointer check was converted to an empty string check,
> > but this was an incorrect conversion because an empty string
> > is a valid regular expression.  Removing this check should
> > fix the test failures.
> >
> > Modified:
> >lldb/trunk/source/Core/RegularExpression.cpp
> >
> > Modified: lldb/trunk/source/Core/RegularExpression.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
> >
> ==
> > --- lldb/trunk/source/Core/RegularExpression.cpp (original)
> > +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
> > @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
> > //-
> > bool RegularExpression::Execute(llvm::StringRef str, Match *match) const
> {
> >   int err = 1;
> > -  if (!str.empty() && m_comp_err == 0) {
> > +  if (m_comp_err == 0) {
> > // Argument to regexec must be null-terminated.
> > std::string reg_str = str;
> > if (match) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

You need to duplicate something on the heap once when you execute the
regex.  And in turn you save tens or hundreds or copies on the way there
because of inefficient string usage.

We could also just un-delete the overload that takes a const char*, then
the duplication would only ever happen when you explicitly use a StringRef.

I don't agree this should be reverted.  In the process of doing this
conversion I eliminated numerous careless string copies.

On Wed, Sep 21, 2016 at 4:38 PM Greg Clayton  wrote:

> This it the perfect example of why not to use a StringRef since the string
> needs to be null terminated. Why did we change this? Now even if you call
> this function:
>
> RegularExpression r(...);
>
> r.Execute("...", ...)
>
> You will need to duplicate the string on the heap just to execute this.
> Please revert this. Anything that requires null terminate is not a
> candidate for converting to StringRef.
>
>
> > On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits <
> lldb-commits@lists.llvm.org> wrote:
> >
> > Author: zturner
> > Date: Wed Sep 21 12:13:51 2016
> > New Revision: 282090
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> > Log:
> > Fix failing regex tests.
> >
> > r282079 converted the regular expression interface to accept
> > and return StringRefs instead of char pointers.  In one case
> > a null pointer check was converted to an empty string check,
> > but this was an incorrect conversion because an empty string
> > is a valid regular expression.  Removing this check should
> > fix the test failures.
> >
> > Modified:
> >lldb/trunk/source/Core/RegularExpression.cpp
> >
> > Modified: lldb/trunk/source/Core/RegularExpression.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
> >
> ==
> > --- lldb/trunk/source/Core/RegularExpression.cpp (original)
> > +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
> > @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
> > //-
> > bool RegularExpression::Execute(llvm::StringRef str, Match *match) const
> {
> >   int err = 1;
> > -  if (!str.empty() && m_comp_err == 0) {
> > +  if (m_comp_err == 0) {
> > // Argument to regexec must be null-terminated.
> > std::string reg_str = str;
> > if (match) {
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282112 - Fix integer sign warning from r282105

[Adrian McCarthy via lldb-commits]

Thanks for fixing it.

On Wed, Sep 21, 2016 at 4:20 PM, Ed Maste  wrote:

> On 21 September 2016 at 21:38, Adrian McCarthy 
> wrote:
> > That fix doesn't look complete:
>
> Thanks, I've applied your fix in r282119, and sorry for being hasty
> with the original change.
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12158: Fix typo in lldb --help

[Ed Maste via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL282123: Fix typo in lldb --help (authored by emaste).

Changed prior to commit:
  https://reviews.llvm.org/D12158?vs=32567=72133#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D12158

Files:
  lldb/trunk/tools/driver/Driver.cpp

Index: lldb/trunk/tools/driver/Driver.cpp
===
--- lldb/trunk/tools/driver/Driver.cpp
+++ lldb/trunk/tools/driver/Driver.cpp
@@ -131,7 +131,7 @@
  "Tells the debugger to execute this one-line lldb command before any file 
"
  "provided on the command line has been loaded."},
 {LLDB_3_TO_5, false, "batch", 'b', no_argument, 0, eArgTypeNone,
- "Tells the debugger to running the commands from -s, -S, -o & -O, and "
+ "Tells the debugger to run the commands from -s, -S, -o & -O, and "
  "then quit.  However if any run command stopped due to a signal or crash, 
"
  "the debugger will return to the interactive prompt at the place of the "
  "crash."},


Index: lldb/trunk/tools/driver/Driver.cpp
===
--- lldb/trunk/tools/driver/Driver.cpp
+++ lldb/trunk/tools/driver/Driver.cpp
@@ -131,7 +131,7 @@
  "Tells the debugger to execute this one-line lldb command before any file "
  "provided on the command line has been loaded."},
 {LLDB_3_TO_5, false, "batch", 'b', no_argument, 0, eArgTypeNone,
- "Tells the debugger to running the commands from -s, -S, -o & -O, and "
+ "Tells the debugger to run the commands from -s, -S, -o & -O, and "
  "then quit.  However if any run command stopped due to a signal or crash, "
  "the debugger will return to the interactive prompt at the place of the "
  "crash."},
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r282123 - Fix typo in lldb --help

[Ed Maste via lldb-commits]

Author: emaste
Date: Wed Sep 21 18:30:36 2016
New Revision: 282123

URL: http://llvm.org/viewvc/llvm-project?rev=282123=rev
Log:
Fix typo in lldb --help

Patch by Yacine Belkadi

Differential Revision:  https://reviews.llvm.org/D12158

Modified:
lldb/trunk/tools/driver/Driver.cpp

Modified: lldb/trunk/tools/driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/driver/Driver.cpp?rev=282123=282122=282123=diff
==
--- lldb/trunk/tools/driver/Driver.cpp (original)
+++ lldb/trunk/tools/driver/Driver.cpp Wed Sep 21 18:30:36 2016
@@ -131,7 +131,7 @@ static OptionDefinition g_options[] = {
  "Tells the debugger to execute this one-line lldb command before any file 
"
  "provided on the command line has been loaded."},
 {LLDB_3_TO_5, false, "batch", 'b', no_argument, 0, eArgTypeNone,
- "Tells the debugger to running the commands from -s, -S, -o & -O, and "
+ "Tells the debugger to run the commands from -s, -S, -o & -O, and "
  "then quit.  However if any run command stopped due to a signal or crash, 
"
  "the debugger will return to the interactive prompt at the place of the "
  "crash."},


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

Re: [Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Greg Clayton via lldb-commits]

This it the perfect example of why not to use a StringRef since the string 
needs to be null terminated. Why did we change this? Now even if you call this 
function:

RegularExpression r(...);

r.Execute("...", ...)

You will need to duplicate the string on the heap just to execute this. Please 
revert this. Anything that requires null terminate is not a candidate for 
converting to StringRef.


> On Sep 21, 2016, at 10:13 AM, Zachary Turner via lldb-commits 
>  wrote:
> 
> Author: zturner
> Date: Wed Sep 21 12:13:51 2016
> New Revision: 282090
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
> Log:
> Fix failing regex tests.
> 
> r282079 converted the regular expression interface to accept
> and return StringRefs instead of char pointers.  In one case
> a null pointer check was converted to an empty string check,
> but this was an incorrect conversion because an empty string
> is a valid regular expression.  Removing this check should
> fix the test failures.
> 
> Modified:
>lldb/trunk/source/Core/RegularExpression.cpp
> 
> Modified: lldb/trunk/source/Core/RegularExpression.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
> ==
> --- lldb/trunk/source/Core/RegularExpression.cpp (original)
> +++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
> @@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
> //-
> bool RegularExpression::Execute(llvm::StringRef str, Match *match) const {
>   int err = 1;
> -  if (!str.empty() && m_comp_err == 0) {
> +  if (m_comp_err == 0) {
> // Argument to regexec must be null-terminated.
> std::string reg_str = str;
> if (match) {
> 
> 
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

Re: [Lldb-commits] [PATCH] D12966: Renderscript plugin support for aarch64

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r248003.


Repository:
  rL LLVM

https://reviews.llvm.org/D12966



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

Re: [Lldb-commits] [lldb] r282112 - Fix integer sign warning from r282105

[Ed Maste via lldb-commits]

On 21 September 2016 at 21:38, Adrian McCarthy  wrote:
> That fix doesn't look complete:

Thanks, I've applied your fix in r282119, and sorry for being hasty
with the original change.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D14920: [LLDB][MIPS] Provide actual number of watchpoints supported

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko added a comment.

Looks like patch was not committed. Please rebase with trunk and run 
Clang-format.


Repository:
  rL LLVM

https://reviews.llvm.org/D14920



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

Re: [Lldb-commits] [PATCH] D16234: Implement missing GoASTContext methods

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r257926.


Repository:
  rL LLVM

https://reviews.llvm.org/D16234



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

Re: [Lldb-commits] [PATCH] D16284: Fix Makefile build

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r259081.


https://reviews.llvm.org/D16284



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

Re: [Lldb-commits] [PATCH] D16695: Update LLDB Windows buildbot to use MSVC 2015 and Python 3.5

[Eugene Zelenko via lldb-commits]

Eugene.Zelenko added a subscriber: Eugene.Zelenko.
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.

Committed in r259246.


https://reviews.llvm.org/D16695



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

[Lldb-commits] [lldb] r282119 - Fix for loop sign fix in r282112 for column = 0

[Ed Maste via lldb-commits]

Author: emaste
Date: Wed Sep 21 17:36:51 2016
New Revision: 282119

URL: http://llvm.org/viewvc/llvm-project?rev=282119=rev
Log:
Fix for loop sign fix in r282112 for column = 0

Modified:
lldb/trunk/source/Core/SourceManager.cpp

Modified: lldb/trunk/source/Core/SourceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/SourceManager.cpp?rev=282119=282118=282119=diff
==
--- lldb/trunk/source/Core/SourceManager.cpp (original)
+++ lldb/trunk/source/Core/SourceManager.cpp Wed Sep 21 17:36:51 2016
@@ -172,7 +172,7 @@ size_t SourceManager::DisplaySourceLines
 m_last_file_sp->GetLine(line, src_line);
 return_value += s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
-for (size_t i = 0; i < column - 1 && i < src_line.length(); ++i)
+for (size_t i = 0; i + 1 < column && i < src_line.length(); ++i)
   return_value += s->PutChar(src_line[i] == '\t' ? '\t' : ' ');
 // Now add the caret.
 return_value += s->Printf("^\n");


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

[Lldb-commits] [lldb] r282117 - Fix an incorrect nullptr conversion.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Wed Sep 21 17:33:30 2016
New Revision: 282117

URL: http://llvm.org/viewvc/llvm-project?rev=282117=rev
Log:
Fix an incorrect nullptr conversion.

Modified:
lldb/trunk/source/Interpreter/Args.cpp

Modified: lldb/trunk/source/Interpreter/Args.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=282117=282116=282117=diff
==
--- lldb/trunk/source/Interpreter/Args.cpp (original)
+++ lldb/trunk/source/Interpreter/Args.cpp Wed Sep 21 17:33:30 2016
@@ -419,7 +419,7 @@ llvm::StringRef Args::ReplaceArgumentAtI
 m_args_quote_char[idx] = quote_char;
 return GetArgumentAtIndex(idx);
   }
-  return nullptr;
+  return llvm::StringRef();
 }
 
 void Args::DeleteArgumentAtIndex(size_t idx) {


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

Re: [Lldb-commits] [lldb] r282112 - Fix integer sign warning from r282105

[Adrian McCarthy via lldb-commits]

That fix doesn't look complete:

 for (size_t i = 0; i < column - 1 && i < src_line.length(); ++i)

`column` is an unsigned integral type, so doing subtraction from it can
lead to surprising results.  It's probably best to write it as:

  for (size_t i = 0; i  + 1 < column && i < src_line.length(); ++i)

That will more gracefully handle the case where column is 0.

Adrian.

On Wed, Sep 21, 2016 at 2:14 PM, Ed Maste via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: emaste
> Date: Wed Sep 21 16:14:31 2016
> New Revision: 282112
>
> URL: http://llvm.org/viewvc/llvm-project?rev=282112=rev
> Log:
> Fix integer sign warning from r282105
>
> Modified:
> lldb/trunk/source/Core/SourceManager.cpp
>
> Modified: lldb/trunk/source/Core/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/
> Core/SourceManager.cpp?rev=282112=282111=282112=diff
> 
> ==
> --- lldb/trunk/source/Core/SourceManager.cpp (original)
> +++ lldb/trunk/source/Core/SourceManager.cpp Wed Sep 21 16:14:31 2016
> @@ -172,7 +172,7 @@ size_t SourceManager::DisplaySourceLines
>  m_last_file_sp->GetLine(line, src_line);
>  return_value += s->Printf("\t");
>  // Insert a space for every non-tab character in the source line.
> -for (int i = 0; i < column - 1 && i < src_line.length(); ++i)
> +for (size_t i = 0; i < column - 1 && i < src_line.length(); ++i)
>return_value += s->PutChar(src_line[i] == '\t' ? '\t' : ' ');
>  // Now add the caret.
>  return_value += s->Printf("^\n");
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r282111 - Fix -Wcovered-switch-default warning in StackFrame.cpp

[Ed Maste via lldb-commits]

On 21 September 2016 at 17:08, Ed Maste via lldb-commits
 wrote:
> Author: emaste
> Date: Wed Sep 21 16:08:30 2016
> New Revision: 282111
>
> URL: http://llvm.org/viewvc/llvm-project?rev=282111=rev
> Log:
> Fix -Wcovered-switch-default warning in StackFrame.cpp
>
> The switch coveres all possible values. If a new one is added in the
> future the compiler will start warning, providing a notification that
> the switch needs updating.

For example, the warning shows a missing enum in ClangASTContext.cpp:

../tools/lldb/source/Symbol/ClangASTContext.cpp:4209:11: warning:
enumeration value 'ObjCTypeParam' not handled in switch [-Wswitch]
  switch (qual_type->getTypeClass()) {
  ^
../tools/lldb/source/Symbol/ClangASTContext.cpp:4922:11: warning:
enumeration value 'ObjCTypeParam' not handled in switch [-Wswitch]
  switch (qual_type->getTypeClass()) {
  ^
../tools/lldb/source/Symbol/ClangASTContext.cpp:5131:11: warning:
enumeration value 'ObjCTypeParam' not handled in switch [-Wswitch]
  switch (qual_type->getTypeClass()) {
  ^
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r282112 - Fix integer sign warning from r282105

[Ed Maste via lldb-commits]

Author: emaste
Date: Wed Sep 21 16:14:31 2016
New Revision: 282112

URL: http://llvm.org/viewvc/llvm-project?rev=282112=rev
Log:
Fix integer sign warning from r282105

Modified:
lldb/trunk/source/Core/SourceManager.cpp

Modified: lldb/trunk/source/Core/SourceManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/SourceManager.cpp?rev=282112=282111=282112=diff
==
--- lldb/trunk/source/Core/SourceManager.cpp (original)
+++ lldb/trunk/source/Core/SourceManager.cpp Wed Sep 21 16:14:31 2016
@@ -172,7 +172,7 @@ size_t SourceManager::DisplaySourceLines
 m_last_file_sp->GetLine(line, src_line);
 return_value += s->Printf("\t");
 // Insert a space for every non-tab character in the source line.
-for (int i = 0; i < column - 1 && i < src_line.length(); ++i)
+for (size_t i = 0; i < column - 1 && i < src_line.length(); ++i)
   return_value += s->PutChar(src_line[i] == '\t' ? '\t' : ' ');
 // Now add the caret.
 return_value += s->Printf("^\n");


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

[Lldb-commits] [lldb] r282111 - Fix -Wcovered-switch-default warning in StackFrame.cpp

[Ed Maste via lldb-commits]

Author: emaste
Date: Wed Sep 21 16:08:30 2016
New Revision: 282111

URL: http://llvm.org/viewvc/llvm-project?rev=282111=rev
Log:
Fix -Wcovered-switch-default warning in StackFrame.cpp

The switch coveres all possible values. If a new one is added in the
future the compiler will start warning, providing a notification that
the switch needs updating.

Modified:
lldb/trunk/source/Target/StackFrame.cpp

Modified: lldb/trunk/source/Target/StackFrame.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StackFrame.cpp?rev=282111=282110=282111=diff
==
--- lldb/trunk/source/Target/StackFrame.cpp (original)
+++ lldb/trunk/source/Target/StackFrame.cpp Wed Sep 21 16:08:30 2016
@@ -1278,8 +1278,6 @@ GetBaseExplainingValue(const Instruction
   return std::make_pair(nullptr, 0);
 }
   }
-  default:
-return std::make_pair(nullptr, 0);
   }
 }
 


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

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL282110: [CMake] Initial support for LLDB.framework (authored 
by cbieneman).

Changed prior to commit:
  https://reviews.llvm.org/D24749?vs=72097=72104#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24749

Files:
  lldb/trunk/CMakeLists.txt
  lldb/trunk/cmake/LLDBDependencies.cmake
  lldb/trunk/cmake/modules/AddLLDB.cmake
  lldb/trunk/cmake/modules/LLDBConfig.cmake
  lldb/trunk/scripts/CMakeLists.txt
  lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
  lldb/trunk/source/API/CMakeLists.txt
  lldb/trunk/tools/argdumper/CMakeLists.txt
  lldb/trunk/tools/darwin-debug/CMakeLists.txt
  lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt
  lldb/trunk/tools/lldb-server/CMakeLists.txt

Index: lldb/trunk/source/API/CMakeLists.txt
===
--- lldb/trunk/source/API/CMakeLists.txt
+++ lldb/trunk/source/API/CMakeLists.txt
@@ -6,6 +6,12 @@
 # for liblldb to link against
 include(${LLDB_PROJECT_ROOT}/cmake/LLDBDependencies.cmake)
 
+option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
+
+if (LLDB_BUILD_FRAMEWORK AND NOT APPLE)
+  message(FATAL_ERROR "LLDB.framework cannot be generated unless targeting Apple platforms.")
+endif()
+
 add_lldb_library(liblldb SHARED
   SBAddress.cpp
   SBAttachInfo.cpp
@@ -120,3 +126,11 @@
 endif()
 target_link_libraries(liblldb PRIVATE ${LLDB_SYSTEM_LIBS})
 
+if(LLDB_BUILD_FRAMEWORK)
+  set_target_properties(liblldb PROPERTIES
+OUTPUT_NAME LLDB
+FRAMEWORK On
+FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
+LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
+endif()
+
Index: lldb/trunk/tools/darwin-debug/CMakeLists.txt
===
--- lldb/trunk/tools/darwin-debug/CMakeLists.txt
+++ lldb/trunk/tools/darwin-debug/CMakeLists.txt
@@ -1,4 +1,4 @@
-add_lldb_executable(lldb-launcher
+add_lldb_executable(lldb-launcher INCLUDE_IN_FRAMEWORK
   darwin-debug.cpp
   )
 
Index: lldb/trunk/tools/argdumper/CMakeLists.txt
===
--- lldb/trunk/tools/argdumper/CMakeLists.txt
+++ lldb/trunk/tools/argdumper/CMakeLists.txt
@@ -1,4 +1,4 @@
-add_lldb_executable(lldb-argdumper
+add_lldb_executable(lldb-argdumper INCLUDE_IN_FRAMEWORK
   argdumper.cpp
   )
 
Index: lldb/trunk/tools/lldb-server/CMakeLists.txt
===
--- lldb/trunk/tools/lldb-server/CMakeLists.txt
+++ lldb/trunk/tools/lldb-server/CMakeLists.txt
@@ -25,7 +25,7 @@
 
 include(../../cmake/LLDBDependencies.cmake)
 
-add_lldb_executable(lldb-server
+add_lldb_executable(lldb-server INCLUDE_IN_FRAMEWORK
 Acceptor.cpp
 lldb-gdbserver.cpp
 lldb-platform.cpp
Index: lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt
===
--- lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt
+++ lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt
@@ -36,7 +36,7 @@
   lldbDebugserverMacOSX_DarwinLog
   )
 
-add_lldb_executable(debugserver
+add_lldb_executable(debugserver INCLUDE_IN_FRAMEWORK
   HasAVX.s
   CFBundle.cpp
   CFString.cpp
@@ -69,23 +69,14 @@
 OUTPUT_STRIP_TRAILING_WHITESPACE
 OUTPUT_VARIABLE CODESIGN_ALLOCATE
 )
-  # Older cmake versions don't support "-E env".
-  if (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} LESS 3.2)
-add_custom_command(TARGET debugserver
-  POST_BUILD
-  # Note: --entitlements option removed, as it causes errors when debugging.
-  # was: COMMAND CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE} codesign --entitlements ${CMAKE_CURRENT_SOURCE_DIR}/../debugserver-entitlements.plist --force --sign ${LLDB_CODESIGN_IDENTITY} debugserver
-  COMMAND CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE} codesign --force --sign ${LLDB_CODESIGN_IDENTITY} debugserver
-  WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
-)
-  else()
-add_custom_command(TARGET debugserver
-  POST_BUILD
-  # Note: --entitlements option removed (see comment above).
-  COMMAND ${CMAKE_COMMAND} -E env CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE} codesign --force --sign ${LLDB_CODESIGN_IDENTITY} debugserver
-  WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
-)
-  endif()
+  add_custom_command(TARGET debugserver
+POST_BUILD
+# Note: --entitlements option removed (see comment above).
+COMMAND ${CMAKE_COMMAND} -E env CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE}
+codesign --force --sign ${LLDB_CODESIGN_IDENTITY}
+$
+WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
+  )
 endif()
 
 install(TARGETS debugserver
Index: lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
===
--- lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
+++ lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
@@ -383,7 +383,7 @@

[Lldb-commits] [lldb] r282110 - [CMake] Initial support for LLDB.framework

[Chris Bieneman via lldb-commits]

Author: cbieneman
Date: Wed Sep 21 16:02:16 2016
New Revision: 282110

URL: http://llvm.org/viewvc/llvm-project?rev=282110=rev
Log:
[CMake] Initial support for LLDB.framework

Summary:
This patch adds a CMake option LLDB_BUILD_FRAMEWORK, which builds libLLDB as a 
macOS framework instead of as a *nix shared library.

With this patch any LLDB executable that has the INCLUDE_IN_FRAMEWORK option 
set will be built into the Framework's resources directory, and a symlink to 
the exeuctable will be placed under the build directory's bin folder. Creating 
the symlinks allows users to run commands from the build directory without 
altering the workflow.

The framework generated by this patch passes the LLDB test suite, but has not 
been tested beyond that. It is not expected to be fully ready to ship, but it 
is a first step.

With this patch binaries that are placed inside the framework aren't being 
properly installed. Fixing that would increase the patch size significantly, so 
I'd like to do that in a follow-up.

Reviewers: zturner, tfiala

Subscribers: beanz, lldb-commits, mgorny

Differential Revision: https://reviews.llvm.org/D24749

Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/cmake/LLDBDependencies.cmake
lldb/trunk/cmake/modules/AddLLDB.cmake
lldb/trunk/cmake/modules/LLDBConfig.cmake
lldb/trunk/scripts/CMakeLists.txt
lldb/trunk/scripts/Python/finishSwigPythonLLDB.py
lldb/trunk/source/API/CMakeLists.txt
lldb/trunk/tools/argdumper/CMakeLists.txt
lldb/trunk/tools/darwin-debug/CMakeLists.txt
lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt
lldb/trunk/tools/lldb-server/CMakeLists.txt

Modified: lldb/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=282110=282109=282110=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Wed Sep 21 16:02:16 2016
@@ -20,6 +20,16 @@ endif()
 # add_subdirectory(include)
 add_subdirectory(docs)
 if (NOT LLDB_DISABLE_PYTHON)
+  set(LLDB_PYTHON_TARGET_DIR ${LLDB_BINARY_DIR}/scripts)
+  if(LLDB_BUILD_FRAMEWORK)
+set(LLDB_PYTHON_TARGET_DIR
+  ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
+  else()
+# Don't set -m when building the framework.
+set(FINISH_EXTRA_ARGS "-m")
+  endif()
+  set(LLDB_WRAP_PYTHON ${LLDB_PYTHON_TARGET_DIR}/LLDBWrapPython.cpp)
+
   add_subdirectory(scripts)
 endif ()
 add_subdirectory(source)
@@ -31,7 +41,13 @@ add_subdirectory(lit)
 if (NOT LLDB_DISABLE_PYTHON)
 # Add a Post-Build Event to copy over Python files and create the symlink 
to liblldb.so for the Python API(hardlink on Windows)
 add_custom_target( finish_swig ALL
-COMMAND ${PYTHON_EXECUTABLE} 
${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py 
"--srcRoot=${LLDB_SOURCE_DIR}" 
"--targetDir=${CMAKE_CURRENT_BINARY_DIR}/scripts" 
"--cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}/scripts" 
"--prefix=${CMAKE_BINARY_DIR}" "--cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}" 
"--lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}" -m
+COMMAND ${PYTHON_EXECUTABLE} 
${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
+"--srcRoot=${LLDB_SOURCE_DIR}"
+"--targetDir=${LLDB_PYTHON_TARGET_DIR}"
+"--cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}/scripts"
+"--prefix=${CMAKE_BINARY_DIR}"
+"--cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}"
+"--lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}" ${FINISH_EXTRA_ARGS}
 DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
 DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/scripts/lldb.py
 COMMENT "Python script sym-linking LLDB Python API")

Modified: lldb/trunk/cmake/LLDBDependencies.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/LLDBDependencies.cmake?rev=282110=282109=282110=diff
==
--- lldb/trunk/cmake/LLDBDependencies.cmake (original)
+++ lldb/trunk/cmake/LLDBDependencies.cmake Wed Sep 21 16:02:16 2016
@@ -209,8 +209,6 @@ set(LLVM_LINK_COMPONENTS
   )
 
 if ( NOT LLDB_DISABLE_PYTHON )
-  set(LLDB_WRAP_PYTHON ${LLDB_BINARY_DIR}/scripts/LLDBWrapPython.cpp)
-
   set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES GENERATED 1)
   if (CLANG_CL)
 set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES COMPILE_FLAGS 
-Wno-unused-function)

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=282110=282109=282110=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Wed Sep 21 16:02:16 2016
@@ -72,10 +72,14 @@ macro(add_lldb_library name)
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "liblldb")
   if (PARAM_SHARED)
+set(out_dir 

[Lldb-commits] [lldb] r282105 - add stop column highlighting support

[Todd Fiala via lldb-commits]

Author: tfiala
Date: Wed Sep 21 15:13:14 2016
New Revision: 282105

URL: http://llvm.org/viewvc/llvm-project?rev=282105=rev
Log:
add stop column highlighting support

This change introduces optional marking of the column within a source
line where a thread is stopped.  This marking will show up when the
source code for a thread stop is displayed, when the debug info
knows the column information, and if the optional column marking is
enabled.

There are two separate methods for handling the marking of the stop
column:

* via ANSI terminal codes, which are added inline to the source line
  display.  The default ANSI mark-up is to underline the column.

* via a pure text-based caret that is added in the appropriate column
  in a newly-inserted blank line underneath the source line in
  question.

There are some new options that control how this all works.

* settings set stop-show-column

  This takes one of 4 values:

  * ansi-or-caret: use the ANSI terminal code mechanism if LLDB
is running with color enabled; if not, use the caret-based,
pure text method (see the "caret" mode below).

  * ansi: only use the ANSI terminal code mechanism to highlight
the stop line.  If LLDB is running with color disabled, no
stop column marking will occur.

  * caret: only use the pure text caret method, which introduces
a newly-inserted line underneath the current line, where
the only character in the new line is a caret that highlights
the stop column in question.

  * none: no stop column marking will be attempted.

* settings set stop-show-column-ansi-prefix

  This is a text format that indicates the ANSI formatting
  code to insert into the stream immediately preceding the
  column where the stop column character will be marked up.
  It defaults to ${ansi.underline}; however, it can contain
  any valid LLDB format codes, e.g.

  ${ansi.fg.red}${ansi.bold}${ansi.underline}

* settings set stop-show-column-ansi-suffix

  This is the text format that specifies the ANSI terminal
  codes to end the markup that was started with the prefix
  described above.  It defaults to: ${ansi.normal}.  This
  should be sufficient for the common cases.

Significant leg-work was done by Adrian Prantl.  (Thanks, Adrian!)

differential review: https://reviews.llvm.org/D20835

reviewers: clayborg, jingham

Modified:
lldb/trunk/include/lldb/API/SBSourceManager.h
lldb/trunk/include/lldb/Core/Debugger.h
lldb/trunk/include/lldb/Core/Disassembler.h
lldb/trunk/include/lldb/Core/SourceManager.h
lldb/trunk/include/lldb/lldb-enumerations.h

lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/packages/Python/lldbsuite/test/settings/TestSettings.py

lldb/trunk/packages/Python/lldbsuite/test/source-manager/TestSourceManager.py
lldb/trunk/scripts/interface/SBSourceManager.i
lldb/trunk/source/API/SBSourceManager.cpp
lldb/trunk/source/Commands/CommandObjectSource.cpp
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Core/Disassembler.cpp
lldb/trunk/source/Core/SourceManager.cpp
lldb/trunk/source/Target/StackFrame.cpp

Modified: lldb/trunk/include/lldb/API/SBSourceManager.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/API/SBSourceManager.h?rev=282105=282104=282105=diff
==
--- lldb/trunk/include/lldb/API/SBSourceManager.h (original)
+++ lldb/trunk/include/lldb/API/SBSourceManager.h Wed Sep 21 15:13:14 2016
@@ -30,6 +30,11 @@ public:
   const lldb::SBFileSpec , uint32_t line, uint32_t context_before,
   uint32_t context_after, const char *current_line_cstr, lldb::SBStream 
);
 
+  size_t DisplaySourceLinesWithLineNumbersAndColumn(
+  const lldb::SBFileSpec , uint32_t line, uint32_t column,
+  uint32_t context_before, uint32_t context_after,
+  const char *current_line_cstr, lldb::SBStream );
+
 protected:
   friend class SBCommandInterpreter;
   friend class SBDebugger;

Modified: lldb/trunk/include/lldb/Core/Debugger.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/Debugger.h?rev=282105=282104=282105=diff
==
--- lldb/trunk/include/lldb/Core/Debugger.h (original)
+++ lldb/trunk/include/lldb/Core/Debugger.h Wed Sep 21 15:13:14 2016
@@ -238,6 +238,12 @@ public:
 
   bool SetUseColor(bool use_color);
 
+  lldb::StopShowColumn GetStopShowColumn() const;
+
+  const FormatEntity::Entry *GetStopShowColumnAnsiPrefix() const;
+
+  const FormatEntity::Entry *GetStopShowColumnAnsiSuffix() const;
+
   uint32_t GetStopSourceLineCount(bool before) const;
 
   StopDisassemblyType GetStopDisassemblyDisplay() const;

Modified: lldb/trunk/include/lldb/Core/Disassembler.h
URL: 

[Lldb-commits] [lldb] r282103 - Probably should add the breakpoint names test directory as well...

[Jim Ingham via lldb-commits]

Author: jingham
Date: Wed Sep 21 14:21:38 2016
New Revision: 282103

URL: http://llvm.org/viewvc/llvm-project?rev=282103=rev
Log:
Probably should add the breakpoint names test directory as well...

Added:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/Makefile

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py

lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/main.c

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/Makefile?rev=282103=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/Makefile
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/Makefile
 Wed Sep 21 14:21:38 2016
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules

Added: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py?rev=282103=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
 (added)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
 Wed Sep 21 14:21:38 2016
@@ -0,0 +1,141 @@
+"""
+Test breakpoint names.
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class BreakpointNames(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@add_test_categories(['pyapi'])
+def test_setting_names(self):
+"""Use Python APIs to test that we can set breakpoint names."""
+self.build()
+self.setup_target()
+self.do_check_names()
+
+def test_illegal_names(self):
+"""Use Python APIs to test that we don't allow illegal names."""
+self.build()
+self.setup_target()
+self.do_check_illegal_names()
+
+def test_using_names(self):
+"""Use Python APIs to test that operations on names works correctly."""
+self.build()
+self.setup_target()
+self.do_check_using_names()
+
+def setup_target(self):
+exe = os.path.join(os.getcwd(), "a.out")
+
+# Create a targets we are making breakpoint in and copying to:
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+self.main_file_spec = lldb.SBFileSpec(os.path.join(os.getcwd(), 
"main.c"))
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+def do_check_names(self):
+"""Use Python APIs to check that we can set & retrieve breakpoint 
names"""
+bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10)
+bkpt_name = "ABreakpoint"
+other_bkpt_name = "_AnotherBreakpoint"
+
+# Add a name and make sure we match it:
+success = bkpt.AddName(bkpt_name)
+self.assertTrue(success, "We couldn't add a legal name to a 
breakpoint.")
+
+matches = bkpt.MatchesName(bkpt_name)
+self.assertTrue(matches, "We didn't match the name we just set")
+
+# Make sure we don't match irrelevant names:
+matches = bkpt.MatchesName("NotABreakpoint")
+self.assertTrue(not matches, "We matched a name we didn't set.")
+
+# Add another name, make sure that works too:
+bkpt.AddName(other_bkpt_name)
+
+matches = bkpt.MatchesName(bkpt_name)
+self.assertTrue(matches, "Adding a name means we didn't match the name 
we just set")
+
+# Remove the name and make sure we no longer match it:
+bkpt.RemoveName(bkpt_name)
+matches = bkpt.MatchesName(bkpt_name)
+self.assertTrue(not matches,"We still match a name after removing it.")
+
+# Make sure the name list has the remaining name:
+name_list = lldb.SBStringList()
+bkpt.GetNames(name_list)
+num_names = name_list.GetSize()
+self.assertTrue(num_names == 1, "Name list has %d items, expected 
1."%(num_names))
+
+name = name_list.GetStringAtIndex(0)
+self.assertTrue(name == other_bkpt_name, "Remaining name was: %s 
expected %s."%(name, 

Re: [Lldb-commits] [PATCH] D24749: [CMake] Initial support for LLDB.framework

[Pavel Labath via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D24749#548778, @tfiala wrote:

> I'd say the install rpath change is probably worth doing on the first round.  
> I think the top-level concept for frameworks is interesting but would be fine 
> to come in as another change.
>
> I'd like to start being able to use this.


sgtm


https://reviews.llvm.org/D24749



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

[Lldb-commits] LLVM buildmaster will be restarted tonight

[Galina Kistanova via lldb-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted after 6 PM Pacific time
today.

Thanks

Galina
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.

[Greg Clayton via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Looks good.


https://reviews.llvm.org/D20835



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

[Lldb-commits] [lldb] r282090 - Fix failing regex tests.

[Zachary Turner via lldb-commits]

Author: zturner
Date: Wed Sep 21 12:13:51 2016
New Revision: 282090

URL: http://llvm.org/viewvc/llvm-project?rev=282090=rev
Log:
Fix failing regex tests.

r282079 converted the regular expression interface to accept
and return StringRefs instead of char pointers.  In one case
a null pointer check was converted to an empty string check,
but this was an incorrect conversion because an empty string
is a valid regular expression.  Removing this check should
fix the test failures.

Modified:
lldb/trunk/source/Core/RegularExpression.cpp

Modified: lldb/trunk/source/Core/RegularExpression.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/RegularExpression.cpp?rev=282090=282089=282090=diff
==
--- lldb/trunk/source/Core/RegularExpression.cpp (original)
+++ lldb/trunk/source/Core/RegularExpression.cpp Wed Sep 21 12:13:51 2016
@@ -102,7 +102,7 @@ bool RegularExpression::Compile(llvm::St
 //-
 bool RegularExpression::Execute(llvm::StringRef str, Match *match) const {
   int err = 1;
-  if (!str.empty() && m_comp_err == 0) {
+  if (m_comp_err == 0) {
 // Argument to regexec must be null-terminated.
 std::string reg_str = str;
 if (match) {


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

Re: [Lldb-commits] [PATCH] D24124: [LLDB][MIPS] Fix register read/write for 32 bit big endian system

[Greg Clayton via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

Much better.


https://reviews.llvm.org/D24124



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

Re: [Lldb-commits] [PATCH] D24603: [LLDB][MIPS] fix Floating point register read/write for big endian

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So it seems like this can be fixed by doing a few things:
1 - just set the RegisterInfo.offset to the actual offset in the buffer and 
don't use the offset as the ptrace key used to grab the register
2 - modify the RegisterInfo.kinds[eRegisterKindProcessPlugin] to be the offset 
to you to use for ptrace

There should be no need what so ever to do manual bit twiddling with src and 
dst.


https://reviews.llvm.org/D24603



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

Re: [Lldb-commits] [PATCH] D20835: Mark the current column when displaying the context of the current breakpoint on the terminal.

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few naming requests.



Comment at: source/Core/Debugger.cpp:151
@@ +150,3 @@
+   "\"caret-only\" mode was selected."},
+  {eStopShowColumnAnsiOnly, "ansi-only",
+   "Highlight the stop column with ANSI terminal codes when running LLDB with "

Make this "ansi" instead of "ansi-only"?


Comment at: source/Core/Debugger.cpp:154
@@ +153,3 @@
+   "color/ANSI enabled."},
+  {eStopShowColumnCaretOnly, "caret-only",
+   "Highlight the stop column with a caret character (^) underneath the stop "

Make this "caret" instead?


Comment at: source/Core/Debugger.cpp:199
@@ +198,3 @@
+ "mark the current position when displaying a stopped context."},
+{"stop-show-column-decorator-pre", OptionValue::eTypeFormatEntity, true, 0,
+ DEFAULT_STOP_COLUMN_FORMAT_PRE, nullptr,

Make this "stop-show-column-ansi-prefix"?


Comment at: source/Core/Debugger.cpp:204
@@ +203,3 @@
+ "immediately before the column to be marked."},
+{"stop-show-column-decorator-post", OptionValue::eTypeFormatEntity, true, 
0,
+ DEFAULT_STOP_COLUMN_FORMAT_POST, nullptr,

Make this "stop-show-column-ansi-suffix"?


https://reviews.llvm.org/D20835



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

Re: [Lldb-commits] [PATCH] D24284: [lldb] Read modules from memory when a local copy is not available

[Zachary Turner via lldb-commits]

Unfortunately this is still crashing for me in every single test.  Can I
ask how you are running the test suite?

I am doing the following from a Windows 10 machine using Visual Studio 15:

cmake -G Ninja -DLLDB_TEST_DEBUG_TEST_CRASHES=1 -DPYTHON_HOME=C:\Python35
-DLLDB_TEST_COMPILER=d:\src\llvmbuild\ninja_release\bin\clang.exe ..\..\llvm
ninja check-lldb

Can you try this same set of steps to see if you can reproduce?

On Tue, Sep 20, 2016 at 3:44 PM Walter  wrote:

> I rebased it
>
> 2016-09-20 15:44 GMT-07:00 walter erquinigo :
>
> wallace updated this revision to Diff 71995.
> wallace added a comment.
>
> rebase
>
>
> https://reviews.llvm.org/D24284
>
> Files:
>   source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
>   source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
>
>
>
>
> --
> Walter Erquínigo Pezo
>
> Every problem has a simple, fast and wrong solution.
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

valentinagiusti added a comment.

ok, I will keep it in mind for some further refactoring, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D24764



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

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL282072: Refactor NativeRegisterContextLinux_x86_64 code. 
(authored by valentinagiusti).

Changed prior to commit:
  https://reviews.llvm.org/D24764?vs=71949=72036#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24764

Files:
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
@@ -117,18 +117,12 @@
   uint32_t m_fctrl_offset_in_userarea;
 
   // Private member methods.
-  bool HasFXSAVE() const;
-
-  bool HasXSAVE() const;
-
   bool IsCPUFeatureAvailable(RegSet feature_code) const;
 
   bool IsRegisterSetAvailable(uint32_t set_index) const;
 
   bool IsGPR(uint32_t reg_index) const;
 
-  XStateType GetXStateType() const;
-
   bool IsFPR(uint32_t reg_index) const;
 
   bool CopyXSTATEtoYMM(uint32_t reg_index, lldb::ByteOrder byte_order);
Index: lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
+++ lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
@@ -20,8 +20,6 @@
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 
-#include 
-
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
 
@@ -664,9 +662,9 @@
 
   ::memcpy(dst, _gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
   dst += GetRegisterInfoInterface().GetGPRSize();
-  if (GetXStateType() == XStateType::FXSAVE)
+  if (m_xstate_type == XStateType::FXSAVE)
 ::memcpy(dst, _fpr.xstate.fxsave, sizeof(m_fpr.xstate.fxsave));
-  else if (GetXStateType() == XStateType::XSAVE) {
+  else if (m_xstate_type == XStateType::XSAVE) {
 lldb::ByteOrder byte_order = GetByteOrder();
 
 if (IsCPUFeatureAvailable(RegSet::avx)) {
@@ -756,16 +754,16 @@
 return error;
 
   src += GetRegisterInfoInterface().GetGPRSize();
-  if (GetXStateType() == XStateType::FXSAVE)
+  if (m_xstate_type == XStateType::FXSAVE)
 ::memcpy(_fpr.xstate.fxsave, src, sizeof(m_fpr.xstate.fxsave));
-  else if (GetXStateType() == XStateType::XSAVE)
+  else if (m_xstate_type == XStateType::XSAVE)
 ::memcpy(_fpr.xstate.xsave, src, sizeof(m_fpr.xstate.xsave));
 
   error = WriteFPR();
   if (error.Fail())
 return error;
 
-  if (GetXStateType() == XStateType::XSAVE) {
+  if (m_xstate_type == XStateType::XSAVE) {
 lldb::ByteOrder byte_order = GetByteOrder();
 
 if (IsCPUFeatureAvailable(RegSet::avx)) {
@@ -801,58 +799,28 @@
   return error;
 }
 
-bool NativeRegisterContextLinux_x86_64::HasFXSAVE() const {
-  unsigned int rax, rbx, rcx, rdx;
-
-  // Check if FXSAVE is enabled.
-  if (!__get_cpuid(1, , , , ))
-return false;
-  if ((rdx & bit_FXSAVE) == bit_FXSAVE) {
-m_xstate_type = XStateType::FXSAVE;
-if (const_cast(this)->ReadFPR().Fail())
-  return false;
-return true;
-  }
-  return false;
-}
-
-bool NativeRegisterContextLinux_x86_64::HasXSAVE() const {
-  unsigned int rax, rbx, rcx, rdx;
-
-  // Check if XSAVE is enabled.
-  if (!__get_cpuid(1, , , , ))
-return false;
-  if ((rcx & bit_OSXSAVE) == bit_OSXSAVE) {
-m_xstate_type = XStateType::XSAVE;
+bool NativeRegisterContextLinux_x86_64::IsCPUFeatureAvailable(
+RegSet feature_code) const {
+  if (m_xstate_type == XStateType::Invalid) {
 if (const_cast(this)->ReadFPR().Fail())
   return false;
-return true;
   }
-  return false;
-}
-
-bool NativeRegisterContextLinux_x86_64::IsCPUFeatureAvailable(
-RegSet feature_code) const {
-  unsigned int rax, rbx, rcx, rdx;
-
-  // Check if XSAVE is enabled.
-  if (!HasXSAVE())
-return false;
-
-  __get_cpuid(1, , , , );
   switch (feature_code) {
-  case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by reading in the XCR0 area of XSAVE.
-if (((rcx & bit_AVX) != 0) && ((m_fpr.xstate.xsave.i387.xcr0 & mask_XSTATE_AVX) == mask_XSTATE_AVX))
+  case RegSet::gpr:
+  case RegSet::fpu:
+return true;
+  case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by
+// reading in the XCR0 area of XSAVE.
+if ((m_fpr.xstate.xsave.i387.xcr0 & mask_XSTATE_AVX) == mask_XSTATE_AVX)
   return true;
-  case RegSet::mpx: // Check if CPU has MPX and if there is kernel support, by reading in the XCR0 area of XSAVE.
-if (__get_cpuid_max(0, NULL) > 7) {
-  __cpuid_count(7, 0, rax, rbx, rcx, rdx);
-  if (((rbx & bit_MPX) != 0) && 

[Lldb-commits] [lldb] r282072 - Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

Author: valentinagiusti
Date: Wed Sep 21 08:33:01 2016
New Revision: 282072

URL: http://llvm.org/viewvc/llvm-project?rev=282072=rev
Log:
Refactor NativeRegisterContextLinux_x86_64 code.

This patch refactors the way the XState type is checked and, in order to
simplify the code, it removes the usage of the 'cpuid' instruction: just 
checking
if the ptrace calls done throuhg ReadFPR is enough to verify both if there is
HW support and if there is kernel support. Also the XCR0 bits are enough to 
check if
there is both HW and kernel support for AVX and MPX.

Differential Revision: https://reviews.llvm.org/D24764

Modified:

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp?rev=282072=282071=282072=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
Wed Sep 21 08:33:01 2016
@@ -20,8 +20,6 @@
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 
-#include 
-
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
 
@@ -664,9 +662,9 @@ Error NativeRegisterContextLinux_x86_64:
 
   ::memcpy(dst, _gpr_x86_64, GetRegisterInfoInterface().GetGPRSize());
   dst += GetRegisterInfoInterface().GetGPRSize();
-  if (GetXStateType() == XStateType::FXSAVE)
+  if (m_xstate_type == XStateType::FXSAVE)
 ::memcpy(dst, _fpr.xstate.fxsave, sizeof(m_fpr.xstate.fxsave));
-  else if (GetXStateType() == XStateType::XSAVE) {
+  else if (m_xstate_type == XStateType::XSAVE) {
 lldb::ByteOrder byte_order = GetByteOrder();
 
 if (IsCPUFeatureAvailable(RegSet::avx)) {
@@ -756,16 +754,16 @@ Error NativeRegisterContextLinux_x86_64:
 return error;
 
   src += GetRegisterInfoInterface().GetGPRSize();
-  if (GetXStateType() == XStateType::FXSAVE)
+  if (m_xstate_type == XStateType::FXSAVE)
 ::memcpy(_fpr.xstate.fxsave, src, sizeof(m_fpr.xstate.fxsave));
-  else if (GetXStateType() == XStateType::XSAVE)
+  else if (m_xstate_type == XStateType::XSAVE)
 ::memcpy(_fpr.xstate.xsave, src, sizeof(m_fpr.xstate.xsave));
 
   error = WriteFPR();
   if (error.Fail())
 return error;
 
-  if (GetXStateType() == XStateType::XSAVE) {
+  if (m_xstate_type == XStateType::XSAVE) {
 lldb::ByteOrder byte_order = GetByteOrder();
 
 if (IsCPUFeatureAvailable(RegSet::avx)) {
@@ -801,58 +799,28 @@ Error NativeRegisterContextLinux_x86_64:
   return error;
 }
 
-bool NativeRegisterContextLinux_x86_64::HasFXSAVE() const {
-  unsigned int rax, rbx, rcx, rdx;
-
-  // Check if FXSAVE is enabled.
-  if (!__get_cpuid(1, , , , ))
-return false;
-  if ((rdx & bit_FXSAVE) == bit_FXSAVE) {
-m_xstate_type = XStateType::FXSAVE;
-if (const_cast(this)->ReadFPR().Fail())
-  return false;
-return true;
-  }
-  return false;
-}
-
-bool NativeRegisterContextLinux_x86_64::HasXSAVE() const {
-  unsigned int rax, rbx, rcx, rdx;
-
-  // Check if XSAVE is enabled.
-  if (!__get_cpuid(1, , , , ))
-return false;
-  if ((rcx & bit_OSXSAVE) == bit_OSXSAVE) {
-m_xstate_type = XStateType::XSAVE;
+bool NativeRegisterContextLinux_x86_64::IsCPUFeatureAvailable(
+RegSet feature_code) const {
+  if (m_xstate_type == XStateType::Invalid) {
 if (const_cast(this)->ReadFPR().Fail())
   return false;
-return true;
   }
-  return false;
-}
-
-bool NativeRegisterContextLinux_x86_64::IsCPUFeatureAvailable(
-RegSet feature_code) const {
-  unsigned int rax, rbx, rcx, rdx;
-
-  // Check if XSAVE is enabled.
-  if (!HasXSAVE())
-return false;
-
-  __get_cpuid(1, , , , );
   switch (feature_code) {
-  case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by 
reading in the XCR0 area of XSAVE.
-if (((rcx & bit_AVX) != 0) && ((m_fpr.xstate.xsave.i387.xcr0 & 
mask_XSTATE_AVX) == mask_XSTATE_AVX))
+  case RegSet::gpr:
+  case RegSet::fpu:
+return true;
+  case RegSet::avx: // Check if CPU has AVX and if there is kernel support, by
+// reading in the XCR0 area of XSAVE.
+if ((m_fpr.xstate.xsave.i387.xcr0 & mask_XSTATE_AVX) == mask_XSTATE_AVX)
   return true;
-  case RegSet::mpx: // Check if CPU has MPX and if there is kernel support, by 
reading in the XCR0 area of XSAVE.
-if (__get_cpuid_max(0, NULL) > 7) {
-  __cpuid_count(7, 0, rax, rbx, rcx, rdx);
-  if (((rbx & bit_MPX) != 0) && ((m_fpr.xstate.xsave.i387.xcr0 & 
mask_XSTATE_MPX) == mask_XSTATE_MPX))
-return true;
-}
-  default:
-return false;
+ break;
+  case RegSet::mpx: 

Re: [Lldb-commits] [PATCH] D24711: [lldb-mi] Fix implementation for a few mi commands

[Hafiz Abid Qadeer via lldb-commits]

abidh added a comment.

Changes looks mostly Ok to me apart from some comments. Please address them and 
add testcases as mentioned by ilia. Also try to do one review for one fix. This 
review is for 3 fixes. When the changes are approved, please commit them in 3 
separate commits (one per fix).



Comment at: tools/lldb-mi/MICmdCmdGdbSet.cpp:421
@@ +420,3 @@
+  lldb::SBDebugger  = m_rLLDBDebugSessionInfo.GetDebugger();
+  lldb::SBDebugger::SetInternalVariable("target.x86-disassembly-flavor", 
rStrValDisasmFlavor.c_str(), rDbgr.GetInstanceName());
+

You are not checking if SetInternalVariable returned an error which can happen 
in this case if the flavor name was not one of "intel", "att" or "default".


Comment at: tools/lldb-mi/MICmdCmdMiscellanous.cpp:515
@@ -507,2 +514,3 @@
+  return MIstatus::failure;
   }
 

It is not really an OutofBand record but rather an output of the command. Why 
not simple add prepend an ~


Comment at: tools/lldb-mi/MICmdCmdTarget.cpp:126
@@ -125,2 +125,3 @@
   lldb::SBStream errMsg;
+  error.GetDescription(errMsg);
   if (!process.IsValid()) {

This does not seem related to any bug fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D24711



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

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Pavel Labath via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I'm sorry, I did not notice that. Go ahead with this patch in that case. It  
looks great apart from this eexisting problem. If you're going to do further 
cleanups here,, I would recommend looking at the cast problem as well.


https://reviews.llvm.org/D24764



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

Re: [Lldb-commits] [PATCH] D24124: [LLDB][MIPS] Fix register read/write for 32 bit big endian system

[Pavel Labath via lldb-commits]

labath added a comment.

It definitely looks cleaner than the original version. I'm fine with this if 
@clayborg is.


https://reviews.llvm.org/D24124



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

Re: [Lldb-commits] [PATCH] D24603: [LLDB][MIPS] fix Floating point register read/write for big endian

[Pavel Labath via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Ok, lets leave that as-is then.. the issue seem s pretty contained for now.


https://reviews.llvm.org/D24603



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

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

valentinagiusti added a comment.

Thechnically it's not correct that I am introducing this issue, because the old 
code already used a cast. It was done in the old and now not existing method 
"GetFPRType()", long before I introduced the MPX changes, and then I later 
moved it into HasXSave()/HasXSave() and now with this current refactoring patch 
I am moving it into IsCPUFeatureAvailable().


https://reviews.llvm.org/D24764



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

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Pavel Labath via lldb-commits]

labath added a comment.

So, the thing is that you already are changing the interface. The difference is 
that you are using the const cast to hide that fact, which is why I dont 
approve of it.

Also, since this is not an existing problem but rather something you are 
introducing in this change, I dont thinl it's appropriate to do it as a 
follow-up,  but rather as a prep work for this  change.

One possibility I would consider is to do any work you need to do before this 
function even executes (e.g. in the constructor - the result of 
IsCpuFeatureAvailable cannot change during the lifetime of the process anyway, 
right?). I am OOO today so I cannot really tell how feasible that ideaIis. I 
can look at It tomorrow.


https://reviews.llvm.org/D24764



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

Re: [Lldb-commits] [PATCH] D24124: [LLDB][MIPS] Fix register read/write for 32 bit big endian system

[Nitesh Jain via lldb-commits]

nitesh.jain added a comment.

In https://reviews.llvm.org/D24124#543823, @clayborg wrote:

> A few things about a the RegisterContext class in case it would change and 
> thing that you are submitting here. The entire register context defines a 
> buffer of bytes that can contain all register values. Each RegisterInfo 
> contains the offset for the value of this register in this buffer. This 
> buffer is said to have a specific byte order (big, little, etc). When a 
> register is read, it should place its bytes into the RegisterContext's buffer 
> of bytes and mark itself as being valid in the register context. Some 
> platforms read multiple registers at a time (they don't have a "read one 
> register value", they just have "read all GPR registers") and lets say you 
> are reading one GPR, and this causes all GPR values to be read, then all 
> bytes from all GPR values will be copied into the register context data 
> buffer and all GPRs should be marked as valid. So to get a RegisterValue for 
> a 32 bit register, we normally will just ask the RegisterInfo for the offset 
> of the register, and then extract the bytes from the buffer using a 
> DataExtractor object. If you have a 64 bit register whose value also contains 
> a 32 bit pseudo register (like rax contains eax on x86), then you should have 
> a RegisterInfo defined for "rax" that says its offset is N, and for a big 
> endian system, you would say that the register offset for "eax" is N + 4. 
> Extracting the value simply becomes extracting the bytes from the buffer 
> without the need for any tricks. After reading all of this, do you still 
> believe you have the right fix in here? It doesn't seem like you ever should 
> need to use DataExtractor::CopyByteOrderedData???


The issue was in RegisterValue::GetUInt64 function returning incorrect value 
for register of size 4/2/1 byte on 32 bit big endian system. We have modify it 
to return value based on register size which will fix the register read/write 
problem on 32 bit big endian system.


https://reviews.llvm.org/D24124



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

Re: [Lldb-commits] [PATCH] D24124: [LLDB][MIPS] Fix register read/write for 32 bit big endian system

[Nitesh Jain via lldb-commits]

nitesh.jain updated the summary for this revision.
nitesh.jain updated this revision to Diff 72024.

https://reviews.llvm.org/D24124

Files:
  source/Core/RegisterValue.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp

Index: source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -169,7 +169,7 @@
 
   if (error.Success())
 // First cast to an unsigned of the same size to avoid sign extension.
-value.SetUInt64(static_cast(data));
+value.SetUInt(static_cast(data), size);
 
   if (log)
 log->Printf("NativeRegisterContextLinux::%s() reg %s: 0x%lx", __FUNCTION__,
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -633,8 +633,11 @@
 default:
   break;
 case 1:
+  return *(const uint8_t *)buffer.bytes;
 case 2:
+  return *(const uint16_t *)buffer.bytes;
 case 4:
+  return *(const uint32_t *)buffer.bytes;
 case 8:
   return *(const uint64_t *)buffer.bytes;
 }


Index: source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -169,7 +169,7 @@
 
   if (error.Success())
 // First cast to an unsigned of the same size to avoid sign extension.
-value.SetUInt64(static_cast(data));
+value.SetUInt(static_cast(data), size);
 
   if (log)
 log->Printf("NativeRegisterContextLinux::%s() reg %s: 0x%lx", __FUNCTION__,
Index: source/Core/RegisterValue.cpp
===
--- source/Core/RegisterValue.cpp
+++ source/Core/RegisterValue.cpp
@@ -633,8 +633,11 @@
 default:
   break;
 case 1:
+  return *(const uint8_t *)buffer.bytes;
 case 2:
+  return *(const uint16_t *)buffer.bytes;
 case 4:
+  return *(const uint32_t *)buffer.bytes;
 case 8:
   return *(const uint64_t *)buffer.bytes;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r282066 - Remove an invalid doxygen `@return` docstring on a void function

[Luke Drummond via lldb-commits]

Author: ldrumm
Date: Wed Sep 21 06:12:50 2016
New Revision: 282066

URL: http://llvm.org/viewvc/llvm-project?rev=282066=rev
Log:
Remove an invalid doxygen `@return` docstring on a void function

`ClangASTSource::FindExternalVisibleDecls` has void return type, so the
previous docstring was misleading.

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h?rev=282066=282065=282066=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.h Wed Sep 
21 06:12:50 2016
@@ -331,9 +331,6 @@ protected:
   /// @param[in] current_id
   /// The ID for the current FindExternalVisibleDecls invocation,
   /// for logging purposes.
-  ///
-  /// @return
-  /// True on success; false otherwise.
   //--
   void FindExternalVisibleDecls(NameSearchContext ,
 lldb::ModuleSP module,


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

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

valentinagiusti added inline comments.


Comment at: 
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:805
@@ -827,2 +804,3 @@
+  if (m_xstate_type == XStateType::Invalid) {
 if (const_cast(this)->ReadFPR().Fail())
   return false;

valentinagiusti wrote:
> valentinagiusti wrote:
> > labath wrote:
> > > Then I think we should make that non-const as well. I mean, what's the 
> > > point of making it const if it does actually modify the state of the 
> > > object. Either that, or implement it in a way that does not modify the 
> > > state.
> > This involves changing the interface of NativeRegisterContextLinux just 
> > because NativeRegisterContextLinux_x86_64 has some specific needs that make 
> > GetRegisterCount use a non-const function. Also, I don't have all the 
> > hardware to test such a cross-platform change.
> Anyway I guess I can submit a follow-up patch with this, I will do it today ;)
So I have looked into this and actually in 
"source/Host/common/NativeRegisterContextRegisterInfo.cpp" the method 
GetRegisterCount() is implemented as const (and the pure virtual method is 
defined in "include/lldb/Host/common/NativeRegisterContext.h"). Unfortunately I 
don't see an easy way 1. to make the method non-const or 2. to avoid that 
GetRegisterSetCount() in NativeRegisterContextLinux_x86_64 needs to call a non 
const function: in order to get the register set count, it must know if the 
XState type is XSAVE, and in order to do that it must call ReadFPR(), or 
directly PtraceWrapper(), which are both non-const.
Could you please tell me what you think is best?
Imho, it would be better to first merge this patch and then clean up the 
NativeRegisterContext code, but I am open to suggestions.


https://reviews.llvm.org/D24764



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

[Lldb-commits] [PATCH] D24793: Delete an (apparently unused) global in ClangExpressionVaraible.cpp

[Luke Drummond via lldb-commits]

ldrumm created this revision.
ldrumm added reviewers: spyffe, clayborg.
ldrumm added a subscriber: LLDB.
ldrumm added a project: LLDB.

I've grepped the entire llvm source tree, and can't find any external 
references, to this symbol.

The build is also clean without it, so I think this is safe to remove.

https://reviews.llvm.org/D24793

Files:
  source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp

Index: source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
@@ -21,8 +21,6 @@
 using namespace lldb_private;
 using namespace clang;
 
-const char *g_clang_expression_variable_kind_name = "ClangExpressionVariable";
-
 ClangExpressionVariable::ClangExpressionVariable(
 ExecutionContextScope *exe_scope, lldb::ByteOrder byte_order,
 uint32_t addr_byte_size)


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.cpp
@@ -21,8 +21,6 @@
 using namespace lldb_private;
 using namespace clang;
 
-const char *g_clang_expression_variable_kind_name = "ClangExpressionVariable";
-
 ClangExpressionVariable::ClangExpressionVariable(
 ExecutionContextScope *exe_scope, lldb::ByteOrder byte_order,
 uint32_t addr_byte_size)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r281717 - [RenderScript] Support tracking and dumping reduction kernels

[Luke Drummond via lldb-commits]

Hi Zachary

Thanks for the review. My comments are inline (I've cut down some of the 
diff context for brevity)


On 16/09/16 15:57, Zachary Turner wrote:
[snip]

-#define MAXLINESTR_(x) "%" STRINGIFY(x) "s"
-#define MAXLINESTR MAXLINESTR_(MAXLINE)
+bool RSModuleDescriptor::ParsePragmaCount(llvm::StringRef *lines,
+  size_t n_lines) {
+  // Skip the pragma prototype line
+  ++lines;
+  for (; n_lines--; ++lines) {
+const auto kv_pair = lines->split(" - ");
+m_pragmas[kv_pair.first.trim().str()] =
kv_pair.second.trim().str();
+  }
+  return true;
+}
+
+bool RSModuleDescriptor::ParseExportReduceCount(llvm::StringRef *lines,
+size_t n_lines) {

This should take an ArrayRef.  In general this is true any
time you're passing an array to a function via (T* items, size_t length).


+  // The list of reduction kernels in the `.rs.info
` symbol is of the form
+  // "signature - accumulatordatasize - reduction_name -
initializer_name -
+  // accumulator_name - combiner_name -
+  // outconverter_name - halter_name"
+  // Where a function is not explicitly named by the user, or is
not generated
+  // by the compiler, it is named "." so the
+  // dash separated list should always be 8 items long
+  Log *log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE);
+  // Skip the exportReduceCount line
+  ++lines;
+  for (; n_lines--; ++lines) {

With ArrayRef this would be written as:

for (StringRef line : lines.drop_front()) {
}

I'm not sure what the technical argument against pointer and length vs 
ArrayRef is, but I agree these range-based for loops are pretty. I'll 
add them in a future commit (I've got some more RenderScript cleanups 
pending, and I'll test this locally first - but this looks like good 
cleanup).


[snip]

__attribute__((kernel))
+   {"exportForEachCount", eExportForEach},
+   // The number of generalreductions: This marked in the
script by `#pragma
+   // reduce()`
+   {"exportReduceCount", eExportReduce},
+   // Total count of all RenderScript specific `#pragmas` used
in the script
+   {"pragmaCount", ePragma},
+   {"objectSlotCount", eObjectSlot}}};

   // parse all text lines of .rs.info 
   for (auto line = info_lines.begin(); line != info_lines.end();
++line) {

How about (for auto line : info_lines) {


We actually need to manually increment the line pointer to skip over the 
already-parsed lines, so the range based for loop doesn't work in this case.




-uint32_t numDefns = 0;
-if (sscanf(line->c_str(), "exportVarCount: %" PRIu32 "",
) == 1) {
-  while (numDefns--)
-m_globals.push_back(RSGlobalDescriptor(this,
(++line)->c_str()));
-} else if (sscanf(line->c_str(), "exportForEachCount: %" PRIu32 "",
-  ) == 1) {
-  while (numDefns--) {
-uint32_t slot = 0;
-name[0] = '\0';
-static const char *fmt_s = "%" PRIu32 " - " MAXLINESTR;
-if (sscanf((++line)->c_str(), fmt_s, , name.data()) ==
2) {
-  if (name[0] != '\0')
-m_kernels.push_back(RSKernelDescriptor(this,
name.data(), slot));
-}
-  }
-} else if (sscanf(line->c_str(), "pragmaCount: %" PRIu32 "",
) ==
-   1) {
-  while (numDefns--) {
-name[0] = value[0] = '\0';
-static const char *fmt_s = MAXLINESTR " - " MAXLINESTR;
-if (sscanf((++line)->c_str(), fmt_s, name.data(),
value.data()) != 0) {
-  if (name[0] != '\0')
-m_pragmas[std::string(name.data())] = value.data();
-}
-  }
-} else {
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE));
-  if (log) {
+const auto kv_pair = line->split(": ");
+const auto key = kv_pair.first;
+const auto val = kv_pair.second.trim();
+
+const auto handler = rs_info_handlers.find(key);
+if (handler == rs_info_handlers.end())
+  continue;

Instead of constructing this map, you could use llvm::StringSwitch, like
this:
int handler = llvm::StringSwitch(key)
.Case("exportVarCount", eExportVar)
.Case("exportForEachCount", eExportForEach)
.Case("exportReduceCount", eExportReduce)
.Case("pragmaCount", ePragma)
.Case("objectSlotCount", eObjectSlot)
.Default(-1);



I wasn't aware at all of `llvm::StringSwitch`. This seems like it is 
more fit for purpose than what I had. Will add.


[snip]

@@ -110,6 +162,7 @@ public:
   const lldb::ModuleSP m_module;
   std::vector 

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

valentinagiusti added inline comments.


Comment at: 
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:805
@@ -827,2 +804,3 @@
+  if (m_xstate_type == XStateType::Invalid) {
 if (const_cast(this)->ReadFPR().Fail())
   return false;

valentinagiusti wrote:
> labath wrote:
> > Then I think we should make that non-const as well. I mean, what's the 
> > point of making it const if it does actually modify the state of the 
> > object. Either that, or implement it in a way that does not modify the 
> > state.
> This involves changing the interface of NativeRegisterContextLinux just 
> because NativeRegisterContextLinux_x86_64 has some specific needs that make 
> GetRegisterCount use a non-const function. Also, I don't have all the 
> hardware to test such a cross-platform change.
Anyway I guess I can submit a follow-up patch with this, I will do it today ;)


https://reviews.llvm.org/D24764



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

Re: [Lldb-commits] [PATCH] D24764: Refactor NativeRegisterContextLinux_x86_64 code.

[Valentina Giusti via lldb-commits]

valentinagiusti added inline comments.


Comment at: 
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:805
@@ -827,2 +804,3 @@
+  if (m_xstate_type == XStateType::Invalid) {
 if (const_cast(this)->ReadFPR().Fail())
   return false;

labath wrote:
> Then I think we should make that non-const as well. I mean, what's the point 
> of making it const if it does actually modify the state of the object. Either 
> that, or implement it in a way that does not modify the state.
This involves changing the interface of NativeRegisterContextLinux just because 
NativeRegisterContextLinux_x86_64 has some specific needs that make 
GetRegisterCount use a non-const function. Also, I don't have all the hardware 
to test such a cross-platform change.


https://reviews.llvm.org/D24764



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