[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 10 inline comments as done.
jankratochvil added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2016
+        for (size_t i = 0; i < num_matches; ++i) {
+          const DIERef &die_ref = method_die_offsets[i];
           DWARFDebugInfo &debug_info = dwarf->DebugInfo();
----------------
aprantl wrote:
> Can this be made into a range-based for loop in a separate commit?
In D77327 it gets changed into a callback anyway so `method_die_offsets` no longer exists there.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:70
     const dw_tag_t die_tag = die_info_array[i].tag;
-    if (die_tag != 0 && die_tag != DW_TAG_class_type &&
-        die_tag != DW_TAG_structure_type)
+    if (!(die_tag == 0 || die_tag == DW_TAG_class_type ||
+          die_tag == DW_TAG_structure_type))
----------------
shafik wrote:
> I also feel like the previous version was more readable. Is there another gain that I am missing?
I find both versions OK but this patch is going to revert my change in rG80237523193d so everyone should be happy.



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2057
 
-      if (die) {
-        switch (die.Tag()) {
-        default:
-        case DW_TAG_subprogram:
-        case DW_TAG_inlined_subroutine:
-        case DW_TAG_try_block:
-        case DW_TAG_catch_block:
-          break;
+  for (size_t i = 0; i < num_die_matches; ++i) {
+    const DIERef &die_ref = die_offsets[i];
----------------
aprantl wrote:
> same here (later)
In D77327 it gets changed into a callback anyway so `die_offsets` no longer exists there.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2915
+          if (!log)
+            continue;
+          std::string qualified_name;
----------------
aprantl wrote:
> These two continues IMO are a bit confusing to read this way. Perhaps in this case an if (log) block with just one continue at the end is easier to read. Up to you.
OK, changed back to `if (log) { ... }`. I even was not sure with that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77326