labath added a comment.
I am only commenting on the c++14 stuff. I think all of the things you need
here are available to us already through various llvm utilities. See inline
comments for details.
For the "meat" of this patchset, I think clayborg is the only one who can do a
proper review of
jankratochvil added a comment.
Going to update the patch.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:92
// C++14: mutable std::shared_timed_mutex m_dwz_uniq_mutex;
mutable std::recursive_mutex m_dwz_uniq_mutex;
labath wrote:
> Is `llvm
labath added a comment.
The thing to be aware of with this testing strategy is that you will probably
be the only person who will be able to run these tests and verify dwz
functionality. If you don't setup a buildbot testing this then other developers
will never even know if they have broken an
Author: labath
Date: Mon Nov 27 05:47:14 2017
New Revision: 319028
URL: http://llvm.org/viewvc/llvm-project?rev=319028&view=rev
Log:
dotest: Mark more android targets as chatty
New android ndk linker started adding more flags to the produced
binaries, which causes older dynamic linkers display wa
Author: labath
Date: Mon Nov 27 09:06:42 2017
New Revision: 319048
URL: http://llvm.org/viewvc/llvm-project?rev=319048&view=rev
Log:
Remove custom TimePoint-formatting code
This was a temporary thing, until llvm has proper support for formatting
time. That time has come, so we can remove the rele
clayborg added a comment.
Jim will need to ok this. A few concerns follow. Most of the time when we don't
get the DWARF -> AST conversion right, it can mean that we might code gen
incorrect code. Is there not enough information for the GNU abi_tag in the
DWARF to actually re-create these classe
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Won't hold up the checkin for the cleaner while loop, but feel free to fix that
and checkin if it works.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:2
clayborg added inline comments.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:216
+ DWARFCompileUnitData *m_data;
+
Is there a reason this is a member variable that I am not seeing? Seems we
could have this class inherit from DWARFCompileUnit
clayborg added a comment.
It would be much easier to read this patch if there were no renames from
"*offset" to "*file_offset". Can we remove these extra renames where it isn't
needed?
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:15
#include
+#include
---
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Just move the eSectionTypeDWARFGNUDebugAltLink enumerator to the end of the
enum and this is good to go.
Comment at: include/lldb/lldb-enumerations.h:647
eS
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Seems like it would be cleaner to leave DWARFDebugInfoEntry and DWARFDie alone
and just make clients deal with accepting DW_TAG_partial_unit when and where
they need to. I don't
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:123
size_t DWARFCompileUnit::ExtractDIEsIfNeeded(bool cu_die_only) {
- const size_t initi
nelhage added a comment.
In https://reviews.llvm.org/D40283#936088, @clayborg wrote:
> Jim will need to ok this. A few concerns follow. Most of the time when we
> don't get the DWARF -> AST conversion right, it can mean that we might code
> gen incorrect code. Is there not enough information fo
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inline comments.
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2050-2052
+if (dwarf_cu->ExtractDIEsIfNeeded(DWARFCompileUnit::
+
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded() seems broken, see inlined
comments.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:114-119
+
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:199-202
+ if (die_ref.cu_offset == DW_INVALID_OFFSET
+ // FIXME: Workaround default cu_
clayborg added a comment.
ok, sounds like the abi_tag stuff is meant to just affect the mangled named
with no code gen implications. Jim should ok this, but I am ok if he is.
https://reviews.llvm.org/D40283
___
lldb-commits mailing list
lldb-commit
sas created this revision.
This method doesn't modify anything in the object it's called on so we
can mark it const to make it usable in a const context.
https://reviews.llvm.org/D40517
Files:
include/lldb/Utility/UUID.h
source/Utility/UUID.cpp
Index: source/Utility/UUID.cpp
=
Author: sas
Date: Mon Nov 27 13:16:37 2017
New Revision: 319095
URL: http://llvm.org/viewvc/llvm-project?rev=319095&view=rev
Log:
Mark UUID::GetByteSize() const
Summary:
This method doesn't modify anything in the object it's called on so we
can mark it const to make it usable in a const context.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319095: Mark UUID::GetByteSize() const (authored by sas).
Repository:
rL LLVM
https://reviews.llvm.org/D40517
Files:
lldb/trunk/include/lldb/Utility/UUID.h
lldb/trunk/source/Utility/UUID.cpp
Inde
sas created this revision.
Formatting needs to be done only once. Ran check-lldb and nothing changes.
https://reviews.llvm.org/D40519
Files:
source/Utility/UUID.cpp
Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
jingham added a comment.
This patch is actually pretty broad, since it will make all C++ declarations
coming from DWARF have overridden names so far as clang is concerned. So we
are relying on the fact that as far as clang is concerned a method with an asm
assigned name is equivalent to method
davide accepted this revision.
davide added a comment.
LGTM
https://reviews.llvm.org/D40519
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added inline comments.
Comment at: source/Utility/UUID.cpp:77-78
void UUID::Dump(Stream *s) const {
- const uint8_t *u = (const uint8_t *)GetBytes();
- s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-
sas updated this revision to Diff 124499.
sas added a comment.
Address @zturner's comment and remove a useless temporary variable.
https://reviews.llvm.org/D40519
Files:
source/Utility/UUID.cpp
Index: source/Utility/UUID.cpp
==
Author: sas
Date: Mon Nov 27 17:26:07 2017
New Revision: 319132
URL: http://llvm.org/viewvc/llvm-project?rev=319132&view=rev
Log:
Remove some duplicated code in UUID.cpp
Summary: Formatting needs to be done only once. Ran check-lldb and nothing
changes.
Reviewers: clayborg, davide
Reviewed By:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319132: Remove some duplicated code in UUID.cpp (authored by
sas).
Repository:
rL LLVM
https://reviews.llvm.org/D40519
Files:
lldb/trunk/source/Utility/UUID.cpp
Index: lldb/trunk/source/Utility/UU
sas created this revision.
This remove a small amount of duplicated code.
https://reviews.llvm.org/D40536
Files:
source/Utility/UUID.cpp
Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
sas created this revision.
This change also prevents looking further than the size of the current
UUID.
https://reviews.llvm.org/D40537
Files:
source/Utility/UUID.cpp
Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.c
sas created this revision.
Herald added subscribers: JDevlieghere, emaste.
In ObjectFileELF we try to read the `.gnu_debuglink` section of the ELF
file to determine the name of the debug symbols file. If this section
does not exist, we stop the search. Instead, what we should do is locate
a file w
zturner added a comment.
You could use llvm's range adapters to make this slightly better.
auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
return llvm::find(Bytes, 0) != Bytes.end();
Another option would just be `return *this != UUID(m_num_uuid_bytes);`
https://reviews.llvm.org/D40537
Hello everyone,
Below are some buildbot numbers for the week of 11/12/2017 - 11/18/2017.
Please see the same data in attached csv files:
The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to
Hello everyone,
Below are some buildbot numbers for the last week of 11/19/2017 -
11/25/2017.
Please see the same data in attached csv files:
The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from gre
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D40536
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lld
davide added a comment.
Yes, what Zachary said. Thanks for the cleanup. There's a decent amount of code
in lldb that can be written in a very compact way but it's manually unrolled. I
think in this case the code produced should be equivalent (and if not, I'd be
curious to see the codegen).
More
davide added subscribers: vsk, aprantl, davide.
davide added a comment.
I thought about this, and it's the only patch from your patchset that I don't
feel really confident about.
I'm afraid it's a little risky to add more code to the objectfile reader
without having proper unittesting.
In partic
As an aside, I don't really like this class. For example, You can
currently assign a UUID[16] to a UUID[20]. That doesn't make a lot of
sense to me.
As a future cleanup, I think this class should probably be a template such
as UUID, and then internally it can store a std::array. And
we can stat
yaml2core would be an excellent idea for a tool.
On Mon, Nov 27, 2017 at 9:48 PM Davide Italiano via Phabricator via
lldb-commits wrote:
> davide added subscribers: vsk, aprantl, davide.
> davide added a comment.
>
> I thought about this, and it's the only patch from your patchset that I
> don't
38 matches
Mail list logo
