[PATCH] D69767: [LLD][MinGW] Support --allow-multiple-definition

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

Looks nice in general, but a couple minor details



================
Comment at: lld/MinGW/Options.td:29
 def dynamicbase: F<"dynamicbase">, HelpText<"Enable ASLR">;
+
 defm entry: Eq<"entry", "Name of entry point symbol">, MetaVarName<"<entry>">;
----------------
Nit: Stray empty line added here


================
Comment at: lld/test/MinGW/driver.test:44
+RUN: ld.lld -### foo.o -m i386pep --allow-multiple-definitions | FileCheck -check-prefix=FORCE_MULTIPLE %s
+RUN: ld.lld -### foo.o -m i386pep --no-allow-multiple-definitions --allow-multiple-definitions | FileCheck -check-prefix=FORCE_MULTIPLE %s
+FORCE_MULTIPLE: -force:multiple
----------------
The newly added option in Options.td is for `--allow-multiple-definition` without a trailing `s`, but here you're testing with a trailing `s`. I presume it should be without `s`, as that's what ld.bfd supports, and that's what Clang's CrossWindows driver supports.

Two notes though: ld.bfd supports this option with both one and two leading dashes, and we try to match that as far as possible. (Previously lld was quite inconsistent wrt which options happened to handle both cases, but I tried cleaning it up some months ago.)

Secondly, ld.bfd doesn't seem to support the negative form. We can still add support for that (since it makes sense to have both forms), but do add a comment somewhere noting that only the nonnegated form matches ld.bfd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69767