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

Martin Storsjö via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 28 00:33:18 PST 2019


mstorsjo marked an inline comment as done.
mstorsjo added inline comments.


================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:803-804
+       (const_sect_name == g_CODE_sect_name))) {
+    return eSectionTypeCode;
+  } else if (sect.flags & llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
+             ((const_sect_name == g_data_sect_name) ||
----------------
labath wrote:
> Now that this is a `return`, you don't need the `else` as per <http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return>.
Ok. What about the individual if/return/else/return within the if statements? Is it preferred to keep them as is here, e.g.

```
if (condition) {
  if (othercondition)
    return eSectionTypeZeroFill;
  else
    return eSectionTypeData;
}
```

Or to skip the inner else in the same way?
```
if (condition) {
  if (othercondition)
    return eSectionTypeZeroFill;
  return eSectionTypeData;
}
```

(Ternary operators for keeping just a single return here would be bad for readability IMO, as some conditions aren't entirely trivial.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70778/new/

https://reviews.llvm.org/D70778





More information about the lldb-commits mailing list