[PATCH] D59743: [WebAssembly] Don't use default GetLinkerPath

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 24 18:16:06 PDT 2019


phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: clang/lib/Driver/ToolChains/Fuchsia.h:89
 
-  const char *getDefaultLinker() const override {
-    return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 
----------------
This seems unrelated?


================
Comment at: clang/lib/Driver/ToolChains/MipsLinux.h:51
 
-  const char *getDefaultLinker() const override {
-    return "ld.lld";
-  }
+  const char *getDefaultLinker() const override { return "ld.lld"; }
 
----------------
ditto


================
Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:50
+    }
+
+    // Since 'wasm-ld' is just another name for lld we accept that name,
----------------
I'd also consider handling the `else if (UseLinker.empty() || UseLinker == "ld")` case which should return the default linker (i.e. `wasm-ld`), this matches the behavior of the generic driver logic in `ToolChain` and can be used to select the default linker for the target.


================
Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:57
+
+  return ToolChain.GetProgramPath("wasm-ld");
+}
----------------
Nit: you could use `getDefaultLinker()` instead of hardcoding `wasm-ld` here so if the name ever changes you only need to change the getter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59743





More information about the cfe-commits mailing list