sbc100 added a comment.

Hi @phosek !

Fixed in rL356847 <https://reviews.llvm.org/rL356847>.

This did make me look a little deeper into this issue though, and wonder if the 
CLANG_DEFAULT_LINKER is implemented in a useful way right now.

I believe the reason this came up for you guys in that you are somehow setting 
CLANG_DEFAULT_LINKER?  Not on the command line but perhaps via the 
`clang/cmake/caches/Fuchsia.cmake` and 
`clang/cmake/caches/Fuchsia-stage2.cmake`?  In regards to that I have two 
questions:

1. Do you need to set CLANG_DEFAULT_LINKER give that you already override 
getDefaultLinker in `clang/lib/Driver/ToolChains/Fuchsia.h`?   I suppose it 
might be useful if you want to overide the linker for other targets too?

2. Does it makes sense for CLANG_DEFAULT_LINKER to able able to override a 
target-specific linker?   In the case of WebAssembly it makes no sense to be 
able to configure an ELF or COFF linker as that default so it would be good for 
us if our `getDefaultLinker` still stands even in the presence of 
CLANG_DEFAULT_LINKER.

Assuming the answer to (1) is "yes we are doing this for some good reason", 
then what do you think about moving the logic for handling CLANG_DEFAULT_LINKER 
into the default implementation of getDefaultLinker in 
`clang/include/clang/Driver/ToolChain.h`.  That way it effects the default, but 
targets can still override the default if they actually care?   They way the 
linker resolution would look more like this:

1. the command line
2. the target-specific default
3. the toolchain-wide default (settable via CLANG_DEFAULT_LINKER)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59721



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to