[PATCH] D25263: [Driver] Allow setting the default linker during build

Jonas Hahnfeld via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 3 00:31:25 PDT 2016


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





More information about the cfe-commits mailing list