[PATCH] D53250: [ToolChain] Use default linker if the toolchain uses a custom one

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In https://reviews.llvm.org/D53250#1264708, @Hahnfeld wrote:

> If I read that patch correctly, this will render `-fuse-ld` with non-absolute paths useless if a toolchain has `DefaultLinker != "ld"`. I don't think that's what we want to do if the user explicitly sets a different linker.


You're right, I missed that case, I'll try to address it.

> I agree that these changes are ugly, but we do the same with `-stdlib=platform` and `-rtlib=platform` for tests that check Clang's choices for a particular platform. For this case it's what you did in https://reviews.llvm.org/D53249: Adding `-fuse-ld=ld` to tests that check the default linker for a platform. (because there is a case for `UseLinker == "ld"`)

I agree about the test case but I think the current logic is broken even for regular use. If the platform specifies a custom linker (e.g. `hegaxon-link`), `CLANG_DEFAULT_LINKER` should not override it because it's unlikely that it'll be usable on that platform. Today we treat `CLANG_DEFAULT_LINKER` as a default value for `-fuse-ld=` but I think we should instead treat it as a default value for `ld` (i.e. default system linker) which will be used if `-fuse-ld=` is unspecified or the platform doesn't specify its own linker. Would you agree with that?

The reason why this is not a problem of `-stdlib=` and `-rtlib=` is because many toolchains override `ToolChain::GetRuntimeLibType` and `ToolChain::GetCXXStdlibType` (e.g. Fuchsia <https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/Fuchsia.cpp#L172> or WebAssembly <https://github.com/llvm-mirror/clang/blob/master/lib/Driver/ToolChains/WebAssembly.cpp#L115>) completely bypassing the default logic. The reason why this is not being used for linker selection is because `-fuse-ld=/path/to/linker` is generally useful even for these targets that use custom linkers (e.g. https://reviews.llvm.org/D53038 explicitly mentions that) so I think we either need to make `ToolChain::GetLinkerPath` smarter about handling platform-specific linkers or we need to extract the logic for handling linker specified via absolute path into a helper method so toolchains can override `ToolChain::GetLinkerPath` without re-implementing chunk of its logic.


Repository:
  rC Clang

https://reviews.llvm.org/D53250