[PATCH] D54384: [llvm-objcopy] Add --build-id-link-dir flag

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/Note.h:18
+namespace elf {
+namespace {
+
----------------
i think we should not have anonymous namespaces in headers,
every time this header is included everything will be duplicated.
so i would remove this anonymous namespace, leave only the "public" interface declarations in this header and move the details of implementation into .cpp


================
Comment at: llvm/tools/llvm-objcopy/ELF/Note.h:22
+
+static const char *ELF_NOTE_GNU = "GNU";
+
----------------
mcgrathr wrote:
> static is always superfluous inside an anonymous namespace.
> This should be `const char *const` or `constexpr` if LLVM allows newer C++.
> Except it's unused, so it shouldn't be here at all.
if i remember correctly the existing Visual Studio build bots were not happy with this, though things might have changed.


Repository:
  rL LLVM

https://reviews.llvm.org/D54384