[Lldb-commits] [PATCH] D70778: [LLDB] [PECOFF] Factorize mapping section names to types using StringSwitch. NFCI.

[Adrian McCarthy via Phabricator via lldb-commits]

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