[PATCH] D76999: [ELF] Print versioned name for better "undefined symbol" diagnostics

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/Symbols.cpp:41
+  // part. This check is safe because every symbol name ends with '\0'.
+  if (name.data()[name.size()] == '@')
+    ret += name.data() + name.size();
----------------
grimar wrote:
> Though it is technically looks correct at this moment, it looks a bit hacky still. It violates the contract `StringRef` provides:
> it is unexpected that user will access the buffer outside of its `Size`.
> 
> Is it a time to make `toString` method a member of `Symbol`? Since it is a global function that now relies on the assumption about internal implementation. (With that you will be able to access `nameData` member directly, what should be cleaner).
> 
> I also wonder what other people think. May be it just me who think it is a bit too hacky :)
This my concern perhaps should not be a stopper for this patch, since you have LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76999