[PATCH] D19494: [ELF] Introduce --reproduce flag

[Davide Italiano via llvm-commits]

davide added inline comments.

================
Comment at: ELF/Driver.cpp:101-107
@@ -99,1 +100,9 @@
 
+// Should this be moved to LLVM/Support ?
+static std::string getFullPath(StringRef FileName) {
+  SmallString<128> CurPath;
+  sys::fs::current_path(CurPath);
+  sys::path::append(CurPath, FileName);
+  return CurPath.str();
+}
+
----------------
ruiu wrote:
> Why do you need to make a path full path?
I don't think I need. Fixed.

================
Comment at: ELF/Driver.cpp:271
@@ +270,3 @@
+  for (auto Arg: Args)
+    OS << Arg << " ";
+}
----------------
ruiu wrote:
> This would leave a space character at end of the line and does not terminate the line with '\n'. As this is not a speed-critical path, I'd do
> 
>   OS << llvm::join(Args.begin(), Args.end(), " ") << "\n";
I don't think your solution works because the way join works right now.
In fact, it doesnt' even build.

In file included from ../tools/lld/ELF/Driver.cpp:21:
../include/llvm/ADT/StringExtras.h:177:20: error: member reference base type 'const char' is not a structure or union
    Len += (*Begin).size();
           ~~~~~~~~^~~~~
../include/llvm/ADT/StringExtras.h:192:10: note: in instantiation of function template specialization 'llvm::join_impl<const char *>' requested here
  return join_impl(Begin, End, Separator, tag());
         ^
../tools/lld/ELF/Driver.cpp:271:15: note: in instantiation of function template specialization 'llvm::join<const char *>' requested here
  OS << llvm::join(*Args.begin(), *Args.end(), " ");
              ^
1 error generated.


We might want to fix in StringExtras.h, but I'd rather do that later.
I added a '\n' at the end.

================
Comment at: ELF/Driver.cpp:436-437
@@ +435,4 @@
+      Config->ReproduceDir = Arg->getValue();
+    if (llvm::sys::fs::exists(Config->ReproduceDir))
+      error("--reproduce: directory " + Config->ReproduceDir + " already exists.");
+  }
----------------
ruiu wrote:
> This error would be caught in dumpLinkerInvocation, no?
It doesn't seem so, i.e. if I remove the check:

[davide at localhost build-clang]$ rm -rf /home/davide/work/llvm/tools/lld/test/ELF/repro2
[davide at localhost build-clang]$ mkdir /home/davide/work/llvm/tools/lld/test/ELF/repro2
[davide at localhost build-clang]$ /home/davide/work/llvm/build-clang/./bin/not /home/davide/work/llvm/build-clang/./bin/ld.lld /home/davide/work/llvm/build-clang/tools/lld/test/ELF/Output/reproduce.s.tmp -o /home/davide/work/llvm/build-clang/tools/lld/test/ELF/Output/reproduce.s.tmp2 --reproduce /home/davide/work/llvm/tools/lld/test/ELF/repro2
[davide at localhost build-clang]$ 



http://reviews.llvm.org/D19494