[PATCH] D48757: Implements /force:multiple in lld-link

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: COFF/SymbolTable.cpp:359
   reportDuplicate(S, F);
-  return nullptr;
+  return dyn_cast<DefinedImportData>(S);
 }
----------------
ryan wrote:
> ruiu wrote:
> > Do you have to change this
> I don't know. It just felt in the spirit of /force:multiple to use the existing symbol. (Doesn't impact me personally, I was just wanted /force for function symbols).
> 
> Do you think it's better to return null in this case? (There's other options here too, e.g. forcing the replaceSymbol case when /force:multiple is on.)
Yeah, it probably makes sense to use the first symbol rather than the last one. I just couldn't understand what this change is for. Could you add a testcase that covers this case?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48757