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
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 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);
---
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
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
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
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
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
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
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
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
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
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
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
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
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
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 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
+
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
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
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
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
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
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
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
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
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
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
28 matches
Mail list logo
