[PATCH] D74450: Add a DWARF transformer class that converts DWARF to GSYM.

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

That's a huge patch and it's difficult for me to tell whether all new code is covered by tests. Ideally the GSYM reader dump functions, for example,  would be a separate patch, etc...



================
Comment at: llvm/include/llvm/MC/StringTableBuilder.h:69
+  bool contains(CachedHashStringRef S) const {
+    return StringIndexMap.find(S) != StringIndexMap.end();
+  }
----------------
I believe the more common pattern is to write this as `return StringIndexMap.count(S)`


================
Comment at: llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp:1
+//===- DwarfTransformer.cpp -------------------------------------*- C++ -*-===//
+//
----------------
Nit: no need to mark a .cpp file as C++.


================
Comment at: llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp:70
+    assert(DwarfFileIdx < FileCache.size());
+    uint32_t GsymFileIdx = FileCache[DwarfFileIdx];
+    if (GsymFileIdx != UINT32_MAX)
----------------
unless you need this for re-entrancy, you can use a `uint32_t &` and avoid the second lookup below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74450