shafik added a comment.
@vsk @jingham I believe I have addressed your comments, please review again.
Comment at:
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp:29
+std::variant v3;
+std::variant v_no_value;
+
shafik updated this revision to Diff 165201.
shafik marked 9 inline comments as done.
shafik added a comment.
Address comments on std::variant formatter
- Adding tests for reference to variant and variant of variant
- Refactoring test to be more efficient
- Refactoring unsafe code that assumed
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
This looks good. I agree with Jonas, however that we might have cases where we
end up with two identical completions that have different descriptions, and we
shouldn't drop them.
jingham updated this revision to Diff 165184.
jingham added a comment.
Add documentation for SBTarget.CreateBreakpointFromScript, and fixed a few
minor nits that Greg pointed out.
https://reviews.llvm.org/D51830
Files:
include/lldb/API/SBAddress.h
include/lldb/API/SBBreakpoint.h
jingham added a comment.
I addressed most of the comments. I remember we argued about how the filters
should work w.r.t. the resolvers back when I first did this, but I still claim
my way is right ;-) I gave some arguments for that in response to your inline
comment.
I'm going to save this
zturner added a comment.
I've been experimenting with DIA locally and after some investigation I'm not
sure this is going to be reliable. Let's say we have a class, we want the decl
context containing the class. For example, on line 366. So we call
`GetDeclContextContainingSymbol`. Despite
Author: jmolenda
Date: Wed Sep 12 14:35:02 2018
New Revision: 342085
URL: http://llvm.org/viewvc/llvm-project?rev=342085=rev
Log:
Commit my attempt to test the change to ProcessGDBRemote
in r336956. This test doesn't actually test the change
that was submitted by Venkata, but it's a good one to
zturner accepted this revision.
zturner added inline comments.
This revision is now accepted and ready to land.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:274
- auto class_parent_id = raw.getClassParentId();
- if (auto class_parent =
clayborg added a comment.
In https://reviews.llvm.org/D51935#1232203, @grimar wrote:
> In https://reviews.llvm.org/D51935#1232195, @clayborg wrote:
>
> > Patch looks good. A few fixes mentioned in inlined comments. And I am
> > unsure how the test suite will run with various compilers if they
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB342075: Do not create new terminals when launching
process on Windows with --no-stdio (authored by xbolva00, committed by ).
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51966
Files:
tatyana-krasnukha added a comment.
Everything that includes Host/Editline.h (Host/Editline.cpp,
Core/IOHandler.cpp, ...). Build fails with "fatal error: histedit.h: No such
file or directory"
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51999
davide added a comment.
How exactly are you building (i.e. what triggers this error)?
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51999
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
tatyana-krasnukha created this revision.
tatyana-krasnukha added a reviewer: compnerd.
Herald added subscribers: lldb-commits, mgorny.
Without that build of Host, Core and Interpreter with custom libedit fails.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51999
Files:
CMakeLists.txt
Author: jmolenda
Date: Wed Sep 12 12:30:03 2018
New Revision: 342072
URL: http://llvm.org/viewvc/llvm-project?rev=342072=rev
Log:
If we fail to get an armv7em-- disassembler from llvm, skip the
tests and don't mark this as a failure. This happens when we've
linked against an llvm without the ARM
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342066: Add compatibility version to liblldb in framework
builds (authored by xiaobai, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D51959
Author: xiaobai
Date: Wed Sep 12 11:10:22 2018
New Revision: 342066
URL: http://llvm.org/viewvc/llvm-project?rev=342066=rev
Log:
Add compatibility version to liblldb in framework builds
Summary:
Building LLDB with xcodebuild sets the compatibility version of liblldb
in LLDB.framework. Building
xiaobai updated this revision to Diff 165126.
xiaobai added a comment.
Fix typo (Thanks for the catch!)
https://reviews.llvm.org/D51959
Files:
source/API/CMakeLists.txt
Index: source/API/CMakeLists.txt
===
---
grimar added a comment.
In https://reviews.llvm.org/D51935#1232195, @clayborg wrote:
> Patch looks good. A few fixes mentioned in inlined comments. And I am unsure
> how the test suite will run with various compilers if they don't support the
> -gdwarf-5 flag. We might need to run obj2yaml on
clayborg requested changes to this revision.
clayborg added a comment.
Patch looks good. A few fixes mentioned in inlined comments. And I am unsure
how the test suite will run with various compilers if they don't support the
-gdwarf-5 flag. We might need to run obj2yaml on a binary and then
grimar planned changes to this revision.
grimar added a comment.
Will reupload.
https://reviews.llvm.org/D51935
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
grimar updated this revision to Diff 165112.
grimar added a comment.
- Addressed review comments.
- Added test case.
- Minor cosmetic changes.
https://reviews.llvm.org/D51935
Files:
include/lldb/lldb-enumerations.h
packages/Python/lldbsuite/test/functionalities/show_location/Makefile
zturner accepted this revision.
zturner added a comment.
This revision is now accepted and ready to land.
I'm fine with this.
https://reviews.llvm.org/D51966
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
labath added a comment.
This makes sense to me, but the windows folks should say what is best for their
platform.
https://reviews.llvm.org/D51966
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
xbolva00 updated this revision to Diff 165103.
xbolva00 retitled this revision from "Do not create new terminals when
launching process on Windows by default" to "Do not create new terminals when
launching process on Windows with --no-stdio".
xbolva00 edited the summary of this revision.
clayborg added a comment.
In https://reviews.llvm.org/D51966#1232057, @xbolva00 wrote:
> So basically the hardest problem is to "find a way to get stdio from a
> process and feed to to LLDB so it shows up when neither --tty nor --no-stdio
> is set" ..
yes. The easier solution is to just
Sounds good, thanks for clarifying. Just want to be safe.
> On Sep 12, 2018, at 9:09 AM, Pavel Labath wrote:
>
> On 12/09/18 16:37, Greg Clayton wrote:
>> Don't we get a blob of register bytes back from ptrace when reading
>> registers? The struct we use must match this byte for byte.
>
>
On 12/09/18 16:37, Greg Clayton wrote:
> Don't we get a blob of register bytes back from ptrace when reading
> registers? The struct we use must match this byte for byte.
That is true. However, this alignment does not set the layout of the
fields in the struct. Rather it sets the alignment of
xbolva00 added a comment.
So basically the hardest problem is to "find a way to get stdio from a process
and feed to to LLDB so it shows up when neither --tty nor --no-stdio is set" ..
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51966
___
JDevlieghere updated this revision to Diff 165094.
JDevlieghere added a comment.
- Document default behavior.
https://reviews.llvm.org/D51934
Files:
packages/Python/lldbsuite/test/functionalities/target_create_deps/Makefile
clayborg added a comment.
The other option is to only remove this flag if "--no-stdio" is set.
The best solution IMHO is:
- find a way to get stdio from a process and feed to to LLDB so it shows up
when neither --tty nor --no-stdio is set
- when --tty is specified, set the CREATE_NEW_CONSOLE
xbolva00 added a comment.
In https://reviews.llvm.org/D51966#1231943, @clayborg wrote:
> We should obey the --tty option here and set CREATE_NEW_CONSOLE if it is set
> no?
I also thought about it. @zturner, @labath ?
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51966
clayborg added inline comments.
Comment at: source/Commands/CommandObjectTarget.cpp:143-151
+static OptionEnumValueElement g_dependents_enumaration[4] = {
+{eLoadDependentsDefault, "default",
+ "Only load dependents when the target is an executables."},
+
Don't we get a blob of register bytes back from ptrace when reading registers?
The struct we use must match this byte for byte. This seems like this change
could affect the ability to get correct register values?
> On Sep 12, 2018, at 1:50 AM, Pavel Labath via lldb-commits
> wrote:
>
>
clayborg added a comment.
We should obey the --tty option here and set CREATE_NEW_CONSOLE if it is set no?
Repository:
rLLDB LLDB
https://reviews.llvm.org/D51966
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
clayborg added a comment.
Only issue now is documentation and why are we requiring addresses to be in
compile unit. See inlined comments.
Comment at: include/lldb/API/SBTarget.h:667
+ lldb::SBBreakpoint BreakpointCreateFromScript(
+ const char *class_name,
+
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB342050: Move SafeMachO from Utility to Host (authored by
labath, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50383?vs=165058=165070#toc
Repository:
rLLDB LLDB
Author: labath
Date: Wed Sep 12 05:26:11 2018
New Revision: 342051
URL: http://llvm.org/viewvc/llvm-project?rev=342051=rev
Log:
Fix two issues in PDBASTParser
- gcc warning about using binary or for or-ing two comparisons (a == b | a == c)
- llvm style prefers static functions to functions in an
Author: labath
Date: Wed Sep 12 05:26:05 2018
New Revision: 342050
URL: http://llvm.org/viewvc/llvm-project?rev=342050=rev
Log:
Move SafeMachO from Utility to Host
Summary:
One of the conclusions of the discussion on D49740 was that SafeMachO is better
off in the Host module (as that's the only
Author: d0k
Date: Wed Sep 12 04:31:18 2018
New Revision: 342047
URL: http://llvm.org/viewvc/llvm-project?rev=342047=rev
Log:
Remove another unused mislayered include.
Modified:
lldb/trunk/source/Target/CPPLanguageRuntime.cpp
Modified: lldb/trunk/source/Target/CPPLanguageRuntime.cpp
URL:
Author: d0k
Date: Wed Sep 12 04:27:10 2018
New Revision: 342046
URL: http://llvm.org/viewvc/llvm-project?rev=342046=rev
Log:
Remove unused include that's also a layering violation.
Modified:
lldb/trunk/source/Target/CPPLanguageRuntime.cpp
Modified:
grimar marked 5 inline comments as done.
grimar added a comment.
Thanks for the review, Greg! I'll address your comments and add the test case
when it will be ready. Hope soon.
https://reviews.llvm.org/D51935
___
lldb-commits mailing list
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
https://reviews.llvm.org/D50383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
labath updated this revision to Diff 165058.
labath added a comment.
Add the file to module map.
https://reviews.llvm.org/D50383
Files:
include/lldb/Host/SafeMachO.h
include/lldb/Utility/SafeMachO.h
include/lldb/module.modulemap
source/Host/common/Symbols.cpp
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342044: Remove manual byte counting from internal Stream
methods. (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
Author: teemperor
Date: Wed Sep 12 03:20:41 2018
New Revision: 342044
URL: http://llvm.org/viewvc/llvm-project?rev=342044=rev
Log:
Remove manual byte counting from internal Stream methods.
Summary:
This patch removes the manual byte counting in all internal Stream methods.
This is now done by
JDevlieghere updated this revision to Diff 165047.
JDevlieghere added a comment.
I totally agree; I've left the argument name unchanged and made the argument
optional. I had to change the semantics slightly (because of the negation in
the argument name) but otherwise things will continue
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342042: Add a basic test for memory region
(authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51930?vs=164862=165045#toc
Author: teemperor
Date: Wed Sep 12 03:04:25 2018
New Revision: 342042
URL: http://llvm.org/viewvc/llvm-project?rev=342042=rev
Log:
Add a basic test for 'memory region'
Summary:
The 'memory region' command is at the moment not tested at all by our test
suite.
This patch just adds a basic test
Author: labath
Date: Wed Sep 12 01:50:08 2018
New Revision: 342029
URL: http://llvm.org/viewvc/llvm-project?rev=342029=rev
Log:
Reduce alignment on struct XSAVE, fixing a gcc warning
The warning is about heap-allocating a struct with bigger alignment
requirements than the standard heap allocator
xbolva00 abandoned this revision.
xbolva00 added a comment.
In https://reviews.llvm.org/D51966#1231533, @labath wrote:
> I think the issue here is that on windows we don't have the ability(*) to
> pipe stdio through lldb, so if you create a process without a console, you
> will not be able to
aleksandr.urakov created this revision.
aleksandr.urakov added a reviewer: zturner.
aleksandr.urakov added a project: LLDB.
Herald added a subscriber: lldb-commits.
This patch adds some symbol tag checks before using the `IPDBRawSymbol`
interface to improve safety and readability.
Repository:
labath added a comment.
I think the issue here is that on windows we don't have the ability(*) to pipe
stdio through lldb, so if you create a process without a console, you will not
be able to use see it's output or pass it some input (probably fine for gui
apps, but bad for console ones).
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
lgtm modulo typos
https://reviews.llvm.org/D51959
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
xbolva00 created this revision.
xbolva00 added reviewers: teemperor, zturner.
Herald added subscribers: lldb-commits, abidh.
Unify behaviour across platforms. (We create new terminals only on Windows).
process launch offers explicit option:
-t ( --tty )
Start the process in a terminal
54 matches
Mail list logo
