amccarth added inline comments.
================ Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:864-866 + // If the StringSwitch above picked any type, including + // eSectionTypeOther, accept that instead of the generic mappings + // based on flags below. ---------------- labath wrote: > This makes pretty weird control flow. I think it would be way clearer if all > of this code were moved into a function like `GetSectionType` (there's a > function like that in ObjectFileELF). Then you can use return statements to > shortcut control flow, like so: > ``` > if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE && > ((const_sect_name == g_code_sect_name) || > (const_sect_name == g_CODE_sect_name))) > return eSectionTypeCode; > if (...) > return eSectionTypeZeroFill; > SectionType type = StringSwitch<SectionType>(name)...; > if (type != Invalid) > return type; > ... > ``` +1. Factoring the decision tree into a separate function is part of what I was trying to ask for in the patch. That it simplifies things with early returns is an added bonus. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70778/new/ https://reviews.llvm.org/D70778 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
