[PATCH] D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 2 inline comments as done.
mstorsjo added inline comments.


================
Comment at: lld/COFF/Symbols.h:232-238
+  bool isEqual(COFFSymbolRef s) const {
+    return va == s.getValue();
+  }
+
+  bool isEqual(uint64_t otherVa) const {
+    return va == otherVa;
+  }
----------------
ruiu wrote:
> Instead of adding these member functions, maybe we should just add an accessor `getVA`? It looks like there's no point to hide the actual VA from other classes.
Sure, I can do that.


================
Comment at: lld/test/COFF/duplicate-absolute-same.s:5
 // RUN: llvm-mc -triple x86_64-windows-msvc -filetype obj -o %t.dupl.obj %t.dupl.s
-// RUN: not lld-link /out:%t.exe %t.obj %t.dupl.obj 2>&1 | FileCheck %s
+// RUN: lld-link /out:%t.exe %t.obj %t.dupl.obj -subsystem:console -entry:entry 2>&1 | FileCheck --allow-empty %s
 
----------------
MaskRay wrote:
> If there is no diagnostic, use `count 0`. Then, `CHECK-NOT` can be turned into a comment.
> 
> Negative diagnostics checks may become stale when the message changes.
Ok, will do.

Yes, there's a clear risk with checking for the error message contents - but if really was an error, it would still be picked up as the lld-link command also would return a failure status. But worth fixing anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71981