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

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 w

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

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; valentin

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

nitesh.jain added inline comments. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp:1183 @@ +1182,3 @@ + case dwarf_config5_mips64: +return reg_info->byte_offset; + case dwarf_cause_mips: Changing LLDB numbering scheme is not f

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

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::ParsePragma

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

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

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

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; valentin

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

Author: ldrumm Date: Wed Sep 21 06:12:50 2016 New Revision: 282066 URL: http://llvm.org/viewvc/llvm-project?rev=282066&view=rev Log: Remove an invalid doxygen `@return` docstring on a void function `ClangASTSource::FindExternalVisibleDecls` has void return type, so the previous docstring was misl

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

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.

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

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 value

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

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

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

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()/HasXS

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

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-co

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

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/listi

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

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 look

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

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).

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

Author: valentinagiusti Date: Wed Sep 21 08:33:01 2016 New Revision: 282072 URL: http://llvm.org/viewvc/llvm-project?rev=282072&view=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

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

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&id=72036#toc Repository: rL LLVM https://rev

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

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/

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

If it helps, I can tell you that on line 623 of ObjectFilePECOFF.cpp, the export_table has number of names == 64, but address of names = 0. So that seems wrong. On Wed, Sep 21, 2016 at 8:38 AM Zachary Turner wrote: > Unfortunately this is still crashing for me in every single test. Can I > ask

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

Oh, I was doing it from linux, I will do it from Windows then... 2016-09-21 8:42 GMT-07:00 Zachary Turner : > If it helps, I can tell you that on line 623 of ObjectFilePECOFF.cpp, the > export_table has number of names == 64, but address of names = 0. So that > seems wrong. > > On Wed, Sep 21, 2

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

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\llvmbui

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

tfiala added a comment. 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. Comment at: cmake/module

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

Author: zturner Date: Wed Sep 21 11:01:28 2016 New Revision: 282079 URL: http://llvm.org/viewvc/llvm-project?rev=282079&view=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 n

[Lldb-commits] [lldb] r282080 - Fix an inefficient StringRef conversion.

Author: zturner Date: Wed Sep 21 11:01:43 2016 New Revision: 282080 URL: http://llvm.org/viewvc/llvm-project?rev=282080&view=rev Log: Fix an inefficient StringRef conversion. Since the original object was already an llvm::SmallString<> there's no point calling c_str() first. Modified: lldb/t

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

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",

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

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 t

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

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] [lldb] r281717 - [RenderScript] Support tracking and dumping reduction kernels

On Wed, Sep 21, 2016 at 3:50 AM Luke Drummond wrote: > 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 MA

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

Author: zturner Date: Wed Sep 21 12:13:51 2016 New Revision: 282090 URL: http://llvm.org/viewvc/llvm-project?rev=282090&view=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 c

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

tfiala updated this revision to Diff 72084. tfiala added a comment. Updates for Greg's latest comments. https://reviews.llvm.org/D20835 Files: include/lldb/API/SBSourceManager.h include/lldb/Core/Debugger.h include/lldb/Core/Disassembler.h include/lldb/Core/SourceManager.h include/lld

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

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] LLVM buildmaster will be restarted tonight

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] D24749: [CMake] Initial support for LLDB.framework

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

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

Author: jingham Date: Wed Sep 21 14:21:38 2016 New Revision: 282103 URL: http://llvm.org/viewvc/llvm-project?rev=282103&view=rev Log: Probably should add the breakpoint names test directory as well... Added: lldb/trunk/packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_name

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

beanz updated this revision to Diff 72097. beanz added a comment. - Use INSTALL_RPATH and BUILD_WITH_INSTALL_RPATH instead of manually setting the rpath linker flags https://reviews.llvm.org/D24749 Files: CMakeLists.txt cmake/LLDBDependencies.cmake cmake/modules/AddLLDB.cmake cmake/mod

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

tfiala added a comment. Okay, LGTM. 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] [lldb] r282105 - add stop column highlighting support

Author: tfiala Date: Wed Sep 21 15:13:14 2016 New Revision: 282105 URL: http://llvm.org/viewvc/llvm-project?rev=282105&view=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 u

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

Author: cbieneman Date: Wed Sep 21 16:02:16 2016 New Revision: 282110 URL: http://llvm.org/viewvc/llvm-project?rev=282110&view=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

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

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&id=72104#toc Repository: rL LLVM https://reviews.llvm.or

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

Author: emaste Date: Wed Sep 21 16:08:30 2016 New Revision: 282111 URL: http://llvm.org/viewvc/llvm-project?rev=282111&view=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,

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

Author: emaste Date: Wed Sep 21 16:14:31 2016 New Revision: 282112 URL: http://llvm.org/viewvc/llvm-project?rev=282112&view=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.o

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

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&view=rev > Log: > Fix -Wcovered-switch-default warning in StackFrame.cpp > > The switch coveres all possi

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

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();

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

Author: zturner Date: Wed Sep 21 17:33:30 2016 New Revision: 282117 URL: http://llvm.org/viewvc/llvm-project?rev=282117&view=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/v

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

Author: emaste Date: Wed Sep 21 17:36:51 2016 New Revision: 282119 URL: http://llvm.org/viewvc/llvm-project?rev=282119&view=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: htt

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

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

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

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

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

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.llv

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

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 li

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

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.

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

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.llv

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

Eugene.Zelenko added a subscriber: Eugene.Zelenko. Eugene.Zelenko added a comment. Type is still in code. https://reviews.llvm.org/D12158 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-c

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

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 exec

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

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&id=72133#toc Repository: rL LLVM https://reviews.llvm.org/D12158 Files: lldb

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

Author: emaste Date: Wed Sep 21 18:30:36 2016 New Revision: 282123 URL: http://llvm.org/viewvc/llvm-project?rev=282123&view=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

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

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. >

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

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 y

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

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 & Ba

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

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

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

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 t

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

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

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

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 Turn

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

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 cha

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

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.

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

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

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

> 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 call

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

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

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

If you like I can also go through and point out all the places where I *removed* copies. On Wed, 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 t

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

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 efficie

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

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

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

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

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

> 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 corre

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

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

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

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 us

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

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 c

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

> 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

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

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: > >

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

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.

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

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 R

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

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.

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

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.

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

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 platform

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

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.

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

Author: tfiala Date: Wed Sep 21 19:59:23 2016 New Revision: 282128 URL: http://llvm.org/viewvc/llvm-project?rev=282128&view=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

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 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.

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

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.

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 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.

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

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.

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

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.

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

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.

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

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

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

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

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

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

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

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

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

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 hand

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

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

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

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

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

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 t

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

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 In

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

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

  1   2   >