labath added a subscriber: lldb-commits.
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
If you are confident that the behavior you are documenting is correct, I think
you can submit patches like these without approval. If you do end up send
labath added a comment.
You're using a fairly novel (to this codebase at least) simplification
technique, so I think we should discuss that first. The way we have normally
done these things is to just have the constructor call the Clear() function.
Personally, I am not really sure what to think
Author: jankratochvil
Date: Sun Nov 19 06:35:07 2017
New Revision: 318626
URL: http://llvm.org/viewvc/llvm-project?rev=318626&view=rev
Log:
Add comments to DWARFCompileUnit length fields/methods
Differential revision: https://reviews.llvm.org/D40211
Modified:
lldb/trunk/source/Plugins/Symbol
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318626: Add comments to DWARFCompileUnit length
fields/methods (authored by jankratochvil).
Changed prior to commit:
https://reviews.llvm.org/D40211?vs=123472&id=123498#toc
Repository:
rL LLVM
https
clayborg added a comment.
Good change in the header file. I am not sure I like the destruct this object
in place and replace with new version... If this is commonly done and
acceptable form of C++ I would be ok with it, but I agree with Pavel, it seems
a little bit off the books.
https://revi
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/DWARFCompileUnit.cpp:74
lldb::offset_t *offset_ptr) {
- Clear(
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Feel free to remove any unused code. No need for review on dead code removal.
So just remove the code, don't add #if 0
https://reviews.llvm.org/D40216
___
jankratochvil added a comment.
In https://reviews.llvm.org/D40212#929716, @clayborg wrote:
> Good change in the header file.
If you mean the in-class initializers they obviously cannot be used without the
in-place construction+destruction as they would stay duplicate to the Clear()
method.
O
clayborg added a comment.
In https://reviews.llvm.org/D40212#929740, @jankratochvil wrote:
> In https://reviews.llvm.org/D40212#929716, @clayborg wrote:
>
> > Good change in the header file.
>
>
> If you mean the in-class initializers they obviously cannot be used without
> the in-place construc
jankratochvil added a comment.
Thanks for the review but then it would become a performance regression, not
the performance improvement I was trying to make.
Withdrawing this patch.
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:74
Author: jankratochvil
Date: Sun Nov 19 11:04:24 2017
New Revision: 318631
URL: http://llvm.org/viewvc/llvm-project?rev=318631&view=rev
Log:
Remove 2 unused methods DWARFDebugInfo::Find and their FindCallbackString
Differential revision: https://reviews.llvm.org/D40216
Modified:
lldb/trunk/so
This revision was automatically updated to reflect the committed changes.
Closed by commit rL318631: Remove 2 unused methods DWARFDebugInfo::Find and
their FindCallbackString (authored by jankratochvil).
Changed prior to commit:
https://reviews.llvm.org/D40216?vs=123479&id=123505#toc
Repositor
On Sun, Nov 19, 2017 at 6:35 AM Jan Kratochvil via Phabricator via
lldb-commits wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL318626: Add comments to DWARFCompileUnit length
> fields/methods (authored by jankratochvil).
>
> Changed prior t
When importing C++ methods into clang AST nodes from the DWARF symbol
table, preserve the DW_AT_linkage_name and use it as the linker ("asm")
name for the symbol.
Concretely, this enables `expression` to call into names that use the GNU
`abi_tag` extension, and enables lldb to call into code using
14 matches
Mail list logo
