[PATCH] D50404: [lld-link] Generalize handling of /debug and /debug:{none,full,fastlink,ghash,symtab}

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.
Herald added a project: LLVM.


================
Comment at: COFF/Driver.cpp:546
+    SmallVector<StringRef, 3> Types;
+    A->getSpelling().split(Types, ',', /*KeepEmpty=*/false);
+
----------------
`getSpelling()` returns the spelling of the name of the option, but not its values. For `/debugtype:pdata`, it returns "/debugtype:" for example. You want `getValue()`, not `getSpelling()`, here.

The tests for `/debugtype:` aren't very good. `invalid-debug-type.test` checks that `/debugtype:invalid` produces a warning. It does: `Types` contains the string `/debugtype:` after the split, which counts as invalid and produces the warning.`pdb-none.test` passes `/debugtype:pdata` but that again just has `/debugtype:` in `Types` and produces the same warning, but the test doesn't use `/WX` or has a `CHECK-NOT` for it, and it doesn't check for the effect of `pdata`.

I.e. the `/debugtype:` switch hasn't been working since this commit, and apparently nobody noticed.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D50404