[PATCH] D88661: llvm-diva - Debug Information Visual Analizer

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVBasicDefinitions.h:27
+// Symbol to enable internal options and optional debugging code.
+#define LV_INTERNAL_OPTIONS
+
----------------
We usually use LLVM_DEBUG for debugging purposes


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVOptions.h:111
+enum class LVAttributeKind {
+  All,           // --attribute=all
+  Argument,      // --attribute=argument
----------------
One `// --attribute=all` is probably sufficient. It can be easily inferred what other values are accepted.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVSupport.cpp:42
+std::string llvm::logicalview::hexSquareString(uint64_t Value) {
+  std::string String;
+  raw_string_ostream(String) << "[" << hexString(Value) << "]";
----------------
Use Twine (https://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes) for lightweight string composing.


================
Comment at: llvm/tools/llvm-diva/Options.h:23
+
+using namespace llvm::cl;
+
----------------
`using namespace` is discouraged in header files.


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

https://reviews.llvm.org/D88661