Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
@labath, maybe this is the only patch you did mean?
This is (more or less -- I can't say I had a really exact picture in my head) what I had in mind, when I said that moving the high-level code out of the DWARFUnit class would be useful for the llvm<->lldb dwarf parser convergence. I am aware that you need more for the DWZ stuff -- that is fine, and I like that you have split that into a separate patch.
Anyway, I think this is a good direction to move in. When the time comes, I'd like to get some of clang ast folks to give this a look too, but maybe that's not needed yet...
This moves a lot of lldb-specific stuff out of low-level dwarf code, and as such, I think this is a great step towards the parser unification.
I'd just like to squeeze one interface tweak to function we are moving around, but otherwise, I think this is fine.
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | ||
---|---|---|
4011 | please return the DWARFDeclContext by value here |
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | ||
---|---|---|
4011 | I did the same also for DWARFDebugInfoEntry::GetDWARFDeclContext. It is a part of the updated patch but you can see it separately. |
Looks great, modulo the inline comment.
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | ||
---|---|---|
3986–4007 | Don't use else after return. In fact, you might as well fold the GetDWARFParser call into the if condition (if (DWARFASTParser *parser = ...) parser->...) |
Checked in, thanks for the review.
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | ||
---|---|---|
3986–4007 | This was only moving the code, I did not expect to also refactor it. But I have thus made these code cleanups which I found. |
This commit broke: http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/7400/
Going to revert the return value refactorization.
That DWARFDebugInfoEntry::GetDWARFDeclContext() refactorization for return value was non-trivial and I see I did screw it up I think despite I did not want to. It is now adding it in opposite order.
Reverted the refactorization by: https://github.com/llvm/llvm-project/commit/6dd0163502ff8c48195643b2b08a123c495743a0
Don't use else after return. In fact, you might as well fold the GetDWARFParser call into the if condition (if (DWARFASTParser *parser = ...) parser->...)