mstorsjo added a comment.
Herald added a subscriber: JDevlieghere.
This looks mostly reasonable to me, a couple discussion points only.
================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:817
+ if (symbol_type != lldb::eSymbolTypeInvalid)
+ exported->SetType(symbol_type);
+ if (exported->GetMangled() == symbol.GetMangled()) {
----------------
As a curious question - since D134265 we get better quality for the symbol
types set from the get go - what are the other cases you foresee where this
will make a difference? I don't mind if there isn't any difference though,
syncing the types from the symbol table which is more expressible probably is
good anyway. Just wondering about the actual utility of it.
================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:828
+ // either names will work. Only synchronize the symbol type.
+ if (symbol.GetType() == lldb::eSymbolTypeInvalid)
+ symbol.SetType(exported->GetType());
----------------
This condition had me puzzled for a moment, until I realized that you're
synchronizing symbol type in the other direction here. Is it worth clarifying
this in the comment?
(Also, I presume the test case doesn't really trigger this case?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134426/new/
https://reviews.llvm.org/D134426
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
