[PATCH] D46608: Add support for ThinLTO plugin option thinlto-object-suffix-replace

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 14 15:56:24 PDT 2018


ruiu added inline comments.


================
Comment at: lld/ELF/Driver.cpp:822-823
+        error(
+            "thinlto-object-suffix-replace expects 'old;new' format, but got " +
+            S.substr(30));
     } else if (!S.startswith("/") && !S.startswith("-fresolution=") &&
----------------
nit: it is perhaps better to split the long literal string into two like this

  error("thinlto-object-suffix-replace expects 'old;new' format, "
        "but got " + S.substr(30));

so that the code looks better.


================
Comment at: lld/ELF/InputFiles.cpp:1037
+  if (Config->ThinLTOIndexOnly) {
+    size_t pos = Path.rfind(Config->ThinLTOObjectSuffixReplace.first.str());
+    if (pos != std::string::npos)
----------------
I don't think this is correct. Don't you need to use ends_with?


================
Comment at: lld/ELF/LTO.cpp:270-278
+      if (Config->ThinLTOIndexOnly) {
+        size_t pos = Path.rfind(Config->ThinLTOObjectSuffixReplace.first.str());
+        if (pos != std::string::npos)
+          Path = (Path.substr(0, pos) +
+                  Config->ThinLTOObjectSuffixReplace.second.str());
+        else
+          error("cannot find suffix " +
----------------
The amount of code duplication is growing, and you probably should do something to against it.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46608





More information about the llvm-commits mailing list