mstorsjo added a comment. No further objections/comments from me on this!
================ 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()); ---------------- alvinhochun wrote: > mstorsjo wrote: > > 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?) > Hmm, I can try to improve the comment. > > The `aliasInt` symbol should trigger this case. Perhaps it will be clearer if > I update the previous patch to include these symbols, so the difference in > output can be seen in this patch. Oh, ok, I had missed that `MapSymbolType` returns code or invalid - I had expected it to return code or data. I guess this is fine then. (Changing that function, or changing the logic here to look at section types is out of scope here anyway, and I don’t know if the difference between data and invalid matters?) 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
