[PATCH] D88097: [llvm-readelf/obj] - Cleanup the code. NFCI.

[George Rimar via Phabricator via llvm-commits]

grimar marked 4 inline comments as done.
grimar added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:3941
+  Fields[2].Str = to_string(format_decimal(Symbol.st_size, 5));
+  Fields[2].Str = to_string(format_decimal(Symbol.st_size, 5));
 
----------------
jhenderson wrote:
> Delete duplicated line.
An issue of rebasing I guess. Thsnks!


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4967
     OS << "    Properties:";
-    for (const auto &Property : getGNUPropertyList<ELFT>(Desc))
+    for (const std::string &Property : getGNUPropertyList<ELFT>(Desc))
       OS << "    " << Property << "\n";
----------------
jhenderson wrote:
> 
`getGNUPropertyList` returns `SmallVector<std::string, 4>`. I think it is better to avoid an implicit conversion.
Switching to `StringRef` might also lead to a behavior change (I don't expect it, but in theory), because `operator<<` is implemented differently:

```
  raw_ostream &operator<<(StringRef Str) {
    // Inline fast path, particularly for strings with a known length.
    size_t Size = Str.size();

    // Make sure we can use the fast path.
    if (Size > (size_t)(OutBufEnd - OutBufCur))
      return write(Str.data(), Size);

    if (Size) {
      memcpy(OutBufCur, Str.data(), Size);
      OutBufCur += Size;
    }
    return *this;
  }

```

```
  raw_ostream &operator<<(const std::string &Str) {
    // Avoid the fast path, it would only increase code size for a marginal win.
    return write(Str.data(), Str.length());
  }
```

I'd like to keep this patch as NFC.


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

https://reviews.llvm.org/D88097