[PATCH] D19551: Handle Windows drive letters and ".." for --reproduce.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 26 12:59:39 PDT 2016


ruiu added inline comments.

================
Comment at: ELF/Driver.cpp:258
@@ -243,6 +257,3 @@
 
-static void dumpLinkerInvocation(ArrayRef<const char *> Args) {
-  std::error_code EC = sys::fs::create_directories(Config->Reproduce,
-    false /* IgnoreExisting */);
-  if (EC) {
-    error(EC, "--reproduce: can't create directory");
+static void logCommandline(ArrayRef<const char *> Args) {
+  if (std::error_code EC = sys::fs::create_directories(
----------------
davide wrote:
> I prefer dumpLinkerInvocation still.
To me this name is more plain and therefore good. It just logs command line -- I can tell it would obviously have no side effect (except writing a log somewhere) just by looking at the name. But dumpLinkerInvocation sounds something more complex.

================
Comment at: ELF/Driver.cpp:272
@@ -257,3 +271,3 @@
   OS << Args[0];
-  for (unsigned I = 1, E = Args.size(); I < E; ++I)
+  for (size_t I = 1, E = Args.size(); I < E; ++I)
     OS << " " << Args[I];
----------------
davide wrote:
> We use unsigned everywhere else in the code base, try to be consistent or change all the other occurrences.
I don't think we do. I actually use size_t where it means the size of an array. This is also technically accurate (even though it is unrealistic that we have more than 2^32 arguments).


http://reviews.llvm.org/D19551





More information about the llvm-commits mailing list