This revision was automatically updated to reflect the committed changes.
Closed by commit rL337778: Added unit test for StreamTee (authored by
teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49708?vs=156926=156929#toc
Author: teemperor
Date: Mon Jul 23 17:01:32 2018
New Revision: 337778
URL: http://llvm.org/viewvc/llvm-project?rev=337778=rev
Log:
Added unit test for StreamTee
Reviewers: davide
Reviewed By: davide
Subscribers: davide, mgorny, lldb-commits
Differential Revision:
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
lg. You probably don't need pre-commit reviews for adding tests. This is
obvious goodness.
https://reviews.llvm.org/D49708
___
lldb-commits
teemperor created this revision.
Herald added a subscriber: mgorny.
https://reviews.llvm.org/D49708
Files:
lldb.xcodeproj/project.pbxproj
unittests/Utility/CMakeLists.txt
unittests/Utility/StreamTeeTest.cpp
Index: unittests/Utility/StreamTeeTest.cpp
Author: jmolenda
Date: Mon Jul 23 16:34:50 2018
New Revision: 337774
URL: http://llvm.org/viewvc/llvm-project?rev=337774=rev
Log:
Change sort-pbxproj.rb to find the project.pbxproj in the
most likely locations. And have it overwrite the original
file with the sorted output.
Modified:
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
This is good, but please add a comment explaining the type before committing.
https://reviews.llvm.org/D49271
___
lldb-commits mailing list
Author: gclayton
Date: Mon Jul 23 15:22:46 2018
New Revision: 337758
URL: http://llvm.org/viewvc/llvm-project?rev=337758=rev
Log:
Fix Xcode project for unit tests.
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
shafik marked 6 inline comments as done.
shafik added a comment.
@davide One more pass
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py:8
+
+lldbinline.MakeInlineTest(__file__,
shafik updated this revision to Diff 156886.
shafik marked 4 inline comments as done.
shafik added a comment.
Addressing comments
- -O0 is not needed in Makefile
- engaged is not friendly terminology so switching to "Has Value"
- Switching test away from lldbinline style due to bug w/
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337741: [cmake] Remove unused ${LLDB_PLUGINS} dependency
from our Objective-C++ CMake… (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: teemperor
Date: Mon Jul 23 14:14:52 2018
New Revision: 337741
URL: http://llvm.org/viewvc/llvm-project?rev=337741=rev
Log:
[cmake] Remove unused ${LLDB_PLUGINS} dependency from our Objective-C++ CMake
config
Summary:
LLDB_PLUGINS doesn't exist as a variable, so this line doesn't add any
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
lg
https://reviews.llvm.org/D49695
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337737: [NFC] Minor code refactoring. (authored by
teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Author: teemperor
Date: Mon Jul 23 13:56:49 2018
New Revision: 337737
URL: http://llvm.org/viewvc/llvm-project?rev=337737=rev
Log:
[NFC] Minor code refactoring.
Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D49696
Modified:
teemperor created this revision.
https://reviews.llvm.org/D49696
Files:
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
---
teemperor created this revision.
Herald added a subscriber: mgorny.
LLDB_PLUGINS doesn't exist as a variable, so this line doesn't add any
dependencies and is
just confusing. It seems this slipped in from the gdb-remote CMake I was using
as a CMake template.
The gdb-remote CMake itself is
erik.pilkington added inline comments.
Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
sgraenitz wrote:
> erik.pilkington
sgraenitz added reviewers: erik.pilkington, labath, clayborg, mgorny, davide,
JDevlieghere.
sgraenitz added a comment.
Sorry if the review is a little bumpy, it's my first one. Added all subscribers
as reviewers now. Hope that's ok.
The current version is a rather simple change, that slightly
sgraenitz added a comment.
Well when repeating this test, the values are not always that far apart from
each other, but on average the old USE_BUILTIN_DEMANGLER path the slower one.
Maybe FastDemangle is still faster than IPD in success case, but the overhead
from the fallback cases is adding
erik.pilkington added a comment.
In https://reviews.llvm.org/D49612#1171550, @sgraenitz wrote:
> Quick local performance check doing `target create clang` in current review
> version vs. master HEAD version (both loading the exact same release build of
> clang) looks promising. It's faster
apolyakov added a comment.
In https://reviews.llvm.org/D47302#1171639, @clayborg wrote:
> We could end up moving anything that is set by a target property into the
> lldb_private target property class if it isn't already there and then that
> would fix things.
If you remember, we started
apolyakov added a comment.
You mean that it's unreasonable to provide such an output to stdout since MI
clients are text redactors, IDE and not people?
https://reviews.llvm.org/D49632
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
apolyakov added a comment.
Another approach is to implement SBTargetSettings' functionality such as
serialization inside lldb_private TargetProperties class and then just add an
API to the SBTarget. For example:
`void SBTarget::SerializeProperties(...)`
What do you think about this way?
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337694: Add support for parsing Breakpad minidump files that
can have extra padding in… (authored by gclayton, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: gclayton
Date: Mon Jul 23 07:16:08 2018
New Revision: 337694
URL: http://llvm.org/viewvc/llvm-project?rev=337694=rev
Log:
Add support for parsing Breakpad minidump files that can have extra padding in
the module, thread and memory lists.
Differential Revision:
clayborg added a comment.
We could end up moving anything that is set by a target property into the
lldb_private target property class if it isn't already there and then that
would fix things.
https://reviews.llvm.org/D47302
___
lldb-commits
sgraenitz updated this revision to Diff 156778.
sgraenitz added a comment.
Remove CMake options LLDB_USE_BUILTIN_DEMANGLER and LLDB_USE_LLVM_DEMANGLER.
https://reviews.llvm.org/D49612
Files:
cmake/modules/LLDBConfig.cmake
lldb.xcodeproj/project.pbxproj
source/Core/Mangled.cpp
Author: apolyakov
Date: Mon Jul 23 07:10:30 2018
New Revision: 337692
URL: http://llvm.org/viewvc/llvm-project?rev=337692=rev
Log:
Fix windows build after r337689
Added missing header.
Modified:
lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
Modified: lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp
clayborg added a comment.
If you look at what this patch is doing, it ends sending text to stdout at the
end. So it seems like this function's sole purpose is to report the process
status after stop to STDOUT. Seeing as this is a machine interface (MI) to a
debugger, I was wondering why it
sgraenitz added inline comments.
Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
sgraenitz wrote:
> sgraenitz wrote:
> >
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337689: [lldb-mi] Re-implement data-info-line command.
(authored by apolyakov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: apolyakov
Date: Mon Jul 23 06:02:41 2018
New Revision: 337689
URL: http://llvm.org/viewvc/llvm-project?rev=337689=rev
Log:
[lldb-mi] Re-implement data-info-line command.
Summary: Now data-info-line command uses SB API instead of HandleCommand.
Reviewers: aprantl, clayborg, jingham
sgraenitz marked an inline comment as done.
sgraenitz added a comment.
Quick local performance check doing `target create clang` in current review
version vs. master HEAD version (both loading the exact same release build of
clang) looks promising. It's faster already now, so I would remove the
sgraenitz marked an inline comment as done.
sgraenitz added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:417-423
+option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler" OFF)
+option(LLDB_USE_LLVM_DEMANGLER "Use llvm's new partial demangler" ON)
sgraenitz updated this revision to Diff 156745.
sgraenitz added a comment.
Enable LLDB_USE_LLVM_DEMANGLER on MSVC platforms.
https://reviews.llvm.org/D49612
Files:
cmake/modules/LLDBConfig.cmake
lldb.xcodeproj/project.pbxproj
source/Core/Mangled.cpp
unittests/Core/CMakeLists.txt
sgraenitz updated this revision to Diff 156743.
sgraenitz added a comment.
Fix: Use malloc instead of new for allocating the demangled_name buffer.
https://reviews.llvm.org/D49612
Files:
cmake/modules/LLDBConfig.cmake
lldb.xcodeproj/project.pbxproj
source/Core/Mangled.cpp
labath added a comment.
In https://reviews.llvm.org/D48351#1169959, @jingham wrote:
> Dump is really meant to be the private description of the object that you
> would use for logging and the like - Description was the public face of a
> class. So while the Description-like functionality
labath added a comment.
In https://reviews.llvm.org/D49612#1171395, @sgraenitz wrote:
> > That's fine. It just needs to be able to demangle itanium names when
> > running on an MSVC platform.
>
> IIUC `cstring_mangling_scheme()` should return `eManglingSchemeItanium` in
> this case and switch
sgraenitz added a comment.
> That's fine. It just needs to be able to demangle itanium names when running
> on an MSVC platform.
IIUC `cstring_mangling_scheme()` should return `eManglingSchemeItanium` in this
case and switch to the case label without the `#if defined(_MSC_VER)`.
labath added a comment.
In https://reviews.llvm.org/D49612#1171363, @sgraenitz wrote:
> > this new demangler should also be available in the MSVC case, should it not?
>
> I don't think the Itanium mangler supports MSVC mangling.
That's fine. It just needs to be able to demangle itanium names
sgraenitz added a comment.
Hi all, thanks for your feedback. I will update the review with code fixes in a
bit.
> Also we should compare demangled names between the two to ensure there are no
> differences as we are doing this.
Yes I can definitely do it manually. When it comes to writing
labath added inline comments.
Comment at: cmake/modules/LLDBConfig.cmake:417-423
+option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler" OFF)
+option(LLDB_USE_LLVM_DEMANGLER "Use llvm's new partial demangler" ON)
endif()
if(LLDB_USE_BUILTIN_DEMANGLER)
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
The extra padding is unfortunate, but I guess we have to live with it now.
Looks good. Thanks.
https://reviews.llvm.org/D49579
___
lldb-commits
aprantl added a comment.
@jingham: do you have any opinion about the right SBAPI for manipulating
settings like Alexander outlined?
https://reviews.llvm.org/D47302
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
44 matches
Mail list logo