Hahnfeld added a comment.

I have two high level remarks here:

1. `CLANG_DEFAULT_LINKER` should override whatever the platform default is. So 
`ToolChain::getDefaultLinker()` should return `ld` as the variable 
`DefaultLinker` currently says and the logic should be in 
`ToolChain::GetLinkerPath()`.
2. The CMake variable is documented to be empty for the platform default. I 
think this will currently not work when used by 
`GetProgramPath(getDefaultLinker())`, will it?



================
Comment at: lib/Driver/ToolChain.cpp:377
     getDriver().Diag(diag::err_drv_invalid_linker_name) << 
A->getAsString(Args);
-    return "";
   }
----------------
You can leave this `return` because Clang will exit anyway after emitting the 
error (as I had to learn...)


================
Comment at: lib/Driver/ToolChain.cpp:721-724
+
+const char *ToolChain::getDefaultLinker() const {
+  return CLANG_DEFAULT_LINKER;
+}
----------------
I think this could go into the header


Repository:
  rL LLVM

https://reviews.llvm.org/D25263



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

Reply via email to