[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-15 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 99084. spyffe added a comment. Two changes after Greg and Pavel's suggestions: - Moved the symbol lookup function into the global context; and - Rewrote the test case's build system to be more generic. https://reviews.llvm.org/D33083 Files: include/lldb/S

[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Sean Callanan via Phabricator via lldb-commits
spyffe closed this revision. spyffe added a comment. Committed r303110. https://reviews.llvm.org/D33077 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r303110 - [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Sean Callanan via lldb-commits
Author: spyffe Date: Mon May 15 14:55:20 2017 New Revision: 303110 URL: http://llvm.org/viewvc/llvm-project?rev=303110&view=rev Log: [TypeSystem] Fix inspection of Objective-C object types ptr_refs exposed a problem in ClangASTContext's implementation: it uses an accessor to downcast a QualType t

[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Sean Callanan via Phabricator via lldb-commits
spyffe updated this revision to Diff 99052. spyffe marked an inline comment as done. spyffe added a comment. Updated the Makefile to fix a problem caught by Pavel Labath. Also relocated the new test to lang/objc. https://reviews.llvm.org/D33077 Files: packages/Python/lldbsuite/test/lang/objc/

[Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation

2017-05-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This looks fine. Like Kamil, I think it would help to document your new interfaces. Going away from StringConvert, you are switching from an API that gives a value on fail to one

[Lldb-commits] [lldb] r303076 - Disable a test in TestReturnValue on arm64 linux

2017-05-15 Thread Pavel Labath via lldb-commits
Author: labath Date: Mon May 15 11:25:28 2017 New Revision: 303076 URL: http://llvm.org/viewvc/llvm-project?rev=303076&view=rev Log: Disable a test in TestReturnValue on arm64 linux as described in pr33042, we cannot reliably retrieve the return value on arm64 in cases it is returned via x8 point

[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/Makefile:10 + +include $(LEVEL)/Makefile.rules labath wrote: > This looks like a copy-paste error, as you have everything twice. good catch Repository: r

Re: [Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation

2017-05-15 Thread Zachary Turner via lldb-commits
Indeed, ideally there is no bool return value and we just write return make_error("foo") On Mon, May 15, 2017 at 5:58 AM Pavel Labath via Phabricator < revi...@reviews.llvm.org> wrote: > labath added a comment. > > I think this is a step in the right direction. Besides reducing > boilerplate, this

[Lldb-commits] [lldb] r303061 - Fix darwin build for r303058

2017-05-15 Thread Pavel Labath via lldb-commits
Author: labath Date: Mon May 15 08:41:38 2017 New Revision: 303061 URL: http://llvm.org/viewvc/llvm-project?rev=303061&view=rev Log: Fix darwin build for r303058 Modified: lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Modified: lldb/trunk/source/Plugins/SymbolVendor/Ma

[Lldb-commits] [lldb] r303058 - Remove an expensive lock from Timer

2017-05-15 Thread Pavel Labath via lldb-commits
Author: labath Date: Mon May 15 08:02:37 2017 New Revision: 303058 URL: http://llvm.org/viewvc/llvm-project?rev=303058&view=rev Log: Remove an expensive lock from Timer The Timer destructor would grab a global mutex in order to update execution time. Add a class to define a category once, statica

[Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL303058: Remove an expensive lock from Timer (authored by labath). Changed prior to commit: https://reviews.llvm.org/D32823?vs=97973&id=98994#toc Repository: rL LLVM https://reviews.llvm.org/D32823

[Lldb-commits] [PATCH] D33167: Get rid of some uses of StringConvert and reduce some indentation

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I think this is a step in the right direction. Besides reducing boilerplate, this will also help us ensure correctness, as we get a constant trickle of bug reports for commands which forgot to set the result status. The name is not ideal, but it's probably the best we ca

[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. A bunch more pedantic comments from me. Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:26 + for (int i = 0; i < thread_count; i++) { +threads.push_back(std::thread([&delay]{while(delay);})); + } Could you add

[Lldb-commits] [PATCH] D33083: [Expression parser] Look up module symbols before hunting globally

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Is this feature really darwin specific? Isn't the `__private_extern__` thingy equivalent to `__attribute__(visibility("hidden")))`, which is supported by gcc and clang alike? Comment at: packages/Python/lldbsuite/test/lang/c/conflicting-symbol/Makefile

[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In https://reviews.llvm.org/D32585#752115, @ravitheja wrote: > In https://reviews.llvm.org/D32585#740632, @labath wrote: > > > I quite like that you have added just the packet plumbing code without an > > concrete implementation. However, that is still a significant amoun

[Lldb-commits] [PATCH] D32295: [RFC] Fix crash in expression evaluation

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath abandoned this revision. labath added a comment. Fix is in https://reviews.llvm.org/D32899. https://reviews.llvm.org/D32295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D32899: [RuntimeDyld] Fix debug section relocation (pr20457)

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks for the help, this definitely looks much better. :) Since I already have it written, would you still be interested in the ProcessAllSections MCJIT test by any chance? I noticed that you none of the MCJIT tests set ProcessAllSections=true, or use Modules with debug

[Lldb-commits] [PATCH] D32899: [RuntimeDyld] Fix debug section relocation (pr20457)

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 98967. labath added a comment. Herald added a subscriber: krytarowski. Add llvm-rtdyld test case https://reviews.llvm.org/D32899 Files: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp test/ExecutionEngine/RuntimeDyld/X86/ELF_x86-64_debug_frame.s Index:

[Lldb-commits] [PATCH] D33077: [TypeSystem] Fix inspection of Objective-C object types

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. As this is an objective C feature, wouldn't a better place for it be in testcases/lang/objc ? Comment at: packages/Python/lldbsuite/test/functionalities/ptr_refs-objc/Makefile:10 + +include $(LEVEL)/Makefile.rules This looks like a cop

[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature

2017-05-15 Thread Abhishek via Phabricator via lldb-commits
abhishek.aggarwal added inline comments. Comment at: tools/CMakeLists.txt:8 add_subdirectory(lldb-mi) +option(LLDB_BUILD_INTEL_PT "Enable Building of Intel(R) Processor Trace Tool" OFF) +if (LLDB_BUILD_INTEL_PT) clayborg wrote: > Can we default this to enabled?

[Lldb-commits] [PATCH] D33035: Tool for using Intel(R) Processor Trace hardware feature

2017-05-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I don't really like that we are adding a public shared library for every tiny intel feature. Could we at least merge this "plugin" with the existing "intel-mpx plugin" to create one "intel support" library? Also, adding an external dependency probably deserves a discussi