Author: jingham
Date: Thu Jul 19 18:20:18 2018
New Revision: 337515
URL: http://llvm.org/viewvc/llvm-project?rev=337515&view=rev
Log:
Defend LoadImageUsingPaths against a path list
with empty paths on it.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/functionalities/load_using_paths/Te
clayborg created this revision.
clayborg added reviewers: labath, zturner.
Breakpad files sometimes extra 4 bytes of padding after the 32 bit count for
memory, module and thread lists. I also created bare minimum minidump files
that contain both padded and not padded files that test each functio
teemperor updated this revision to Diff 156341.
teemperor added a comment.
- Test case for completing when we only have a forward decl in another file (as
suggested by Fred).
https://reviews.llvm.org/D48465
Files:
include/lldb/Expression/ExpressionParser.h
include/lldb/Expression/UserExpre
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337475: Added unit tests for Flags (authored by teemperor,
committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D49435?vs=155914&id=156319#toc
Repo
Author: teemperor
Date: Thu Jul 19 10:45:51 2018
New Revision: 337475
URL: http://llvm.org/viewvc/llvm-project?rev=337475&view=rev
Log:
Added unit tests for Flags
Reviewers: labath
Reviewed By: labath
Subscribers: labath, mgorny, lldb-commits
Differential Revision: https://reviews.llvm.org/D49
clayborg added a comment.
I am fine if this helps with not pulling in the world. Seems like it should
move out of Core then no? Seems weird to make the change yet keep it in the
same directory.
https://reviews.llvm.org/D48351
___
lldb-commits mail
labath added a comment.
The Stream part isn't the problem. The problem is that the dumping code is
implemented in terms of `ExecutionContextScope`, which then pulls in pretty
much everything. If it was just the object "dumping itself" then I would be
fine with it as a method, but here it's usin
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
I don't agree that moving dump routines away from the class that they are
dumping is a good idea in general. It makes it harder for somebody new to the
code to find out how to print out the
clayborg added a comment.
Is this a case of keeping RegisterValue from needing to link against the stream
stuff so code inside a directory doesn't depend on code from other directories?
Not sure why we wouldn't want to just ask the object itself to dump itself...
https://reviews.llvm.org/D4835
apolyakov updated this revision to Diff 156299.
apolyakov added a comment.
Converted data-info-line python test to a lit one.
https://reviews.llvm.org/D49062
Files:
lit/tools/lldb-mi/data/data-info-line.test
lit/tools/lldb-mi/data/inputs/data-info-line.c
lit/tools/lldb-mi/data/lit.local.c
zturner added inline comments.
Comment at: lit/SymbolFile/PDB/class-layout.test:3
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o
%T/ClassLayoutTest.cpp.obj
+RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main
/OUT:%T/ClassLayoutTest.cpp.exe
+
aleksandr.urakov added inline comments.
Comment at: lit/SymbolFile/PDB/class-layout.test:3
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o
%T/ClassLayoutTest.cpp.obj
+RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main
/OUT:%T/ClassLayoutTest.
zturner added a comment.
Seems good to me. Separating dump code from core logic is always helpful.
https://reviews.llvm.org/D48351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
I guess this is fine. @jingham?
https://reviews.llvm.org/D48351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
labath added a comment.
Does anyone have an opinion on this?
https://reviews.llvm.org/D48351
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Author: labath
Date: Thu Jul 19 07:38:30 2018
New Revision: 337459
URL: http://llvm.org/viewvc/llvm-project?rev=337459&view=rev
Log:
ELF: Replace the header-extension unit test with a lit one
The new test checks that we are actually able to read data from these
kinds of elf headers correctly inst
aleksandr.urakov added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:273
+udt->getClassParent()) {
+ access = GetDefaultAccessibilityForUdtKind(udt_kind);
+}
Yes, we can occur here, as I have mentioned at the line 18
Thank you a lot for explanations and commit!
On Thu, Jul 19, 2018 at 4:40 PM Pavel Labath wrote:
> Thanks.
>
> I agree you're not in the best position to make these kinds of changes
> (and I don't think I would have asked you to do them). In fact, I was
> already considering just changing that f
Thanks.
I agree you're not in the best position to make these kinds of changes
(and I don't think I would have asked you to do them). In fact, I was
already considering just changing that function myself, so I went
ahead and did that now in r337452. As far as I can tell, no tests need
to be update
Author: labath
Date: Thu Jul 19 06:30:56 2018
New Revision: 337452
URL: http://llvm.org/viewvc/llvm-project?rev=337452&view=rev
Log:
Fix whitespace formatting in DWARFExpression::DumpLocation
we were printing an extra space before the start for the expression and
an extra space after some dwarf o
aleksandr.urakov added inline comments.
Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37
+ };
+ union { // Test unnamed union. MSVC treats it as `int a; float b;`
+int a;
Here is a problem. `MicrosoftRecordLayoutBuilder` asserts every field or
aleksandr.urakov added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:186
+return lldb::eAccessProtected;
+ return lldb::eAccessNone;
+}
zturner wrote:
> I wonder if this would be better as an `llvm_unreachable`. Being able to
On Thu, Jul 19, 2018 at 2:14 PM Pavel Labath wrote:
> I knew I should have stayed quiet :P, but now that I am in, here's my
> reasoning:
>
Thank you for not staying quiet, I think it's the only way to have a dialog
and solve the problems :)
I find your argumentation convincing. The problem is th
I knew I should have stayed quiet :P, but now that I am in, here's my reasoning:
> - For eight years (the spaces were commited in 2010) some tests may have
> relied on these spaces. It's a good case - we can run tests and see it;
For seven of those eight years we haven't been using this function
But can you, please, explain me why?
Excuse me for so much fuss, I just wanted to explain my position... We have
two cases now:
- Just commit such a patch without an additional investigation. But it's
not a good idea, because we can't be 100% sure that it is a typo (and even
if it is, we still may
aleksandr.urakov added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:696
+ base_ast_type.GetOpaqueQualType(), access,
+ pdb_base_class->isVirtualBaseClass(), /*base_of_class*/ true);
+ base_classes.push_back(base_spec);
---
On Thu, 19 Jul 2018 at 10:04, Aleksandr Urakov via Phabricator
wrote:
> So peoples who commited such spaces found them beautiful (or used the
> approved formatting style, or used them for some special formatting cases),
Or it's simply a typo.
I don't want to make a bigger fuss of this than it a
aleksandr.urakov added a comment.
Can you explain me, please, why do you think that we should remove these spaces?
I have the following considerations:
- For eight years (the spaces were commited in 2010) some tests may have relied
on these spaces. It's a good case - we can run tests and see it
28 matches
Mail list logo
