[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl accepted this revision. aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; Could you add a

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 427081. shafik removed reviewers: teemperor, jingham, jasonmolenda. shafik added a comment. Herald added a project: All. - Expanded test - applied clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +other_die = dcu->LookupAddress( +symtab->SymbolAtIndex(symbol_indexes[index])->GetFileAddress()); + } should there be an early exit

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I'm really sorry, I don't understand what kind of review is expected here given the state of this patch. - The patch **still** doesn't include the tests that I asked for in my

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:402-403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; break; JDevlieghere wrote: > What's the

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 358773. shafik marked 8 inline comments as done. shafik added a comment. - Removed virtual from `FindTypeForAutoReturnForDIE` - Added missing nullptr checks - Modernized the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

JDevlieghere added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:402-403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; break; What's the purpose of this?

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I guess Phabricator just sent this back to review automatically when you updated the diff? Just mark this as 'needs review' again when this is ready for review. I'll send this

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:406 + virtual lldb::TypeSP + FindTypeForAutoReturnForDIE(const DWARFDIE , teemperor wrote: > If this is virtual then I guess `SymbolFileDWARFDwo` should overload

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 358373. shafik added a comment. - Modified `FindTypeForAutoReturnForDIE` to take into account if we have multiple symbols with the same name. - Modified `ParseSubroutine` to take into account that case we get the definition first, this can happen when we set

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

clayborg requested changes to this revision. clayborg added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1305 + + // If a function has an auto return type we need to find the defintion since + // that will have the deduced return

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. I think this looks good, thanks for fixing this! I believe there should be a few more tests for all the corner cases we can run into here (those tests can all be just Python

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added a comment. In D105564#2867717 , @probinson wrote: >> Currently when we have a member function that has an auto return type and >> the definition is out of line we generate two DWARF DIE. >> One for the declaration and a second one for the

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

probinson added a comment. > Currently when we have a member function that has an auto return type and the > definition is out of line we generate two DWARF DIE. > One for the declaration and a second one for the definition, the definition > holds the deduced type but does not contain a

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added a comment. @aprantl after your comments and discussion offline I changed my approach to do this lookup using the symbol table and it worked out. The main issue with the first approach was that gcc would also have to be updated in order for them to change their approach to

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 357578. shafik added reviewers: jingham, jasonmolenda. shafik added a comment. Changing approach based on Adrian's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564 Files:

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added a comment. Could LLDB find the linkage name on the declaration, look that name up in the symbol table, and find the DW_TAG_subprogram DIE for the symbol's start address? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +if (!try_resolving_type) + return true; + aprantl wrote: > This block looks like it's more complicated than it needs to be. Could you > just say >

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added a comment. I think it would be best to split out the Clang change into a separately tested patch. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2693 +if (!try_resolving_type) + return true; + This block looks like

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

aprantl added a comment. I think it would be best to split out the Clang change into a separately tested patch. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1694 + // If the declared return type is "auto" we want the linkage name to go + // with the defintion. In

[Lldb-commits] [PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik created this revision. shafik added reviewers: aprantl, teemperor, labath. Herald added a subscriber: arphaman. shafik requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Currently when we have a member function that has an auto